From 3284c43fbe20e7252046b273802bff5212a8518b Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 26 Sep 2024 15:20:51 -0400 Subject: [PATCH] fix up minor comments from Lucasz --- .../auditpolicy/auditpolicy_controller.go | 22 ++++++++------- .../nsfinalizer/finalizer_controller.go | 28 ++++++++++--------- .../apiserver/controller/workload/workload.go | 9 ++++-- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/pkg/operator/apiserver/controller/auditpolicy/auditpolicy_controller.go b/pkg/operator/apiserver/controller/auditpolicy/auditpolicy_controller.go index 826d3fd6e9..f43b64922c 100644 --- a/pkg/operator/apiserver/controller/auditpolicy/auditpolicy_controller.go +++ b/pkg/operator/apiserver/controller/auditpolicy/auditpolicy_controller.go @@ -4,13 +4,12 @@ import ( "context" "time" - applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" - configv1 "github.com/openshift/api/config/v1" operatorsv1 "github.com/openshift/api/operator/v1" operatorv1 "github.com/openshift/api/operator/v1" configinformers "github.com/openshift/client-go/config/informers/externalversions" configv1listers "github.com/openshift/client-go/config/listers/config/v1" + applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/apiserver/audit" "github.com/openshift/library-go/pkg/operator/events" @@ -35,7 +34,7 @@ type auditPolicyController struct { // NewAuditPolicyController create a controller that watches the config.openshift.io/v1 APIServer object // and reconciles a ConfigMap in the target namespace with the audit.k8s.io/v1 policy.yaml file. func NewAuditPolicyController( - name string, + instanceName string, targetNamespace string, targetConfigMapName string, apiserverConfigLister configv1listers.APIServerLister, @@ -46,7 +45,7 @@ func NewAuditPolicyController( eventRecorder events.Recorder, ) factory.Controller { c := &auditPolicyController{ - controllerInstanceName: factory.ControllerInstanceName(name, "AuditPolicy"), + controllerInstanceName: factory.ControllerInstanceName(instanceName, "AuditPolicy"), operatorClient: operatorClient, apiserverConfigLister: apiserverConfigLister, kubeClient: kubeClient, @@ -54,12 +53,15 @@ func NewAuditPolicyController( targetConfigMapName: targetConfigMapName, } - return factory.New().WithSync(c.sync).ResyncEvery(10*time.Second).WithInformers( - configInformers.Config().V1().APIServers().Informer(), - kubeInformersForTargetNamesace.Core().V1().ConfigMaps().Informer(), - operatorClient.Informer(), - ).ToController( - "auditPolicyController", // don't change what is passed here unless you also remove the old FooDegraded condition + return factory.New(). + WithSync(c.sync). + ResyncEvery(10*time.Second). + WithInformers( + configInformers.Config().V1().APIServers().Informer(), + kubeInformersForTargetNamesace.Core().V1().ConfigMaps().Informer(), + operatorClient.Informer(), + ).ToController( + c.controllerInstanceName, eventRecorder.WithComponentSuffix("audit-policy-controller"), ) } diff --git a/pkg/operator/apiserver/controller/nsfinalizer/finalizer_controller.go b/pkg/operator/apiserver/controller/nsfinalizer/finalizer_controller.go index caaeda5292..6ce01d8d05 100644 --- a/pkg/operator/apiserver/controller/nsfinalizer/finalizer_controller.go +++ b/pkg/operator/apiserver/controller/nsfinalizer/finalizer_controller.go @@ -20,8 +20,8 @@ import ( ) type finalizerController struct { - name string - namespaceName string + controllerInstanceName string + namespaceName string namespaceGetter v1.NamespacesGetter podLister corev1listers.PodLister @@ -41,20 +41,22 @@ func NewFinalizerController( namespaceGetter v1.NamespacesGetter, eventRecorder events.Recorder, ) factory.Controller { - fullname := "NamespaceFinalizerController_" + namespaceName c := &finalizerController{ - name: fullname, - namespaceName: namespaceName, - namespaceGetter: namespaceGetter, - podLister: kubeInformersForTargetNamespace.Core().V1().Pods().Lister(), - dsLister: kubeInformersForTargetNamespace.Apps().V1().DaemonSets().Lister(), + controllerInstanceName: factory.ControllerInstanceName(namespaceName, "NamespaceFinalizerController"), + namespaceName: namespaceName, + namespaceGetter: namespaceGetter, + podLister: kubeInformersForTargetNamespace.Core().V1().Pods().Lister(), + dsLister: kubeInformersForTargetNamespace.Apps().V1().DaemonSets().Lister(), } - return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithInformers( - kubeInformersForTargetNamespace.Core().V1().Pods().Informer(), - kubeInformersForTargetNamespace.Apps().V1().DaemonSets().Informer(), - ).ToController( - fullname, // don't change what is passed here unless you also remove the old FooDegraded condition + return factory.New(). + ResyncEvery(time.Minute). + WithSync(c.sync). + WithInformers( + kubeInformersForTargetNamespace.Core().V1().Pods().Informer(), + kubeInformersForTargetNamespace.Apps().V1().DaemonSets().Informer(), + ).ToController( + c.controllerInstanceName, eventRecorder.WithComponentSuffix("finalizer-controller"), ) } diff --git a/pkg/operator/apiserver/controller/workload/workload.go b/pkg/operator/apiserver/controller/workload/workload.go index 443c1ed8f3..c972ecdd16 100644 --- a/pkg/operator/apiserver/controller/workload/workload.go +++ b/pkg/operator/apiserver/controller/workload/workload.go @@ -49,6 +49,8 @@ type Delegate interface { // Callers must provide a sync function for delegation. It should bring the desired workload into operation. // The returned state along with errors will be converted into conditions and persisted in the status field. type Controller struct { + controllerInstanceName string + // conditionsPrefix an optional prefix that will be used as operator's condition type field for example APIServerDeploymentDegraded where APIServer indicates the prefix conditionsPrefix string operatorNamespace string @@ -77,7 +79,7 @@ type Controller struct { // the "operatorNamespace" is used to set "version-mapping" in the correct namespace // // the "targetNamespace" represent the namespace for the managed resource (DaemonSet) -func NewController(name, operatorNamespace, targetNamespace, targetOperandVersion, operandNamePrefix, conditionsPrefix string, +func NewController(instanceName, operatorNamespace, targetNamespace, targetOperandVersion, operandNamePrefix, conditionsPrefix string, operatorClient v1helpers.OperatorClient, kubeClient kubernetes.Interface, podLister corev1listers.PodLister, @@ -89,6 +91,7 @@ func NewController(name, operatorNamespace, targetNamespace, targetOperandVersio versionRecorder status.VersionGetter, ) factory.Controller { controllerRef := &Controller{ + controllerInstanceName: factory.ControllerInstanceName(instanceName, "Workload"), operatorNamespace: operatorNamespace, targetNamespace: targetNamespace, targetOperandVersion: targetOperandVersion, @@ -100,7 +103,7 @@ func NewController(name, operatorNamespace, targetNamespace, targetOperandVersio delegate: delegate, openshiftClusterConfigClient: openshiftClusterConfigClient, versionRecorder: versionRecorder, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name), + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), instanceName), } c := factory.New() @@ -111,7 +114,7 @@ func NewController(name, operatorNamespace, targetNamespace, targetOperandVersio return c.WithSync(controllerRef.sync). WithInformers(informers...). ToController( - fmt.Sprintf("%sWorkloadController", name), // don't change what is passed here unless you also remove the old FooDegraded condition + controllerRef.controllerInstanceName, eventRecorder, ) }