From 7c873327b679a70337288da62b96dd610858181d Mon Sep 17 00:00:00 2001 From: Maciej Skrocki Date: Wed, 28 Jun 2023 22:45:56 +0000 Subject: [PATCH] Convert controller name to reconciler variable. --- .../endpointslice/endpointslice_controller.go | 9 +++-- .../src/k8s.io/endpointslice/reconciler.go | 39 ++++++++++++------- .../k8s.io/endpointslice/reconciler_test.go | 7 +++- staging/src/k8s.io/endpointslice/utils.go | 19 ++------- .../src/k8s.io/endpointslice/utils_test.go | 4 +- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/pkg/controller/endpointslice/endpointslice_controller.go b/pkg/controller/endpointslice/endpointslice_controller.go index c25214d5344..262fc04dee3 100644 --- a/pkg/controller/endpointslice/endpointslice_controller.go +++ b/pkg/controller/endpointslice/endpointslice_controller.go @@ -174,6 +174,7 @@ func NewController(ctx context.Context, podInformer coreinformers.PodInformer, c.endpointSliceTracker, c.topologyCache, c.eventRecorder, + controllerName, ) return c @@ -366,7 +367,7 @@ func (c *Controller) syncService(logger klog.Logger, key string) error { esLabelSelector := labels.Set(map[string]string{ discovery.LabelServiceName: service.Name, - discovery.LabelManagedBy: endpointslicerec.GetReconcilerName(), + discovery.LabelManagedBy: c.reconciler.GetControllerName(), }).AsSelectorPreValidated() endpointSlices, err := c.endpointSliceLister.EndpointSlices(service.Namespace).List(esLabelSelector) @@ -432,7 +433,7 @@ func (c *Controller) onEndpointSliceAdd(obj interface{}) { utilruntime.HandleError(fmt.Errorf("Invalid EndpointSlice provided to onEndpointSliceAdd()")) return } - if endpointslicerec.ManagedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice) { + if c.reconciler.ManagedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice) { c.queueServiceForEndpointSlice(endpointSlice) } } @@ -459,7 +460,7 @@ func (c *Controller) onEndpointSliceUpdate(logger klog.Logger, prevObj, obj inte c.queueServiceForEndpointSlice(prevEndpointSlice) return } - if endpointslicerec.ManagedByChanged(prevEndpointSlice, endpointSlice) || (endpointslicerec.ManagedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice)) { + if c.reconciler.ManagedByChanged(prevEndpointSlice, endpointSlice) || (c.reconciler.ManagedByController(endpointSlice) && c.endpointSliceTracker.ShouldSync(endpointSlice)) { c.queueServiceForEndpointSlice(endpointSlice) } } @@ -469,7 +470,7 @@ func (c *Controller) onEndpointSliceUpdate(logger klog.Logger, prevObj, obj inte // endpointSliceTracker. func (c *Controller) onEndpointSliceDelete(obj interface{}) { endpointSlice := getEndpointSliceFromDeleteAction(obj) - if endpointSlice != nil && endpointslicerec.ManagedByController(endpointSlice) && c.endpointSliceTracker.Has(endpointSlice) { + if endpointSlice != nil && c.reconciler.ManagedByController(endpointSlice) && c.endpointSliceTracker.Has(endpointSlice) { // This returns false if we didn't expect the EndpointSlice to be // deleted. If that is the case, we queue the Service for another sync. if !c.endpointSliceTracker.HandleDeletion(endpointSlice) { diff --git a/staging/src/k8s.io/endpointslice/reconciler.go b/staging/src/k8s.io/endpointslice/reconciler.go index 827bcdc01a2..d1f59af8ce3 100644 --- a/staging/src/k8s.io/endpointslice/reconciler.go +++ b/staging/src/k8s.io/endpointslice/reconciler.go @@ -39,12 +39,6 @@ import ( "k8s.io/klog/v2" ) -const ( - // controllerName is a unique value used with LabelManagedBy to indicated - // the component managing an EndpointSlice. - controllerName = "endpointslice-controller.k8s.io" -) - // Reconciler is responsible for transforming current EndpointSlice state into // desired state type Reconciler struct { @@ -57,7 +51,8 @@ type Reconciler struct { // to enable TopologyAwareHints. topologyCache *topologycache.TopologyCache // eventRecorder allows Reconciler to record and publish events. - eventRecorder record.EventRecorder + eventRecorder record.EventRecorder + controllerName string } // endpointMeta includes the attributes we group slices on, this type helps with @@ -236,7 +231,7 @@ func (r *Reconciler) reconcileByAddressType(logger klog.Logger, service *corev1. // When no endpoint slices would usually exist, we need to add a placeholder. if len(existingSlices) == len(slicesToDelete) && len(slicesToCreate) < 1 { // Check for existing placeholder slice outside of the core control flow - placeholderSlice := newEndpointSlice(logger, service, &endpointMeta{ports: []discovery.EndpointPort{}, addressType: addressType}) + placeholderSlice := newEndpointSlice(logger, service, &endpointMeta{ports: []discovery.EndpointPort{}, addressType: addressType}, r.controllerName) if len(slicesToDelete) == 1 && placeholderSliceCompare.DeepEqual(slicesToDelete[0], placeholderSlice) { // We are about to unnecessarily delete/recreate the placeholder, remove it now. slicesToDelete = slicesToDelete[:0] @@ -293,7 +288,7 @@ func (r *Reconciler) reconcileByAddressType(logger klog.Logger, service *corev1. } -func NewReconciler(client clientset.Interface, nodeLister corelisters.NodeLister, maxEndpointsPerSlice int32, endpointSliceTracker *endpointsliceutil.EndpointSliceTracker, topologyCache *topologycache.TopologyCache, eventRecorder record.EventRecorder) *Reconciler { +func NewReconciler(client clientset.Interface, nodeLister corelisters.NodeLister, maxEndpointsPerSlice int32, endpointSliceTracker *endpointsliceutil.EndpointSliceTracker, topologyCache *topologycache.TopologyCache, eventRecorder record.EventRecorder, controllerName string) *Reconciler { return &Reconciler{ client: client, nodeLister: nodeLister, @@ -302,13 +297,10 @@ func NewReconciler(client clientset.Interface, nodeLister corelisters.NodeLister metricsCache: metrics.NewCache(maxEndpointsPerSlice), topologyCache: topologyCache, eventRecorder: eventRecorder, + controllerName: controllerName, } } -func GetReconcilerName() string { - return controllerName -} - // placeholderSliceCompare is a conversion func for comparing two placeholder endpoint slices. // It only compares the specific fields we care about. var placeholderSliceCompare = conversion.EqualitiesOrDie( @@ -463,7 +455,7 @@ func (r *Reconciler) reconcileByPortMapping( } // generate the slice labels and check if parent labels have changed - labels, labelsChanged := setEndpointSliceLabels(logger, existingSlice, service) + labels, labelsChanged := setEndpointSliceLabels(logger, existingSlice, service, r.controllerName) // If an endpoint was updated or removed, mark for update or delete if endpointUpdated || len(existingSlice.Endpoints) != len(newEndpoints) { @@ -536,7 +528,7 @@ func (r *Reconciler) reconcileByPortMapping( // If we didn't find a sliceToFill, generate a new empty one. if sliceToFill == nil { - sliceToFill = newEndpointSlice(logger, service, endpointMeta) + sliceToFill = newEndpointSlice(logger, service, endpointMeta, r.controllerName) } else { // deep copy required to modify this slice. sliceToFill = sliceToFill.DeepCopy() @@ -577,3 +569,20 @@ func (r *Reconciler) reconcileByPortMapping( func (r *Reconciler) DeleteService(namespace, name string) { r.metricsCache.DeleteService(types.NamespacedName{Namespace: namespace, Name: name}) } + +func (r *Reconciler) GetControllerName() string { + return r.controllerName +} + +// ManagedByChanged returns true if one of the provided EndpointSlices is +// managed by the EndpointSlice controller while the other is not. +func (r *Reconciler) ManagedByChanged(endpointSlice1, endpointSlice2 *discovery.EndpointSlice) bool { + return r.ManagedByController(endpointSlice1) != r.ManagedByController(endpointSlice2) +} + +// ManagedByController returns true if the controller of the provided +// EndpointSlices is the EndpointSlice controller. +func (r *Reconciler) ManagedByController(endpointSlice *discovery.EndpointSlice) bool { + managedBy := endpointSlice.Labels[discovery.LabelManagedBy] + return managedBy == r.controllerName +} diff --git a/staging/src/k8s.io/endpointslice/reconciler_test.go b/staging/src/k8s.io/endpointslice/reconciler_test.go index f2992729068..03d4f92e981 100644 --- a/staging/src/k8s.io/endpointslice/reconciler_test.go +++ b/staging/src/k8s.io/endpointslice/reconciler_test.go @@ -46,6 +46,10 @@ import ( "k8s.io/utils/pointer" ) +const ( + controllerName = "endpointslice-controller.k8s.io" +) + func expectAction(t *testing.T, actions []k8stesting.Action, index int, verb, resource string) { t.Helper() if len(actions) <= index { @@ -555,7 +559,7 @@ func TestReconcile1EndpointSlice(t *testing.T) { }, { desc: "Existing placeholder that's the same", - existing: newEndpointSlice(logger, &svc, &endpointMeta{ports: []discovery.EndpointPort{}, addressType: discovery.AddressTypeIPv4}), + existing: newEndpointSlice(logger, &svc, &endpointMeta{ports: []discovery.EndpointPort{}, addressType: discovery.AddressTypeIPv4}, controllerName), wantMetrics: expectedMetrics{desiredSlices: 1, actualSlices: 1, desiredEndpoints: 0, addedPerSync: 0, removedPerSync: 0, numCreated: 0, numUpdated: 0, numDeleted: 0, slicesChangedPerSync: 0}, }, { @@ -1986,6 +1990,7 @@ func newReconciler(client *fake.Clientset, nodes []*corev1.Node, maxEndpointsPer endpointsliceutil.NewEndpointSliceTracker(), nil, eventRecorder, + controllerName, ) } diff --git a/staging/src/k8s.io/endpointslice/utils.go b/staging/src/k8s.io/endpointslice/utils.go index c003774d76f..3aa35ec20c1 100644 --- a/staging/src/k8s.io/endpointslice/utils.go +++ b/staging/src/k8s.io/endpointslice/utils.go @@ -124,7 +124,7 @@ func getEndpointAddresses(podStatus v1.PodStatus, service *v1.Service, addressTy // newEndpointSlice returns an EndpointSlice generated from a service and // endpointMeta. -func newEndpointSlice(logger klog.Logger, service *v1.Service, endpointMeta *endpointMeta) *discovery.EndpointSlice { +func newEndpointSlice(logger klog.Logger, service *v1.Service, endpointMeta *endpointMeta, controllerName string) *discovery.EndpointSlice { gvk := schema.GroupVersionKind{Version: "v1", Kind: "Service"} ownerRef := metav1.NewControllerRef(service, gvk) epSlice := &discovery.EndpointSlice{ @@ -139,7 +139,7 @@ func newEndpointSlice(logger klog.Logger, service *v1.Service, endpointMeta *end Endpoints: []discovery.Endpoint{}, } // add parent service labels - epSlice.Labels, _ = setEndpointSliceLabels(logger, epSlice, service) + epSlice.Labels, _ = setEndpointSliceLabels(logger, epSlice, service, controllerName) return epSlice } @@ -213,7 +213,7 @@ func ServiceControllerKey(endpointSlice *discovery.EndpointSlice) (string, error // setEndpointSliceLabels returns a map with the new endpoint slices labels and true if there was an update. // Slices labels must be equivalent to the Service labels except for the reserved IsHeadlessService, LabelServiceName and LabelManagedBy labels // Changes to IsHeadlessService, LabelServiceName and LabelManagedBy labels on the Service do not result in updates to EndpointSlice labels. -func setEndpointSliceLabels(logger klog.Logger, epSlice *discovery.EndpointSlice, service *v1.Service) (map[string]string, bool) { +func setEndpointSliceLabels(logger klog.Logger, epSlice *discovery.EndpointSlice, service *v1.Service, controllerName string) (map[string]string, bool) { updated := false epLabels := make(map[string]string) svcLabels := make(map[string]string) @@ -368,19 +368,6 @@ func hintsEnabled(annotations map[string]string) bool { return val == "Auto" || val == "auto" } -// ManagedByChanged returns true if one of the provided EndpointSlices is -// managed by the EndpointSlice controller while the other is not. -func ManagedByChanged(endpointSlice1, endpointSlice2 *discovery.EndpointSlice) bool { - return ManagedByController(endpointSlice1) != ManagedByController(endpointSlice2) -} - -// ManagedByController returns true if the controller of the provided -// EndpointSlices is the EndpointSlice controller. -func ManagedByController(endpointSlice *discovery.EndpointSlice) bool { - managedBy := endpointSlice.Labels[discovery.LabelManagedBy] - return managedBy == controllerName -} - // isServiceIPSet aims to check if the service's ClusterIP is set or not // the objective is not to perform validation here // copied from k8s.io/kubernetes/pkg/apis/core/v1/helper diff --git a/staging/src/k8s.io/endpointslice/utils_test.go b/staging/src/k8s.io/endpointslice/utils_test.go index 04caafdd0b8..eaa464b4473 100644 --- a/staging/src/k8s.io/endpointslice/utils_test.go +++ b/staging/src/k8s.io/endpointslice/utils_test.go @@ -209,7 +209,7 @@ func TestNewEndpointSlice(t *testing.T) { t.Run(tc.name, func(t *testing.T) { logger, _ := ktesting.NewTestContext(t) svc := tc.updateSvc(service) - generatedSlice := newEndpointSlice(logger, &svc, &endpointMeta) + generatedSlice := newEndpointSlice(logger, &svc, &endpointMeta, controllerName) assert.EqualValues(t, tc.expectedSlice, generatedSlice) }) } @@ -880,7 +880,7 @@ func TestSetEndpointSliceLabels(t *testing.T) { t.Run(tc.name, func(t *testing.T) { logger, _ := ktesting.NewTestContext(t) svc := tc.updateSvc(service) - labels, updated := setEndpointSliceLabels(logger, tc.epSlice, &svc) + labels, updated := setEndpointSliceLabels(logger, tc.epSlice, &svc, controllerName) assert.EqualValues(t, updated, tc.expectedUpdate) assert.EqualValues(t, tc.expectedLabels, labels) })