Skip to content

Commit

Permalink
aggregator: make linter happy
Browse files Browse the repository at this point in the history 
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

Kubernetes-commit: bbdc247406aa21d16644828771b377c042bfdeb6
  • Loading branch information
@sttts @k8s-publishing-bot
sttts authored and k8s-publishing-bot committed Jul 21, 2024
1 parent 34c473f commit 006e6b9
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 31 deletions.
4 changes: 2 additions & 2 deletions pkg/controllers/status/remote/remote_available_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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
}

Expand Down
82 changes: 53 additions & 29 deletions pkg/controllers/status/remote/remote_available_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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})
Expand All @@ -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{
Expand All @@ -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)))
}
Expand All @@ -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++ {
Expand All @@ -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

Expand All @@ -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)
}
Expand All @@ -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,
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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})
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()))
}
Expand Down

0 comments on commit 006e6b9

Please sign in to comment.