From 541f036a4960003c8443d99ec805e3e7baebad19 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 20 Nov 2018 11:57:25 -0500 Subject: [PATCH] only update the apiservice status if the status has changed --- .../pkg/controllers/status/BUILD | 2 ++ .../status/available_controller.go | 32 ++++++++++++------- .../status/available_controller_test.go | 20 ++++++++++++ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/BUILD b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/BUILD index 3810051bc5..f644a13265 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/BUILD +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/BUILD @@ -13,6 +13,7 @@ go_library( importpath = "k8s.io/kube-aggregator/pkg/controllers/status", deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", @@ -46,6 +47,7 @@ go_test( "//staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration:go_default_library", "//staging/src/k8s.io/kube-aggregator/pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//staging/src/k8s.io/kube-aggregator/pkg/client/listers/apiregistration/internalversion:go_default_library", + "//vendor/github.com/davecgh/go-spew/spew:go_default_library", ], ) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go index 66a4d0f2f7..0018492c79 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go @@ -26,6 +26,7 @@ import ( "k8s.io/klog" "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -140,7 +141,7 @@ func NewAvailableConditionController( } func (c *AvailableConditionController) sync(key string) error { - inAPIService, err := c.apiServiceLister.Get(key) + originalAPIService, err := c.apiServiceLister.Get(key) if apierrors.IsNotFound(err) { return nil } @@ -148,7 +149,7 @@ func (c *AvailableConditionController) sync(key string) error { return err } - apiService := inAPIService.DeepCopy() + apiService := originalAPIService.DeepCopy() availableCondition := apiregistration.APIServiceCondition{ Type: apiregistration.Available, @@ -159,7 +160,7 @@ func (c *AvailableConditionController) sync(key string) error { // local API services are always considered available if apiService.Spec.Service == nil { apiregistration.SetAPIServiceCondition(apiService, apiregistration.NewLocalAvailableAPIServiceCondition()) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } @@ -169,14 +170,14 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "ServiceNotFound" availableCondition.Message = fmt.Sprintf("service/%s in %q is not present", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } else if err != nil { availableCondition.Status = apiregistration.ConditionUnknown availableCondition.Reason = "ServiceAccessError" availableCondition.Message = fmt.Sprintf("service/%s in %q cannot be checked due to: %v", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace, err) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } @@ -193,7 +194,7 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "ServicePortError" availableCondition.Message = fmt.Sprintf("service/%s in %q is not listening on port 443", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } @@ -203,14 +204,14 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "EndpointsNotFound" availableCondition.Message = fmt.Sprintf("cannot find endpoints for service/%s in %q", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } else if err != nil { availableCondition.Status = apiregistration.ConditionUnknown availableCondition.Reason = "EndpointsAccessError" availableCondition.Message = fmt.Sprintf("service/%s in %q cannot be checked due to: %v", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace, err) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } hasActiveEndpoints := false @@ -225,7 +226,7 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "MissingEndpoints" availableCondition.Message = fmt.Sprintf("endpoints for service/%s in %q have no addresses", apiService.Spec.Service.Name, apiService.Spec.Service.Namespace) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } } @@ -259,7 +260,7 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "FailedDiscoveryCheck" availableCondition.Message = fmt.Sprintf("no response from %v: %v", discoveryURL, err) apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, updateErr := c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, updateErr := updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) if updateErr != nil { return updateErr } @@ -272,10 +273,19 @@ func (c *AvailableConditionController) sync(key string) error { availableCondition.Reason = "Passed" availableCondition.Message = "all checks passed" apiregistration.SetAPIServiceCondition(apiService, availableCondition) - _, err = c.apiServiceClient.APIServices().UpdateStatus(apiService) + _, err = updateAPIServiceStatus(c.apiServiceClient, originalAPIService, apiService) return err } +// updateAPIServiceStatus only issues an update if a change is detected. We have a tight resync loop to quickly detect dead +// apiservices. Doing that means we don't want to quickly issue no-op updates. +func updateAPIServiceStatus(client apiregistrationclient.APIServicesGetter, originalAPIService, newAPIService *apiregistration.APIService) (*apiregistration.APIService, error) { + if equality.Semantic.DeepEqual(originalAPIService.Status, newAPIService.Status) { + return newAPIService, nil + } + return client.APIServices().UpdateStatus(newAPIService) +} + func (c *AvailableConditionController) Run(threadiness int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer c.queue.ShutDown() diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go index d82a0a0bd3..895cc9a74b 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go @@ -19,6 +19,8 @@ package apiserver import ( "testing" + "github.com/davecgh/go-spew/spew" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1listers "k8s.io/client-go/listers/core/v1" @@ -228,3 +230,21 @@ func TestSync(t *testing.T) { } } } + +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() + updateAPIServiceStatus(fakeClient.Apiregistration(), foo, foo) + if e, a := 0, len(fakeClient.Actions()); e != a { + t.Error(spew.Sdump(fakeClient.Actions())) + } + + fakeClient.ClearActions() + updateAPIServiceStatus(fakeClient.Apiregistration(), foo, bar) + if e, a := 1, len(fakeClient.Actions()); e != a { + t.Error(spew.Sdump(fakeClient.Actions())) + } + +}