From 87f8e9e199e8be727cf09f238e256ea59d8c7b23 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 17 Jul 2024 10:50:28 +0200 Subject: [PATCH] aggregator: split availability controller into local and remote part Signed-off-by: Dr. Stefan Schimanski Kubernetes-commit: 834cd7ca4a1c08b5d32d5e2da377310764f2c11c --- pkg/apiserver/apiserver.go | 54 +++-- .../local/local_available_controller.go | 227 ++++++++++++++++++ .../local/local_available_controller_test.go | 168 +++++++++++++ .../remote/remote_available_controller.go | 28 +-- .../remote_available_controller_test.go | 9 + 5 files changed, 451 insertions(+), 35 deletions(-) create mode 100644 pkg/controllers/status/local/local_available_controller.go create mode 100644 pkg/controllers/status/local/local_available_controller_test.go diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 82d67bf8..ac91fcf6 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -51,8 +51,9 @@ import ( openapiaggregator "k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator" openapiv3controller "k8s.io/kube-aggregator/pkg/controllers/openapiv3" openapiv3aggregator "k8s.io/kube-aggregator/pkg/controllers/openapiv3/aggregator" + localavailability "k8s.io/kube-aggregator/pkg/controllers/status/local" availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics" - statuscontrollers "k8s.io/kube-aggregator/pkg/controllers/status/remote" + remoteavailability "k8s.io/kube-aggregator/pkg/controllers/status/remote" apiservicerest "k8s.io/kube-aggregator/pkg/registry/apiservice/rest" openapicommon "k8s.io/kube-openapi/pkg/common" ) @@ -102,12 +103,11 @@ type ExtraConfig struct { RejectForwardingRedirects bool - // DisableAvailableConditionController disables the controller that updates the Available conditions for - // APIServices, Endpoints and Services. This controller runs in kube-aggregator and can interfere with - // Generic Control Plane components when certain apis are not available. - // TODO: We should find a better way to handle this. For now it will be for Generic Control Plane authors to - // disable this controller if they see issues. - DisableAvailableConditionController bool + // DisableRemoteAvailableConditionController disables the controller that updates the Available conditions for + // remote APIServices via querying endpoints of the referenced services. In generic controlplane use-cases, + // the concept of services and endpoints might differ, and might require another implementation of this + // controller. Local APIService are reconciled nevertheless. + DisableRemoteAvailableConditionController bool } // Config represents the configuration needed to create an APIAggregator. @@ -320,6 +320,12 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg }) } + s.GenericAPIServer.AddPostStartHookOrDie("start-kube-aggregator-informers", func(context genericapiserver.PostStartHookContext) error { + informerFactory.Start(context.Done()) + c.GenericConfig.SharedInformerFactory.Start(context.Done()) + return nil + }) + // create shared (remote and local) availability metrics // TODO: decouple from legacyregistry metrics := availabilitymetrics.New() @@ -328,10 +334,25 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg return nil, err } - // If the AvailableConditionController is disabled, we don't need to start the informers - // and the controller. - if !c.ExtraConfig.DisableAvailableConditionController { - availableController, err := statuscontrollers.NewAvailableConditionController( + // always run local availability controller + local, err := localavailability.New( + informerFactory.Apiregistration().V1().APIServices(), + apiregistrationClient.ApiregistrationV1(), + metrics, + ) + if err != nil { + return nil, err + } + s.GenericAPIServer.AddPostStartHookOrDie("apiservice-status-local-available-controller", func(context genericapiserver.PostStartHookContext) error { + // if we end up blocking for long periods of time, we may need to increase workers. + go local.Run(5, context.Done()) + return nil + }) + + // conditionally run remote availability controller. This could be replaced in certain + // generic controlplane use-cases where there is another concept of services and/or endpoints. + if !c.ExtraConfig.DisableRemoteAvailableConditionController { + remote, err := remoteavailability.New( informerFactory.Apiregistration().V1().APIServices(), c.GenericConfig.SharedInformerFactory.Core().V1().Services(), c.GenericConfig.SharedInformerFactory.Core().V1().Endpoints(), @@ -344,16 +365,9 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg if err != nil { return nil, err } - - s.GenericAPIServer.AddPostStartHookOrDie("start-kube-aggregator-informers", func(context genericapiserver.PostStartHookContext) error { - informerFactory.Start(context.Done()) - c.GenericConfig.SharedInformerFactory.Start(context.Done()) - return nil - }) - - s.GenericAPIServer.AddPostStartHookOrDie("apiservice-status-available-controller", func(context genericapiserver.PostStartHookContext) error { + s.GenericAPIServer.AddPostStartHookOrDie("apiservice-status-remote-available-controller", func(context genericapiserver.PostStartHookContext) error { // if we end up blocking for long periods of time, we may need to increase workers. - go availableController.Run(5, context.Done()) + go remote.Run(5, context.Done()) return nil }) } diff --git a/pkg/controllers/status/local/local_available_controller.go b/pkg/controllers/status/local/local_available_controller.go new file mode 100644 index 00000000..982ff69f --- /dev/null +++ b/pkg/controllers/status/local/local_available_controller.go @@ -0,0 +1,227 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package external + +import ( + "context" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog/v2" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + apiregistrationv1apihelper "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/helper" + apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1" + informers "k8s.io/kube-aggregator/pkg/client/informers/externalversions/apiregistration/v1" + listers "k8s.io/kube-aggregator/pkg/client/listers/apiregistration/v1" + "k8s.io/kube-aggregator/pkg/controllers" + availabilitymetrics "k8s.io/kube-aggregator/pkg/controllers/status/metrics" +) + +// AvailableConditionController handles checking the availability of registered local API services. +type AvailableConditionController struct { + apiServiceClient apiregistrationclient.APIServicesGetter + + apiServiceLister listers.APIServiceLister + apiServiceSynced cache.InformerSynced + + // To allow injection for testing. + syncFn func(key string) error + + queue workqueue.TypedRateLimitingInterface[string] + + // metrics registered into legacy registry + metrics *availabilitymetrics.Metrics +} + +// New returns a new local availability AvailableConditionController. +func New( + apiServiceInformer informers.APIServiceInformer, + apiServiceClient apiregistrationclient.APIServicesGetter, + metrics *availabilitymetrics.Metrics, +) (*AvailableConditionController, error) { + c := &AvailableConditionController{ + apiServiceClient: apiServiceClient, + apiServiceLister: apiServiceInformer.Lister(), + queue: workqueue.NewTypedRateLimitingQueueWithConfig( + // We want a fairly tight requeue time. The controller listens to the API, but because it relies on the routability of the + // service network, it is possible for an external, non-watchable factor to affect availability. This keeps + // the maximum disruption time to a minimum, but it does prevent hot loops. + workqueue.NewTypedItemExponentialFailureRateLimiter[string](5*time.Millisecond, 30*time.Second), + workqueue.TypedRateLimitingQueueConfig[string]{Name: "LocalAvailabilityController"}, + ), + metrics: metrics, + } + + // resync on this one because it is low cardinality and rechecking the actual discovery + // allows us to detect health in a more timely fashion when network connectivity to + // nodes is snipped, but the network still attempts to route there. See + // https://github.com/openshift/origin/issues/17159#issuecomment-341798063 + apiServiceHandler, _ := apiServiceInformer.Informer().AddEventHandlerWithResyncPeriod( + cache.ResourceEventHandlerFuncs{ + AddFunc: c.addAPIService, + UpdateFunc: c.updateAPIService, + DeleteFunc: c.deleteAPIService, + }, + 30*time.Second) + c.apiServiceSynced = apiServiceHandler.HasSynced + + c.syncFn = c.sync + + return c, nil +} + +func (c *AvailableConditionController) sync(key string) error { + originalAPIService, err := c.apiServiceLister.Get(key) + if apierrors.IsNotFound(err) { + c.metrics.ForgetAPIService(key) + return nil + } + if err != nil { + return err + } + + if originalAPIService.Spec.Service != nil { + // this controller only handles local APIServices + return nil + } + + // local API services are always considered available + apiService := originalAPIService.DeepCopy() + apiregistrationv1apihelper.SetAPIServiceCondition(apiService, apiregistrationv1apihelper.NewLocalAvailableAPIServiceCondition()) + _, err = c.updateAPIServiceStatus(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 (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService, newAPIService *apiregistrationv1.APIService) (*apiregistrationv1.APIService, error) { + // update this metric on every sync operation to reflect the actual state + c.metrics.SetUnavailableGauge(newAPIService) + + if equality.Semantic.DeepEqual(originalAPIService.Status, newAPIService.Status) { + return newAPIService, nil + } + + orig := apiregistrationv1apihelper.GetAPIServiceConditionByType(originalAPIService, apiregistrationv1.Available) + now := apiregistrationv1apihelper.GetAPIServiceConditionByType(newAPIService, apiregistrationv1.Available) + unknown := apiregistrationv1.APIServiceCondition{ + Type: apiregistrationv1.Available, + Status: apiregistrationv1.ConditionUnknown, + } + if orig == nil { + orig = &unknown + } + if now == nil { + now = &unknown + } + if *orig != *now { + klog.V(2).InfoS("changing APIService availability", "name", newAPIService.Name, "oldStatus", orig.Status, "newStatus", now.Status, "message", now.Message, "reason", now.Reason) + } + + newAPIService, err := c.apiServiceClient.APIServices().UpdateStatus(context.TODO(), newAPIService, metav1.UpdateOptions{}) + if err != nil { + return nil, err + } + + c.metrics.SetUnavailableCounter(originalAPIService, newAPIService) + return newAPIService, nil +} + +// Run starts the AvailableConditionController loop which manages the availability condition of API services. +func (c *AvailableConditionController) Run(workers int, stopCh <-chan struct{}) { + defer utilruntime.HandleCrash() + defer c.queue.ShutDown() + + klog.Info("Starting LocalAvailability controller") + defer klog.Info("Shutting down LocalAvailability controller") + + // This waits not just for the informers to sync, but for our handlers + // to be called; since the handlers are three different ways of + // enqueueing the same thing, waiting for this permits the queue to + // maximally de-duplicate the entries. + if !controllers.WaitForCacheSync("LocalAvailability", stopCh, c.apiServiceSynced) { + return + } + + for i := 0; i < workers; i++ { + go wait.Until(c.runWorker, time.Second, stopCh) + } + + <-stopCh +} + +func (c *AvailableConditionController) runWorker() { + for c.processNextWorkItem() { + } +} + +// processNextWorkItem deals with one key off the queue. It returns false when it's time to quit. +func (c *AvailableConditionController) processNextWorkItem() bool { + key, quit := c.queue.Get() + if quit { + return false + } + defer c.queue.Done(key) + + err := c.syncFn(key) + if err == nil { + c.queue.Forget(key) + return true + } + + utilruntime.HandleError(fmt.Errorf("%v failed with: %w", key, err)) + c.queue.AddRateLimited(key) + + return true +} + +func (c *AvailableConditionController) addAPIService(obj interface{}) { + castObj := obj.(*apiregistrationv1.APIService) + klog.V(4).Infof("Adding %s", castObj.Name) + c.queue.Add(castObj.Name) +} + +func (c *AvailableConditionController) updateAPIService(oldObj, _ interface{}) { + oldCastObj := oldObj.(*apiregistrationv1.APIService) + klog.V(4).Infof("Updating %s", oldCastObj.Name) + c.queue.Add(oldCastObj.Name) +} + +func (c *AvailableConditionController) deleteAPIService(obj interface{}) { + castObj, ok := obj.(*apiregistrationv1.APIService) + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + klog.Errorf("Couldn't get object from tombstone %#v", obj) + return + } + castObj, ok = tombstone.Obj.(*apiregistrationv1.APIService) + if !ok { + klog.Errorf("Tombstone contained object that is not expected %#v", obj) + return + } + } + klog.V(4).Infof("Deleting %q", castObj.Name) + c.queue.Add(castObj.Name) +} diff --git a/pkg/controllers/status/local/local_available_controller_test.go b/pkg/controllers/status/local/local_available_controller_test.go new file mode 100644 index 00000000..a64c1d7d --- /dev/null +++ b/pkg/controllers/status/local/local_available_controller_test.go @@ -0,0 +1,168 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package external + +import ( + "strings" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/dump" + clienttesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" + apiregistration "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + "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 int32 = 1234 +) + +func newLocalAPIService(name string) *apiregistration.APIService { + return &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + } +} + +func newRemoteAPIService(name string) *apiregistration.APIService { + return &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: apiregistration.APIServiceSpec{ + Group: strings.SplitN(name, ".", 2)[0], + Version: strings.SplitN(name, ".", 2)[1], + Service: &apiregistration.ServiceReference{ + Namespace: "foo", + Name: "bar", + Port: ptr.To(testServicePort), + }, + }, + } +} + +func TestSync(t *testing.T) { + tests := []struct { + name string + + apiServiceName string + apiServices []runtime.Object + + expectedAvailability apiregistration.APIServiceCondition + expectedAction bool + }{ + { + name: "local", + apiServiceName: "local.group", + apiServices: []runtime.Object{newLocalAPIService("local.group")}, + expectedAvailability: apiregistration.APIServiceCondition{ + Type: apiregistration.Available, + Status: apiregistration.ConditionTrue, + Reason: "Local", + Message: "Local APIServices are always available", + }, + expectedAction: true, + }, + { + name: "remote", + apiServiceName: "remote.group", + apiServices: []runtime.Object{newRemoteAPIService("remote.group")}, + expectedAction: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset(tc.apiServices...) + apiServiceIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) + for _, obj := range tc.apiServices { + if err := apiServiceIndexer.Add(obj); err != nil { + t.Fatalf("failed to add object to indexer: %v", err) + } + } + + c := AvailableConditionController{ + apiServiceClient: fakeClient.ApiregistrationV1(), + apiServiceLister: listers.NewAPIServiceLister(apiServiceIndexer), + metrics: availabilitymetrics.New(), + } + if err := c.sync(tc.apiServiceName); err != nil { + t.Fatalf("unexpect sync error: %v", err) + } + + // ought to have one action writing status + if e, a := tc.expectedAction, len(fakeClient.Actions()) == 1; e != a { + t.Fatalf("%v expected %v, got %v", tc.name, e, fakeClient.Actions()) + } + if tc.expectedAction { + action, ok := fakeClient.Actions()[0].(clienttesting.UpdateAction) + if !ok { + t.Fatalf("%v got %v", tc.name, ok) + } + + if e, a := 1, len(action.GetObject().(*apiregistration.APIService).Status.Conditions); e != a { + t.Fatalf("%v expected %v, got %v", tc.name, e, action.GetObject()) + } + condition := action.GetObject().(*apiregistration.APIService).Status.Conditions[0] + if e, a := tc.expectedAvailability.Type, condition.Type; e != a { + t.Errorf("%v expected %v, got %#v", tc.name, e, condition) + } + if e, a := tc.expectedAvailability.Status, condition.Status; e != a { + t.Errorf("%v expected %v, got %#v", tc.name, e, condition) + } + if e, a := tc.expectedAvailability.Reason, condition.Reason; e != a { + t.Errorf("%v expected %v, got %#v", tc.name, e, condition) + } + if e, a := tc.expectedAvailability.Message, condition.Message; !strings.HasPrefix(a, e) { + t.Errorf("%v expected %v, got %#v", tc.name, e, condition) + } + if condition.LastTransitionTime.IsZero() { + t.Error("expected lastTransitionTime to be non-zero") + } + } + }) + } +} + +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(foo) + c := AvailableConditionController{ + apiServiceClient: fakeClient.ApiregistrationV1().(apiregistrationclient.APIServicesGetter), + metrics: availabilitymetrics.New(), + } + + if _, err := c.updateAPIServiceStatus(foo, foo); err != nil { + t.Fatalf("unexpected updateAPIServiceStatus error: %v", err) + } + if e, a := 0, len(fakeClient.Actions()); e != a { + t.Error(dump.Pretty(fakeClient.Actions())) + } + + fakeClient.ClearActions() + if _, err := c.updateAPIServiceStatus(foo, bar); err != nil { + t.Fatalf("unexpected updateAPIServiceStatus error: %v", err) + } + if e, a := 1, len(fakeClient.Actions()); e != a { + t.Error(dump.Pretty(fakeClient.Actions())) + } +} diff --git a/pkg/controllers/status/remote/remote_available_controller.go b/pkg/controllers/status/remote/remote_available_controller.go index 146c6a51..ade74470 100644 --- a/pkg/controllers/status/remote/remote_available_controller.go +++ b/pkg/controllers/status/remote/remote_available_controller.go @@ -88,8 +88,8 @@ type AvailableConditionController struct { metrics *availabilitymetrics.Metrics } -// NewAvailableConditionController returns a new AvailableConditionController. -func NewAvailableConditionController( +// New returns a new remote APIService AvailableConditionController. +func New( apiServiceInformer informers.APIServiceInformer, serviceInformer v1informers.ServiceInformer, endpointsInformer v1informers.EndpointsInformer, @@ -110,7 +110,7 @@ func NewAvailableConditionController( // service network, it is possible for an external, non-watchable factor to affect availability. This keeps // the maximum disruption time to a minimum, but it does prevent hot loops. workqueue.NewTypedItemExponentialFailureRateLimiter[string](5*time.Millisecond, 30*time.Second), - workqueue.TypedRateLimitingQueueConfig[string]{Name: "AvailableConditionController"}, + workqueue.TypedRateLimitingQueueConfig[string]{Name: "RemoteAvailabilityController"}, ), proxyTransportDial: proxyTransportDial, proxyCurrentCertKeyContent: proxyCurrentCertKeyContent, @@ -159,6 +159,13 @@ func (c *AvailableConditionController) sync(key string) error { return err } + if originalAPIService.Spec.Service == nil { + // handled by the local APIService controller + return nil + } + + apiService := originalAPIService.DeepCopy() + // if a particular transport was specified, use that otherwise build one // construct an http client that will ignore TLS verification (if someone owns the network and messes with your status // that's not so bad) and sets a very short timeout. This is a best effort GET that provides no additional information @@ -188,21 +195,12 @@ func (c *AvailableConditionController) sync(key string) error { }, } - apiService := originalAPIService.DeepCopy() - availableCondition := apiregistrationv1.APIServiceCondition{ Type: apiregistrationv1.Available, Status: apiregistrationv1.ConditionTrue, LastTransitionTime: metav1.Now(), } - // local API services are always considered available - if apiService.Spec.Service == nil { - apiregistrationv1apihelper.SetAPIServiceCondition(apiService, apiregistrationv1apihelper.NewLocalAvailableAPIServiceCondition()) - _, err := c.updateAPIServiceStatus(originalAPIService, apiService) - return err - } - service, err := c.serviceLister.Services(apiService.Spec.Service.Namespace).Get(apiService.Spec.Service.Name) if apierrors.IsNotFound(err) { availableCondition.Status = apiregistrationv1.ConditionFalse @@ -410,14 +408,14 @@ func (c *AvailableConditionController) Run(workers int, stopCh <-chan struct{}) defer utilruntime.HandleCrash() defer c.queue.ShutDown() - klog.Info("Starting AvailableConditionController") - defer klog.Info("Shutting down AvailableConditionController") + klog.Info("Starting RemoteAvailability controller") + defer klog.Info("Shutting down RemoteAvailability controller") // This waits not just for the informers to sync, but for our handlers // to be called; since the handlers are three different ways of // enqueueing the same thing, waiting for this permits the queue to // maximally de-duplicate the entries. - if !controllers.WaitForCacheSync("AvailableConditionController", stopCh, c.apiServiceSynced, c.servicesSynced, c.endpointsSynced) { + if !controllers.WaitForCacheSync("RemoteAvailability", stopCh, c.apiServiceSynced, c.servicesSynced, c.endpointsSynced) { return } diff --git a/pkg/controllers/status/remote/remote_available_controller_test.go b/pkg/controllers/status/remote/remote_available_controller_test.go index d08c9ae1..acfe9ba3 100644 --- a/pkg/controllers/status/remote/remote_available_controller_test.go +++ b/pkg/controllers/status/remote/remote_available_controller_test.go @@ -225,6 +225,7 @@ func TestSync(t *testing.T) { expectedAvailability apiregistration.APIServiceCondition expectedSyncError string + expectedSkipped bool }{ { name: "local", @@ -237,6 +238,7 @@ func TestSync(t *testing.T) { Reason: "Local", Message: "Local APIServices are always available", }, + expectedSkipped: true, }, { name: "no service", @@ -420,6 +422,13 @@ func TestSync(t *testing.T) { t.Fatalf("%v unexpected sync error: %v", tc.name, err) } + if tc.expectedSkipped { + if len(fakeClient.Actions()) > 0 { + t.Fatalf("%v expected no actions, got %v", tc.name, fakeClient.Actions()) + } + return + } + // ought to have one action writing status if e, a := 1, len(fakeClient.Actions()); e != a { t.Fatalf("%v expected %v, got %v", tc.name, e, fakeClient.Actions())