Merge pull request red-hat-storage#64 from bernerhat/BZ-2251308

Redesign for ClusterVersion controller into OperatorConfigMap controller

controllers/clusterversion_controller.go → controllers/operatorconfigmap_controller.go

@@ -66,16 +66,16 @@ const (
 	subscriptionLabelValue = "webhook.subscription.ocs.openshift.io"
 )
 
-// ClusterVersionReconciler reconciles a ClusterVersion object
-type ClusterVersionReconciler struct {
+// OperatorConfigMapReconciler reconciles a ClusterVersion object
+type OperatorConfigMapReconciler struct {
 	client.Client
-	OperatorDeployment *appsv1.Deployment
-	OperatorNamespace  string
-	ConsolePort        int32
-	Scheme             *runtime.Scheme
+	OperatorNamespace string
+	ConsolePort       int32
+	Scheme            *runtime.Scheme
 
 	log               logr.Logger
 	ctx               context.Context
+	operatorConfigMap *corev1.ConfigMap
 	consoleDeployment *appsv1.Deployment
 	cephFSDeployment  *appsv1.Deployment
 	cephFSDaemonSet   *appsv1.DaemonSet
@@ -85,7 +85,7 @@ type ClusterVersionReconciler struct {
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (c *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (c *OperatorConfigMapReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	clusterVersionPredicates := builder.WithPredicates(
 		predicate.GenerationChangedPredicate{},
 	)
@@ -99,12 +99,13 @@ func (c *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			},
 		),
 	)
-	// Reconcile the ClusterVersion object when the operator config map is updated
-	enqueueClusterVersionRequest := handler.EnqueueRequestsFromMapFunc(
+	// Reconcile the OperatorConfigMap object when the cluster's version object is updated
+	enqueueConfigMapRequest := handler.EnqueueRequestsFromMapFunc(
 		func(_ context.Context, _ client.Object) []reconcile.Request {
 			return []reconcile.Request{{
 				NamespacedName: types.NamespacedName{
-					Name: clusterVersionName,
+					Name:      operatorConfigMapName,
+					Namespace: c.OperatorNamespace,
 				},
 			}}
 		},
@@ -128,24 +129,23 @@ func (c *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	)
 
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&configv1.ClusterVersion{}, clusterVersionPredicates).
-		Watches(&corev1.ConfigMap{}, enqueueClusterVersionRequest, configMapPredicates).
-		Watches(&extv1.CustomResourceDefinition{}, enqueueClusterVersionRequest, builder.OnlyMetadata).
-		Watches(&opv1a1.Subscription{}, enqueueClusterVersionRequest, subscriptionPredicates).
-		Watches(&admrv1.ValidatingWebhookConfiguration{}, enqueueClusterVersionRequest, webhookPredicates).
+		For(&corev1.ConfigMap{}, configMapPredicates).
+		Watches(&configv1.ClusterVersion{}, enqueueConfigMapRequest, clusterVersionPredicates).
+		Watches(&extv1.CustomResourceDefinition{}, enqueueConfigMapRequest, builder.OnlyMetadata).
+		Watches(&opv1a1.Subscription{}, enqueueConfigMapRequest, subscriptionPredicates).
+		Watches(&admrv1.ValidatingWebhookConfiguration{}, enqueueConfigMapRequest, webhookPredicates).
 		Complete(c)
 }
 
 //+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch
-//+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions/status,verbs=get;update;patch
-//+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions/finalizers,verbs=update
+//+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions,verbs=get;list;watch
 //+kubebuilder:rbac:groups="apps",resources=deployments,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups="apps",resources=deployments/finalizers,verbs=update
 //+kubebuilder:rbac:groups="apps",resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups="apps",resources=daemonsets/finalizers,verbs=update
 //+kubebuilder:rbac:groups="storage.k8s.io",resources=csidrivers,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;delete
+//+kubebuilder:rbac:groups="",resources=configmaps/finalizers,verbs=update
 //+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=get;list;watch;create;patch;update
 //+kubebuilder:rbac:groups=monitoring.coreos.com,resources=prometheusrules,verbs=get;list;watch;create;update
 //+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete
@@ -155,10 +155,18 @@ func (c *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
-func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (c *OperatorConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	c.ctx = ctx
-	c.log = log.FromContext(ctx, "ClusterVersion", req)
-	c.log.Info("Reconciling ClusterVersion")
+	c.log = log.FromContext(ctx, "OperatorConfigMap", req)
+	c.log.Info("Reconciling OperatorConfigMap")
+
+	c.operatorConfigMap = &corev1.ConfigMap{}
+	c.operatorConfigMap.Name = req.Name
+	c.operatorConfigMap.Namespace = req.Namespace
+	if err := c.get(c.operatorConfigMap); err != nil {
+		c.log.Error(err, "failed to get the operator's configMap")
+		return reconcile.Result{}, err
+	}
 
 	if err := c.reconcileSubscriptionValidatingWebhook(); err != nil {
 		c.log.Error(err, "unable to register subscription validating webhook")
@@ -179,12 +187,14 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		c.log.Error(err, "failed to perform precheck for deploying CSI")
 		return ctrl.Result{}, err
 	} else if deployCSI {
-		instance := configv1.ClusterVersion{}
-		if err = c.Client.Get(context.TODO(), req.NamespacedName, &instance); err != nil {
-			return ctrl.Result{}, err
+		clusterVersion := &configv1.ClusterVersion{}
+		clusterVersion.Name = clusterVersionName
+		if err := c.get(clusterVersion); err != nil {
+			c.log.Error(err, "failed to get the clusterVersion version of the OCP cluster")
+			return reconcile.Result{}, err
 		}
 
-		if err := csi.InitializeSidecars(c.log, instance.Status.Desired.Version); err != nil {
+		if err := csi.InitializeSidecars(c.log, clusterVersion.Status.Desired.Version); err != nil {
 			c.log.Error(err, "unable to initialize sidecars")
 			return ctrl.Result{}, err
 		}
@@ -221,8 +231,8 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		if err := c.own(monConfigMap); err != nil {
 			return ctrl.Result{}, err
 		}
-		err = c.create(monConfigMap)
-		if err != nil && !kerrors.IsAlreadyExists(err) {
+
+		if err := c.create(monConfigMap); err != nil && !kerrors.IsAlreadyExists(err) {
 			c.log.Error(err, "failed to create monitor configmap", "name", monConfigMap.Name)
 			return ctrl.Result{}, err
 		}
@@ -242,8 +252,8 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		if err := c.own(encConfigMap); err != nil {
 			return ctrl.Result{}, err
 		}
-		err = c.create(encConfigMap)
-		if err != nil && !kerrors.IsAlreadyExists(err) {
+
+		if err := c.create(encConfigMap); err != nil && !kerrors.IsAlreadyExists(err) {
 			c.log.Error(err, "failed to create monitor configmap", "name", encConfigMap.Name)
 			return ctrl.Result{}, err
 		}
@@ -324,35 +334,28 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		// ownerReference on it as its cluster scoped resource
 		cephfsCSIDriver := templates.CephFSCSIDriver.DeepCopy()
 		cephfsCSIDriver.ObjectMeta.Name = csi.GetCephFSDriverName()
-		err = csi.CreateCSIDriver(c.ctx, c.Client, cephfsCSIDriver)
-		if err != nil {
+		if err := csi.CreateCSIDriver(c.ctx, c.Client, cephfsCSIDriver); err != nil {
 			c.log.Error(err, "unable to create cephfs CSIDriver")
 			return ctrl.Result{}, err
 		}
 
 		rbdCSIDriver := templates.RbdCSIDriver.DeepCopy()
 		rbdCSIDriver.ObjectMeta.Name = csi.GetRBDDriverName()
-		err = csi.CreateCSIDriver(c.ctx, c.Client, rbdCSIDriver)
-		if err != nil {
+		if err := csi.CreateCSIDriver(c.ctx, c.Client, rbdCSIDriver); err != nil {
 			c.log.Error(err, "unable to create rbd CSIDriver")
 			return ctrl.Result{}, err
 		}
 
 		prometheusRule := &monitoringv1.PrometheusRule{}
-		err = k8sYAML.NewYAMLOrJSONDecoder(bytes.NewBufferString(string(pvcPrometheusRules)), 1000).Decode(prometheusRule)
-		if err != nil {
+		if err := k8sYAML.NewYAMLOrJSONDecoder(bytes.NewBufferString(string(pvcPrometheusRules)), 1000).Decode(prometheusRule); err != nil {
 			c.log.Error(err, "Unable to retrieve prometheus rules.", "prometheusRule", klog.KRef(prometheusRule.Namespace, prometheusRule.Name))
 			return ctrl.Result{}, err
 		}
 
-		operatorConfig, err := c.getOperatorConfig()
-		if err != nil {
-			return ctrl.Result{}, err
-		}
 		prometheusRule.SetNamespace(c.OperatorNamespace)
 
 		err = c.createOrUpdate(prometheusRule, func() error {
-			applyLabels(operatorConfig.Data["OCS_METRICS_LABELS"], &prometheusRule.ObjectMeta)
+			applyLabels(c.operatorConfigMap.Data["OCS_METRICS_LABELS"], &prometheusRule.ObjectMeta)
 			return c.own(prometheusRule)
 		})
 		if err != nil {
@@ -366,7 +369,7 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	return ctrl.Result{}, nil
 }
 
-func (c *ClusterVersionReconciler) createOrUpdate(obj client.Object, f controllerutil.MutateFn) error {
+func (c *OperatorConfigMapReconciler) createOrUpdate(obj client.Object, f controllerutil.MutateFn) error {
 	result, err := controllerutil.CreateOrUpdate(c.ctx, c.Client, obj, f)
 	if err != nil {
 		return err
@@ -375,11 +378,11 @@ func (c *ClusterVersionReconciler) createOrUpdate(obj client.Object, f controlle
 	return nil
 }
 
-func (c *ClusterVersionReconciler) own(obj client.Object) error {
-	return controllerutil.SetControllerReference(c.OperatorDeployment, obj, c.Client.Scheme())
+func (c *OperatorConfigMapReconciler) own(obj client.Object) error {
+	return controllerutil.SetControllerReference(c.operatorConfigMap, obj, c.Client.Scheme())
 }
 
-func (c *ClusterVersionReconciler) create(obj client.Object) error {
+func (c *OperatorConfigMapReconciler) create(obj client.Object) error {
 	return c.Client.Create(c.ctx, obj)
 }
 
@@ -403,27 +406,15 @@ func applyLabels(label string, t *metav1.ObjectMeta) {
 	t.Labels = promLabel
 }
 
-func (c *ClusterVersionReconciler) getOperatorConfig() (*corev1.ConfigMap, error) {
-	cm := &corev1.ConfigMap{}
-	err := c.Client.Get(c.ctx, types.NamespacedName{Name: operatorConfigMapName, Namespace: c.OperatorNamespace}, cm)
-	if err != nil && !kerrors.IsNotFound(err) {
-		return nil, err
-	}
-	return cm, nil
-}
-
-func (c *ClusterVersionReconciler) ensureConsolePlugin() error {
+func (c *OperatorConfigMapReconciler) ensureConsolePlugin() error {
 	c.consoleDeployment = &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      console.DeploymentName,
 			Namespace: c.OperatorNamespace,
 		},
 	}
 
-	err := c.Client.Get(c.ctx, types.NamespacedName{
-		Name:      console.DeploymentName,
-		Namespace: c.OperatorNamespace,
-	}, c.consoleDeployment)
+	err := c.get(c.consoleDeployment)
 	if err != nil {
 		c.log.Error(err, "failed to get the deployment for the console")
 		return err
@@ -483,15 +474,8 @@ func (c *ClusterVersionReconciler) ensureConsolePlugin() error {
 	return nil
 }
 
-func (c *ClusterVersionReconciler) getDeployCSIConfig() (bool, error) {
-	operatorConfig := &corev1.ConfigMap{}
-	operatorConfig.Name = operatorConfigMapName
-	operatorConfig.Namespace = c.OperatorNamespace
-	if err := c.get(operatorConfig); err != nil {
-		return false, fmt.Errorf("failed to get operator configmap: %v", err)
-	}
-
-	data := operatorConfig.Data
+func (c *OperatorConfigMapReconciler) getDeployCSIConfig() (bool, error) {
+	data := c.operatorConfigMap.Data
 	if data == nil {
 		data = map[string]string{}
 	}
@@ -526,11 +510,11 @@ func (c *ClusterVersionReconciler) getDeployCSIConfig() (bool, error) {
 	return deployCSI, nil
 }
 
-func (c *ClusterVersionReconciler) get(obj client.Object, opts ...client.GetOption) error {
+func (c *OperatorConfigMapReconciler) get(obj client.Object, opts ...client.GetOption) error {
 	return c.Get(c.ctx, client.ObjectKeyFromObject(obj), obj, opts...)
 }
 
-func (c *ClusterVersionReconciler) reconcileSubscriptionValidatingWebhook() error {
+func (c *OperatorConfigMapReconciler) reconcileSubscriptionValidatingWebhook() error {
 	whConfig := &admrv1.ValidatingWebhookConfiguration{}
 	whConfig.Name = templates.SubscriptionWebhookName
 
@@ -583,7 +567,7 @@ func (c *ClusterVersionReconciler) reconcileSubscriptionValidatingWebhook() erro
 	return nil
 }
 
-func labelClientOperatorSubscription(c *ClusterVersionReconciler) error {
+func labelClientOperatorSubscription(c *OperatorConfigMapReconciler) error {
 	subscriptionList := &opv1a1.SubscriptionList{}
 	err := c.List(c.ctx, subscriptionList, client.InNamespace(c.OperatorNamespace))
 	if err != nil {

main.go

@@ -17,7 +17,6 @@ limitations under the License.
 package main
 
 import (
-	"context"
 	"flag"
 	"os"
 
@@ -124,12 +123,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	// apiclient.New() returns a client without cache.
-	// cache is not initialized before mgr.Start()
-	// we need this because we need to interact with OperatorCondition
-	apiClient, err := client.New(mgr.GetConfig(), client.Options{
-		Scheme: mgr.GetScheme(),
-	})
 	if err != nil {
 		setupLog.Error(err, "Unable to get Client")
 		os.Exit(1)
@@ -195,20 +188,13 @@ func main() {
 		os.Exit(1)
 	}
 
-	operatorDeployment, err := utils.GetOperatorDeployment(context.TODO(), apiClient)
-	if err != nil {
-		setupLog.Error(err, "unable to get operator deployment")
-		os.Exit(1)
-	}
-
-	if err = (&controllers.ClusterVersionReconciler{
-		Client:             mgr.GetClient(),
-		Scheme:             mgr.GetScheme(),
-		OperatorDeployment: operatorDeployment,
-		OperatorNamespace:  utils.GetOperatorNamespace(),
-		ConsolePort:        int32(consolePort),
+	if err = (&controllers.OperatorConfigMapReconciler{
+		Client:            mgr.GetClient(),
+		Scheme:            mgr.GetScheme(),
+		OperatorNamespace: utils.GetOperatorNamespace(),
+		ConsolePort:       int32(consolePort),
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ClusterVersionReconciler")
+		setupLog.Error(err, "unable to create controller", "controller", "OperatorConfigMapReconciler")
 		os.Exit(1)
 	}