aggregator: make linter happy

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

Author: sttts

staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go

@@ -313,7 +313,7 @@ func (c *AvailableConditionController) sync(key string) error {
 						resp.Body.Close()
 						// we should always been in the 200s or 300s
 						if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
-							errCh <- fmt.Errorf("bad status from %v: %v", discoveryURL, resp.StatusCode)
+							errCh <- fmt.Errorf("bad status from %v: %d", discoveryURL, resp.StatusCode)
 							return
 						}
 					}
@@ -324,7 +324,7 @@ func (c *AvailableConditionController) sync(key string) error {
 				select {
 				case err = <-errCh:
 					if err != nil {
-						results <- fmt.Errorf("failing or missing response from %v: %v", discoveryURL, err)
+						results <- fmt.Errorf("failing or missing response from %v: %w", discoveryURL, err)
 						return
 					}
 

staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller_test.go

@@ -25,11 +25,9 @@ import (
 	"testing"
 	"time"
 
-	availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics"
-	"k8s.io/utils/pointer"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/dump"
 	v1listers "k8s.io/client-go/listers/core/v1"
 	clienttesting "k8s.io/client-go/testing"
@@ -39,11 +37,13 @@ import (
 	"k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake"
 	apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1"
 	listers "k8s.io/kube-aggregator/pkg/client/listers/apiregistration/v1"
+	availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics"
+	"k8s.io/utils/ptr"
 )
 
 const (
-	testServicePort     = 1234
-	testServicePortName = "testPort"
+	testServicePort     int32 = 1234
+	testServicePortName       = "testPort"
 )
 
 func newEndpoints(namespace, name string) *v1.Endpoints {
@@ -100,13 +100,18 @@ func newRemoteAPIService(name string) *apiregistration.APIService {
 			Service: &apiregistration.ServiceReference{
 				Namespace: "foo",
 				Name:      "bar",
-				Port:      pointer.Int32Ptr(testServicePort),
+				Port:      ptr.To(testServicePort),
 			},
 		},
 	}
 }
 
-func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableConditionController, *fake.Clientset) {
+type T interface {
+	Fatalf(format string, args ...interface{})
+	Errorf(format string, args ...interface{})
+}
+
+func setupAPIServices(t T, apiServices []runtime.Object) (*AvailableConditionController, *fake.Clientset) {
 	fakeClient := fake.NewSimpleClientset()
 	apiServiceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
 	serviceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
@@ -118,7 +123,9 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
 	defer testServer.Close()
 
 	for _, o := range apiServices {
-		apiServiceIndexer.Add(o)
+		if err := apiServiceIndexer.Add(o); err != nil {
+			t.Fatalf("failed to add APIService: %v", err)
+		}
 	}
 
 	c := AvailableConditionController{
@@ -145,7 +152,7 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
 func BenchmarkBuildCache(b *testing.B) {
 	apiServiceName := "remote.group"
 	// model 1 APIService pointing at a given service, and 30 pointing at local group/versions
-	apiServices := []*apiregistration.APIService{newRemoteAPIService(apiServiceName)}
+	apiServices := []runtime.Object{newRemoteAPIService(apiServiceName)}
 	for i := 0; i < 30; i++ {
 		apiServices = append(apiServices, newLocalAPIService(fmt.Sprintf("local.group%d", i)))
 	}
@@ -154,7 +161,7 @@ func BenchmarkBuildCache(b *testing.B) {
 	for i := 0; i < 100; i++ {
 		services = append(services, newService("foo", fmt.Sprintf("bar%d", i), testServicePort, testServicePortName))
 	}
-	c, _ := setupAPIServices(apiServices)
+	c, _ := setupAPIServices(b, apiServices)
 	b.ReportAllocs()
 	b.ResetTimer()
 	for n := 1; n <= b.N; n++ {
@@ -175,7 +182,7 @@ func TestBuildCache(t *testing.T) {
 		name string
 
 		apiServiceName string
-		apiServices    []*apiregistration.APIService
+		apiServices    []runtime.Object
 		services       []*v1.Service
 		endpoints      []*v1.Endpoints
 
@@ -184,13 +191,13 @@ func TestBuildCache(t *testing.T) {
 		{
 			name:           "api service",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			c, fakeClient := setupAPIServices(tc.apiServices)
+			c, fakeClient := setupAPIServices(t, tc.apiServices)
 			for _, svc := range tc.services {
 				c.addService(svc)
 			}
@@ -210,18 +217,19 @@ func TestSync(t *testing.T) {
 		name string
 
 		apiServiceName  string
-		apiServices     []*apiregistration.APIService
+		apiServices     []runtime.Object
 		services        []*v1.Service
 		endpoints       []*v1.Endpoints
 		backendStatus   int
 		backendLocation string
 
 		expectedAvailability apiregistration.APIServiceCondition
+		expectedSyncError    string
 	}{
 		{
 			name:           "local",
 			apiServiceName: "local.group",
-			apiServices:    []*apiregistration.APIService{newLocalAPIService("local.group")},
+			apiServices:    []runtime.Object{newLocalAPIService("local.group")},
 			backendStatus:  http.StatusOK,
 			expectedAvailability: apiregistration.APIServiceCondition{
 				Type:    apiregistration.Available,
@@ -233,7 +241,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "no service",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "not-bar", testServicePort, testServicePortName)},
 			backendStatus:  http.StatusOK,
 			expectedAvailability: apiregistration.APIServiceCondition{
@@ -246,7 +254,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "service on bad port",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services: []*v1.Service{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
 				Spec: v1.ServiceSpec{
@@ -268,7 +276,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "no endpoints",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			backendStatus:  http.StatusOK,
 			expectedAvailability: apiregistration.APIServiceCondition{
@@ -281,7 +289,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "missing endpoints",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:      []*v1.Endpoints{newEndpoints("foo", "bar")},
 			backendStatus:  http.StatusOK,
@@ -295,7 +303,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "wrong endpoint port name",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:      []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, "wrongName")},
 			backendStatus:  http.StatusOK,
@@ -309,7 +317,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "remote",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:      []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
 			backendStatus:  http.StatusOK,
@@ -323,7 +331,7 @@ func TestSync(t *testing.T) {
 		{
 			name:           "remote-bad-return",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:      []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
 			backendStatus:  http.StatusForbidden,
@@ -333,11 +341,12 @@ func TestSync(t *testing.T) {
 				Reason:  "FailedDiscoveryCheck",
 				Message: `failing or missing response from`,
 			},
+			expectedSyncError: "failing or missing response from",
 		},
 		{
 			name:            "remote-redirect",
 			apiServiceName:  "remote.group",
-			apiServices:     []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:     []runtime.Object{newRemoteAPIService("remote.group")},
 			services:        []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:       []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
 			backendStatus:   http.StatusFound,
@@ -348,11 +357,12 @@ func TestSync(t *testing.T) {
 				Reason:  "FailedDiscoveryCheck",
 				Message: `failing or missing response from`,
 			},
+			expectedSyncError: "failing or missing response from",
 		},
 		{
 			name:           "remote-304",
 			apiServiceName: "remote.group",
-			apiServices:    []*apiregistration.APIService{newRemoteAPIService("remote.group")},
+			apiServices:    []runtime.Object{newRemoteAPIService("remote.group")},
 			services:       []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
 			endpoints:      []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
 			backendStatus:  http.StatusNotModified,
@@ -362,12 +372,13 @@ func TestSync(t *testing.T) {
 				Reason:  "FailedDiscoveryCheck",
 				Message: `failing or missing response from`,
 			},
+			expectedSyncError: "failing or missing response from",
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			fakeClient := fake.NewSimpleClientset()
+			fakeClient := fake.NewSimpleClientset(tc.apiServices...)
 			apiServiceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
 			serviceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
 			endpointsIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
@@ -398,7 +409,16 @@ func TestSync(t *testing.T) {
 				proxyCurrentCertKeyContent: func() ([]byte, []byte) { return emptyCert(), emptyCert() },
 				metrics:                    availabilitymetrics.New(),
 			}
-			c.sync(tc.apiServiceName)
+			err := c.sync(tc.apiServiceName)
+			if tc.expectedSyncError != "" {
+				if err == nil {
+					t.Fatalf("%v expected error with %q, got none", tc.name, tc.expectedSyncError)
+				} else if !strings.Contains(err.Error(), tc.expectedSyncError) {
+					t.Fatalf("%v expected error with %q, got %q", tc.name, tc.expectedSyncError, err.Error())
+				}
+			} else if err != nil {
+				t.Fatalf("%v unexpected sync error: %v", tc.name, err)
+			}
 
 			// ought to have one action writing status
 			if e, a := 1, len(fakeClient.Actions()); e != a {
@@ -445,19 +465,23 @@ func TestUpdateAPIServiceStatus(t *testing.T) {
 	foo := &apiregistration.APIService{Status: apiregistration.APIServiceStatus{Conditions: []apiregistration.APIServiceCondition{{Type: "foo"}}}}
 	bar := &apiregistration.APIService{Status: apiregistration.APIServiceStatus{Conditions: []apiregistration.APIServiceCondition{{Type: "bar"}}}}
 
-	fakeClient := fake.NewSimpleClientset()
+	fakeClient := fake.NewSimpleClientset(foo)
 	c := AvailableConditionController{
 		apiServiceClient: fakeClient.ApiregistrationV1().(apiregistrationclient.APIServicesGetter),
 		metrics:          availabilitymetrics.New(),
 	}
 
-	c.updateAPIServiceStatus(foo, foo)
+	if _, err := c.updateAPIServiceStatus(foo, foo); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
 	if e, a := 0, len(fakeClient.Actions()); e != a {
 		t.Error(dump.Pretty(fakeClient.Actions()))
 	}
 
 	fakeClient.ClearActions()
-	c.updateAPIServiceStatus(foo, bar)
+	if _, err := c.updateAPIServiceStatus(foo, bar); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
 	if e, a := 1, len(fakeClient.Actions()); e != a {
 		t.Error(dump.Pretty(fakeClient.Actions()))
 	}