Merge pull request #41473 from jamiehannaford/bump-ir-cov

Automatic merge from submit-queue (batch tested with PRs 42379, 42668, 42876, 41473, 43260)

Bump test coverage for initialresources

**What this PR does / why we need it**:

Increases test coverage for `plugin/pkg/admission/initialresources` to 80.2%.

**Which issue this PR fixes** 

https://github.com/kubernetes/kubernetes/issues/39559

**Special notes for your reviewer**:

Some of these features require E2E tests to achieve 100% coverage, the tests here are just unit tests, so I've tried to delineate as much as possible. I also saw that we might be exporting a lot of this to Heapster in the future.

**Release note**:

```release-note
Test coverage for initialresources increased to 80.2% percent.
```

plugin/pkg/admission/initialresources/BUILD

@@ -40,13 +40,19 @@ go_test(
     name = "go_default_test",
     srcs = [
         "admission_test.go",
+        "data_source_test.go",
+        "gcm_test.go",
         "hawkular_test.go",
+        "influxdb_test.go",
     ],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
         "//vendor:github.com/stretchr/testify/require",
+        "//vendor:golang.org/x/oauth2",
+        "//vendor:golang.org/x/oauth2/google",
+        "//vendor:google.golang.org/api/cloudmonitoring/v2beta2",
         "//vendor:k8s.io/apimachinery/pkg/api/resource",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apiserver/pkg/admission",

plugin/pkg/admission/initialresources/admission_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package initialresources
 
 import (
+	"errors"
 	"testing"
 	"time"
 
@@ -112,15 +113,48 @@ func expectNoAnnotation(t *testing.T, pod *api.Pod) {
 func admit(t *testing.T, ir admission.Interface, pods []*api.Pod) {
 	for i := range pods {
 		p := pods[i]
-		if err := ir.Admit(admission.NewAttributesRecord(p, nil, api.Kind("Pod").WithVersion("version"), "test", p.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)); err != nil {
+
+		podKind := api.Kind("Pod").WithVersion("version")
+		podRes := api.Resource("pods").WithVersion("version")
+		attrs := admission.NewAttributesRecord(p, nil, podKind, "test", p.ObjectMeta.Name, podRes, "", admission.Create, nil)
+		if err := ir.Admit(attrs); err != nil {
 			t.Error(err)
 		}
 	}
 }
 
+func testAdminScenarios(t *testing.T, ir admission.Interface, p *api.Pod) {
+	podKind := api.Kind("Pod").WithVersion("version")
+	podRes := api.Resource("pods").WithVersion("version")
+
+	var tests = []struct {
+		attrs       admission.Attributes
+		expectError bool
+	}{
+		{
+			admission.NewAttributesRecord(p, nil, podKind, "test", p.ObjectMeta.Name, podRes, "foo", admission.Create, nil),
+			false,
+		},
+		{
+			admission.NewAttributesRecord(&api.ReplicationController{}, nil, podKind, "test", "", podRes, "", admission.Create, nil),
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		err := ir.Admit(test.attrs)
+		if err != nil && test.expectError == false {
+			t.Error(err)
+		} else if err == nil && test.expectError == true {
+			t.Error("Error expected for Admit but received none")
+		}
+	}
+}
+
 func performTest(t *testing.T, ir admission.Interface) {
 	pods := getPods()
 	admit(t, ir, pods)
+	testAdminScenarios(t, ir, pods[0])
 
 	verifyPod(t, pods[0], 100, 100)
 	verifyPod(t, pods[1], 100, 300)
@@ -133,6 +167,14 @@ func performTest(t *testing.T, ir admission.Interface) {
 	expectNoAnnotation(t, pods[3])
 }
 
+func TestEstimateReturnsErrorFromSource(t *testing.T) {
+	f := func(_ api.ResourceName, _ int64, _, ns string, exactMatch bool, start, end time.Time) (int64, int64, error) {
+		return 0, 0, errors.New("Example error")
+	}
+	ir := newInitialResources(&fakeSource{f: f}, 90, false)
+	admit(t, ir, getPods())
+}
+
 func TestEstimationBasedOnTheSameImageSameNamespace7d(t *testing.T) {
 	f := func(_ api.ResourceName, _ int64, _, ns string, exactMatch bool, start, end time.Time) (int64, int64, error) {
 		if exactMatch && end.Sub(start) == week && ns == "test-ns" {
@@ -141,7 +183,6 @@ func TestEstimationBasedOnTheSameImageSameNamespace7d(t *testing.T) {
 		return 200, 120, nil
 	}
 	performTest(t, newInitialResources(&fakeSource{f: f}, 90, false))
-
 }
 
 func TestEstimationBasedOnTheSameImageSameNamespace30d(t *testing.T) {

plugin/pkg/admission/initialresources/data_source_test.go

@@ -0,0 +1,45 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package initialresources
+
+import "testing"
+
+func TestInfluxDBDataSource(t *testing.T) {
+	ds, _ := newDataSource("influxdb")
+	if _, ok := ds.(*influxdbSource); !ok {
+		t.Errorf("newDataSource did not return valid InfluxDB type")
+	}
+}
+
+func TestGCMDataSource(t *testing.T) {
+	// No ProjectID set
+	newDataSource("gcm")
+}
+
+func TestHawkularDataSource(t *testing.T) {
+	ds, _ := newDataSource("hawkular")
+	if _, ok := ds.(*hawkularSource); !ok {
+		t.Errorf("newDataSource did not return valid hawkularSource type")
+	}
+}
+
+func TestNoDataSourceFound(t *testing.T) {
+	ds, err := newDataSource("")
+	if ds != nil || err == nil {
+		t.Errorf("newDataSource found for empty input")
+	}
+}

plugin/pkg/admission/initialresources/gcm_test.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package initialresources
+
+import (
+	"testing"
+	"time"
+
+	"golang.org/x/oauth2"
+	"golang.org/x/oauth2/google"
+	gcm "google.golang.org/api/cloudmonitoring/v2beta2"
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func TestGCMReturnsErrorIfClientCannotConnect(t *testing.T) {
+	client := oauth2.NewClient(oauth2.NoContext, google.ComputeTokenSource(""))
+	service, _ := gcm.New(client)
+	source := &gcmSource{
+		project:    "",
+		gcmService: service,
+	}
+
+	_, _, err := source.GetUsagePercentile(api.ResourceCPU, 90, "", "", true, time.Now(), time.Now())
+	if err == nil {
+		t.Errorf("Expected error from GCM")
+	}
+
+	_, _, err = source.GetUsagePercentile(api.ResourceMemory, 90, "", "foo", false, time.Now(), time.Now())
+	if err == nil {
+		t.Errorf("Expected error from GCM")
+	}
+}

plugin/pkg/admission/initialresources/hawkular.go

@@ -95,11 +95,7 @@ func (hs *hawkularSource) GetUsagePercentile(kind api.ResourceName, perc int64,
 		m = append(m, metrics.Tenant(namespace))
 	}
 
-	p, err := metrics.ConvertToFloat64(perc)
-	if err != nil {
-		return 0, 0, err
-	}
-
+	p := float64(perc)
 	m = append(m, metrics.Filters(metrics.TagsFilter(q), metrics.BucketsFilter(1), metrics.StartTimeFilter(start), metrics.EndTimeFilter(end), metrics.PercentilesFilter([]float64{p})))
 
 	bp, err := hs.client.ReadBuckets(metrics.Counter, m...)

plugin/pkg/admission/initialresources/hawkular_test.go

@@ -67,9 +67,17 @@ func TestTaqQuery(t *testing.T) {
 	assert.Equal(t, 2, len(tQe))
 	assert.Equal(t, testImageWithReference(), tQre[containerImageTag])
 	assert.Equal(t, "cpu/usage", tQre[descriptorTag])
+
+	kind = api.ResourceMemory
+	tQ = tagQuery(kind, testImageWithReference(), true)
+	assert.Equal(t, "memory/usage", tQ[descriptorTag])
+
+	kind = api.ResourceStorage
+	tQ = tagQuery(kind, testImageWithReference(), true)
+	assert.Equal(t, "", tQ[descriptorTag])
 }
 
-func TestGetUsagePercentile(t *testing.T) {
+func newSource(t *testing.T) (map[string]string, dataSource) {
 	tenant := "16a8884e4c155457ee38a8901df6b536"
 	reqs := make(map[string]string)
 
@@ -89,11 +97,41 @@ func TestGetUsagePercentile(t *testing.T) {
 		}
 	}))
 
-	paramUri := fmt.Sprintf("%s?user=test&pass=yep", s.URL)
+	paramUri := fmt.Sprintf("%s?user=test&pass=yep&tenant=foo&insecure=true", s.URL)
 
 	hSource, err := newHawkularSource(paramUri)
 	assert.NoError(t, err)
 
+	return reqs, hSource
+}
+
+func TestInsecureMustBeBool(t *testing.T) {
+	paramUri := fmt.Sprintf("localhost?user=test&pass=yep&insecure=foo")
+	_, err := newHawkularSource(paramUri)
+	if err == nil {
+		t.Errorf("Expected error from newHawkularSource")
+	}
+}
+
+func TestCAFileMustExist(t *testing.T) {
+	paramUri := fmt.Sprintf("localhost?user=test&pass=yep&caCert=foo")
+	_, err := newHawkularSource(paramUri)
+	if err == nil {
+		t.Errorf("Expected error from newHawkularSource")
+	}
+}
+
+func TestServiceAccountIsMutuallyExclusiveWithAuth(t *testing.T) {
+	paramUri := fmt.Sprintf("localhost?user=test&pass=yep&useServiceAccount=true")
+	_, err := newHawkularSource(paramUri)
+	if err == nil {
+		t.Errorf("Expected error from newHawkularSource")
+	}
+}
+
+func TestGetUsagePercentile(t *testing.T) {
+	reqs, hSource := newSource(t)
+
 	usage, samples, err := hSource.GetUsagePercentile(api.ResourceCPU, 90, testImageWithVersion(), "16a8884e4c155457ee38a8901df6b536", true, time.Now(), time.Now())
 	assert.NoError(t, err)
 

plugin/pkg/admission/initialresources/influxdb_test.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package initialresources
+
+import (
+	"testing"
+	"time"
+
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func TestInfluxDBGetUsagePercentileCPU(t *testing.T) {
+	source, _ := newInfluxdbSource("", "", "", "")
+	_, _, err := source.GetUsagePercentile(api.ResourceCPU, 90, "", "", true, time.Now(), time.Now())
+	if err == nil {
+		t.Errorf("Expected error because InfluxDB is temporarily disabled")
+	}
+}
+
+func TestInfluxDBGetUsagePercentileMemory(t *testing.T) {
+	source, _ := newInfluxdbSource("", "", "", "")
+	_, _, err := source.GetUsagePercentile(api.ResourceMemory, 90, "", "foo", false, time.Now(), time.Now())
+	if err == nil {
+		t.Errorf("Expected error because InfluxDB is temporarily disabled")
+	}
+}