Skip to content

Commit

Permalink
Merge pull request #71273 from deads2k/agg-02-detect
Browse files Browse the repository at this point in the history 
only update the apiservice status if the status has changed
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Dec 1, 2018
2 parents 4855b78 + 541f036 commit 4b9b7ff
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 11 deletions.
2 changes: 2 additions & 0 deletions staging/src/k8s.io/kube-aggregator/pkg/controllers/status/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
],
)

Expand Down
32 changes: 21 additions & 11 deletions staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -140,15 +141,15 @@ 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
}
if err != nil {
return err
}

apiService := inAPIService.DeepCopy()
apiService := originalAPIService.DeepCopy()

availableCondition := apiregistration.APIServiceCondition{
Type: apiregistration.Available,
Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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()
Expand Down
20 changes: 20 additions & 0 deletions staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()))
}

}

0 comments on commit 4b9b7ff

Please sign in to comment.