Skip to content

Commit 006e6b9

Browse files
stttsk8s-publishing-bot
authored andcommitted
aggregator: make linter happy
Signed-off-by: Dr. Stefan Schimanski <[email protected]> Kubernetes-commit: bbdc247406aa21d16644828771b377c042bfdeb6
1 parent 34c473f commit 006e6b9

File tree

2 files changed

+55
-31
lines changed

2 files changed

+55
-31
lines changed

pkg/controllers/status/remote/remote_available_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func (c *AvailableConditionController) sync(key string) error {
313313
resp.Body.Close()
314314
// we should always been in the 200s or 300s
315315
if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
316-
errCh <- fmt.Errorf("bad status from %v: %v", discoveryURL, resp.StatusCode)
316+
errCh <- fmt.Errorf("bad status from %v: %d", discoveryURL, resp.StatusCode)
317317
return
318318
}
319319
}
@@ -324,7 +324,7 @@ func (c *AvailableConditionController) sync(key string) error {
324324
select {
325325
case err = <-errCh:
326326
if err != nil {
327-
results <- fmt.Errorf("failing or missing response from %v: %v", discoveryURL, err)
327+
results <- fmt.Errorf("failing or missing response from %v: %w", discoveryURL, err)
328328
return
329329
}
330330

pkg/controllers/status/remote/remote_available_controller_test.go

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@ import (
2525
"testing"
2626
"time"
2727

28-
availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics"
29-
"k8s.io/utils/pointer"
30-
3128
v1 "k8s.io/api/core/v1"
3229
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/runtime"
3331
"k8s.io/apimachinery/pkg/util/dump"
3432
v1listers "k8s.io/client-go/listers/core/v1"
3533
clienttesting "k8s.io/client-go/testing"
@@ -39,11 +37,13 @@ import (
3937
"k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake"
4038
apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1"
4139
listers "k8s.io/kube-aggregator/pkg/client/listers/apiregistration/v1"
40+
availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics"
41+
"k8s.io/utils/ptr"
4242
)
4343

4444
const (
45-
testServicePort = 1234
46-
testServicePortName = "testPort"
45+
testServicePort int32 = 1234
46+
testServicePortName = "testPort"
4747
)
4848

4949
func newEndpoints(namespace, name string) *v1.Endpoints {
@@ -100,13 +100,18 @@ func newRemoteAPIService(name string) *apiregistration.APIService {
100100
Service: &apiregistration.ServiceReference{
101101
Namespace: "foo",
102102
Name: "bar",
103-
Port: pointer.Int32Ptr(testServicePort),
103+
Port: ptr.To(testServicePort),
104104
},
105105
},
106106
}
107107
}
108108

109-
func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableConditionController, *fake.Clientset) {
109+
type T interface {
110+
Fatalf(format string, args ...interface{})
111+
Errorf(format string, args ...interface{})
112+
}
113+
114+
func setupAPIServices(t T, apiServices []runtime.Object) (*AvailableConditionController, *fake.Clientset) {
110115
fakeClient := fake.NewSimpleClientset()
111116
apiServiceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
112117
serviceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
@@ -118,7 +123,9 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
118123
defer testServer.Close()
119124

120125
for _, o := range apiServices {
121-
apiServiceIndexer.Add(o)
126+
if err := apiServiceIndexer.Add(o); err != nil {
127+
t.Fatalf("failed to add APIService: %v", err)
128+
}
122129
}
123130

124131
c := AvailableConditionController{
@@ -145,7 +152,7 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
145152
func BenchmarkBuildCache(b *testing.B) {
146153
apiServiceName := "remote.group"
147154
// model 1 APIService pointing at a given service, and 30 pointing at local group/versions
148-
apiServices := []*apiregistration.APIService{newRemoteAPIService(apiServiceName)}
155+
apiServices := []runtime.Object{newRemoteAPIService(apiServiceName)}
149156
for i := 0; i < 30; i++ {
150157
apiServices = append(apiServices, newLocalAPIService(fmt.Sprintf("local.group%d", i)))
151158
}
@@ -154,7 +161,7 @@ func BenchmarkBuildCache(b *testing.B) {
154161
for i := 0; i < 100; i++ {
155162
services = append(services, newService("foo", fmt.Sprintf("bar%d", i), testServicePort, testServicePortName))
156163
}
157-
c, _ := setupAPIServices(apiServices)
164+
c, _ := setupAPIServices(b, apiServices)
158165
b.ReportAllocs()
159166
b.ResetTimer()
160167
for n := 1; n <= b.N; n++ {
@@ -175,7 +182,7 @@ func TestBuildCache(t *testing.T) {
175182
name string
176183

177184
apiServiceName string
178-
apiServices []*apiregistration.APIService
185+
apiServices []runtime.Object
179186
services []*v1.Service
180187
endpoints []*v1.Endpoints
181188

@@ -184,13 +191,13 @@ func TestBuildCache(t *testing.T) {
184191
{
185192
name: "api service",
186193
apiServiceName: "remote.group",
187-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
194+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
188195
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
189196
},
190197
}
191198
for _, tc := range tests {
192199
t.Run(tc.name, func(t *testing.T) {
193-
c, fakeClient := setupAPIServices(tc.apiServices)
200+
c, fakeClient := setupAPIServices(t, tc.apiServices)
194201
for _, svc := range tc.services {
195202
c.addService(svc)
196203
}
@@ -210,18 +217,19 @@ func TestSync(t *testing.T) {
210217
name string
211218

212219
apiServiceName string
213-
apiServices []*apiregistration.APIService
220+
apiServices []runtime.Object
214221
services []*v1.Service
215222
endpoints []*v1.Endpoints
216223
backendStatus int
217224
backendLocation string
218225

219226
expectedAvailability apiregistration.APIServiceCondition
227+
expectedSyncError string
220228
}{
221229
{
222230
name: "local",
223231
apiServiceName: "local.group",
224-
apiServices: []*apiregistration.APIService{newLocalAPIService("local.group")},
232+
apiServices: []runtime.Object{newLocalAPIService("local.group")},
225233
backendStatus: http.StatusOK,
226234
expectedAvailability: apiregistration.APIServiceCondition{
227235
Type: apiregistration.Available,
@@ -233,7 +241,7 @@ func TestSync(t *testing.T) {
233241
{
234242
name: "no service",
235243
apiServiceName: "remote.group",
236-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
244+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
237245
services: []*v1.Service{newService("foo", "not-bar", testServicePort, testServicePortName)},
238246
backendStatus: http.StatusOK,
239247
expectedAvailability: apiregistration.APIServiceCondition{
@@ -246,7 +254,7 @@ func TestSync(t *testing.T) {
246254
{
247255
name: "service on bad port",
248256
apiServiceName: "remote.group",
249-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
257+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
250258
services: []*v1.Service{{
251259
ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
252260
Spec: v1.ServiceSpec{
@@ -268,7 +276,7 @@ func TestSync(t *testing.T) {
268276
{
269277
name: "no endpoints",
270278
apiServiceName: "remote.group",
271-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
279+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
272280
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
273281
backendStatus: http.StatusOK,
274282
expectedAvailability: apiregistration.APIServiceCondition{
@@ -281,7 +289,7 @@ func TestSync(t *testing.T) {
281289
{
282290
name: "missing endpoints",
283291
apiServiceName: "remote.group",
284-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
292+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
285293
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
286294
endpoints: []*v1.Endpoints{newEndpoints("foo", "bar")},
287295
backendStatus: http.StatusOK,
@@ -295,7 +303,7 @@ func TestSync(t *testing.T) {
295303
{
296304
name: "wrong endpoint port name",
297305
apiServiceName: "remote.group",
298-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
306+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
299307
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
300308
endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, "wrongName")},
301309
backendStatus: http.StatusOK,
@@ -309,7 +317,7 @@ func TestSync(t *testing.T) {
309317
{
310318
name: "remote",
311319
apiServiceName: "remote.group",
312-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
320+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
313321
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
314322
endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
315323
backendStatus: http.StatusOK,
@@ -323,7 +331,7 @@ func TestSync(t *testing.T) {
323331
{
324332
name: "remote-bad-return",
325333
apiServiceName: "remote.group",
326-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
334+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
327335
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
328336
endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
329337
backendStatus: http.StatusForbidden,
@@ -333,11 +341,12 @@ func TestSync(t *testing.T) {
333341
Reason: "FailedDiscoveryCheck",
334342
Message: `failing or missing response from`,
335343
},
344+
expectedSyncError: "failing or missing response from",
336345
},
337346
{
338347
name: "remote-redirect",
339348
apiServiceName: "remote.group",
340-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
349+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
341350
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
342351
endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
343352
backendStatus: http.StatusFound,
@@ -348,11 +357,12 @@ func TestSync(t *testing.T) {
348357
Reason: "FailedDiscoveryCheck",
349358
Message: `failing or missing response from`,
350359
},
360+
expectedSyncError: "failing or missing response from",
351361
},
352362
{
353363
name: "remote-304",
354364
apiServiceName: "remote.group",
355-
apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")},
365+
apiServices: []runtime.Object{newRemoteAPIService("remote.group")},
356366
services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)},
357367
endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)},
358368
backendStatus: http.StatusNotModified,
@@ -362,12 +372,13 @@ func TestSync(t *testing.T) {
362372
Reason: "FailedDiscoveryCheck",
363373
Message: `failing or missing response from`,
364374
},
375+
expectedSyncError: "failing or missing response from",
365376
},
366377
}
367378

368379
for _, tc := range tests {
369380
t.Run(tc.name, func(t *testing.T) {
370-
fakeClient := fake.NewSimpleClientset()
381+
fakeClient := fake.NewSimpleClientset(tc.apiServices...)
371382
apiServiceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
372383
serviceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
373384
endpointsIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
@@ -398,7 +409,16 @@ func TestSync(t *testing.T) {
398409
proxyCurrentCertKeyContent: func() ([]byte, []byte) { return emptyCert(), emptyCert() },
399410
metrics: availabilitymetrics.New(),
400411
}
401-
c.sync(tc.apiServiceName)
412+
err := c.sync(tc.apiServiceName)
413+
if tc.expectedSyncError != "" {
414+
if err == nil {
415+
t.Fatalf("%v expected error with %q, got none", tc.name, tc.expectedSyncError)
416+
} else if !strings.Contains(err.Error(), tc.expectedSyncError) {
417+
t.Fatalf("%v expected error with %q, got %q", tc.name, tc.expectedSyncError, err.Error())
418+
}
419+
} else if err != nil {
420+
t.Fatalf("%v unexpected sync error: %v", tc.name, err)
421+
}
402422

403423
// ought to have one action writing status
404424
if e, a := 1, len(fakeClient.Actions()); e != a {
@@ -445,19 +465,23 @@ func TestUpdateAPIServiceStatus(t *testing.T) {
445465
foo := &apiregistration.APIService{Status: apiregistration.APIServiceStatus{Conditions: []apiregistration.APIServiceCondition{{Type: "foo"}}}}
446466
bar := &apiregistration.APIService{Status: apiregistration.APIServiceStatus{Conditions: []apiregistration.APIServiceCondition{{Type: "bar"}}}}
447467

448-
fakeClient := fake.NewSimpleClientset()
468+
fakeClient := fake.NewSimpleClientset(foo)
449469
c := AvailableConditionController{
450470
apiServiceClient: fakeClient.ApiregistrationV1().(apiregistrationclient.APIServicesGetter),
451471
metrics: availabilitymetrics.New(),
452472
}
453473

454-
c.updateAPIServiceStatus(foo, foo)
474+
if _, err := c.updateAPIServiceStatus(foo, foo); err != nil {
475+
t.Fatalf("unexpected error: %v", err)
476+
}
455477
if e, a := 0, len(fakeClient.Actions()); e != a {
456478
t.Error(dump.Pretty(fakeClient.Actions()))
457479
}
458480

459481
fakeClient.ClearActions()
460-
c.updateAPIServiceStatus(foo, bar)
482+
if _, err := c.updateAPIServiceStatus(foo, bar); err != nil {
483+
t.Fatalf("unexpected error: %v", err)
484+
}
461485
if e, a := 1, len(fakeClient.Actions()); e != a {
462486
t.Error(dump.Pretty(fakeClient.Actions()))
463487
}

0 commit comments

Comments
 (0)