Update the owner reference of resources, if the reference is not correct. (#230)

This addresses the situation when a resource, that a SolrCloud or SolrPrometheusExporter is supposed to control, has a different OwnerReference with controller= true. If this is found, then the controlling owner resource will be downgraded to controller=false, and a controlling OwnerReference will be created for either the SolrCloud or SolrPrometheusExporter.

Author: HoustonPutman

controllers/solrcloud_controller.go

@@ -116,21 +116,26 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	// Generate Common Service
 	commonService := util.GenerateCommonService(instance)
-	if err := controllerutil.SetControllerReference(instance, commonService, r.scheme); err != nil {
-		return requeueOrNot, err
-	}
 
 	// Check if the Common Service already exists
 	commonServiceLogger := logger.WithValues("service", commonService.Name)
 	foundCommonService := &corev1.Service{}
 	err = r.Get(context.TODO(), types.NamespacedName{Name: commonService.Name, Namespace: commonService.Namespace}, foundCommonService)
 	if err != nil && errors.IsNotFound(err) {
 		commonServiceLogger.Info("Creating Common Service")
-		err = r.Create(context.TODO(), commonService)
-	} else if err == nil && util.CopyServiceFields(commonService, foundCommonService, commonServiceLogger) {
+		if err = controllerutil.SetControllerReference(instance, commonService, r.scheme); err == nil {
+			err = r.Create(context.TODO(), commonService)
+		}
+	} else if err == nil {
+		var needsUpdate bool
+		needsUpdate, err = util.OvertakeControllerRef(instance, foundCommonService, r.scheme)
+		needsUpdate = util.CopyServiceFields(commonService, foundCommonService, commonServiceLogger) || needsUpdate
+
 		// Update the found Service and write the result back if there are any changes
-		commonServiceLogger.Info("Updating Common Service")
-		err = r.Update(context.TODO(), foundCommonService)
+		if needsUpdate && err == nil {
+			commonServiceLogger.Info("Updating Common Service")
+			err = r.Update(context.TODO(), foundCommonService)
+		}
 	}
 	if err != nil {
 		return requeueOrNot, err
@@ -161,21 +166,26 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	// Generate HeadlessService
 	if instance.UsesHeadlessService() {
 		headless := util.GenerateHeadlessService(instance)
-		if err := controllerutil.SetControllerReference(instance, headless, r.scheme); err != nil {
-			return requeueOrNot, err
-		}
 
 		// Check if the HeadlessService already exists
 		headlessServiceLogger := logger.WithValues("service", headless.Name)
 		foundHeadless := &corev1.Service{}
 		err = r.Get(context.TODO(), types.NamespacedName{Name: headless.Name, Namespace: headless.Namespace}, foundHeadless)
 		if err != nil && errors.IsNotFound(err) {
-			headlessServiceLogger.Info("Creating HeadlessService")
-			err = r.Create(context.TODO(), headless)
-		} else if err == nil && util.CopyServiceFields(headless, foundHeadless, headlessServiceLogger) {
+			headlessServiceLogger.Info("Creating Headless Service")
+			if err = controllerutil.SetControllerReference(instance, headless, r.scheme); err == nil {
+				err = r.Create(context.TODO(), headless)
+			}
+		} else if err == nil {
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(instance, foundHeadless, r.scheme)
+			needsUpdate = util.CopyServiceFields(headless, foundHeadless, headlessServiceLogger) || needsUpdate
+
 			// Update the found HeadlessService and write the result back if there are any changes
-			headlessServiceLogger.Info("Updating HeadlessService")
-			err = r.Update(context.TODO(), foundHeadless)
+			if needsUpdate && err == nil {
+				headlessServiceLogger.Info("Updating Headless Service")
+				err = r.Update(context.TODO(), foundHeadless)
+			}
 		}
 		if err != nil {
 			return requeueOrNot, err
@@ -235,9 +245,6 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	if reconcileConfigInfo[util.SolrXmlFile] == "" {
 		// no user provided solr.xml, so create the default
 		configMap := util.GenerateConfigMap(instance)
-		if err := controllerutil.SetControllerReference(instance, configMap, r.scheme); err != nil {
-			return requeueOrNot, err
-		}
 
 		reconcileConfigInfo[util.SolrXmlMd5Annotation] = fmt.Sprintf("%x", md5.Sum([]byte(configMap.Data[util.SolrXmlFile])))
 		reconcileConfigInfo[util.SolrXmlFile] = configMap.Name
@@ -248,11 +255,19 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		err = r.Get(context.TODO(), types.NamespacedName{Name: configMap.Name, Namespace: configMap.Namespace}, foundConfigMap)
 		if err != nil && errors.IsNotFound(err) {
 			configMapLogger.Info("Creating ConfigMap")
-			err = r.Create(context.TODO(), configMap)
-		} else if err == nil && util.CopyConfigMapFields(configMap, foundConfigMap, configMapLogger) {
+			if err = controllerutil.SetControllerReference(instance, configMap, r.scheme); err == nil {
+				err = r.Create(context.TODO(), configMap)
+			}
+		} else if err == nil {
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(instance, foundConfigMap, r.scheme)
+			needsUpdate = util.CopyConfigMapFields(configMap, foundConfigMap, configMapLogger) || needsUpdate
+
 			// Update the found ConfigMap and write the result back if there are any changes
-			configMapLogger.Info("Updating ConfigMap")
-			err = r.Update(context.TODO(), foundConfigMap)
+			if needsUpdate && err == nil {
+				configMapLogger.Info("Updating ConfigMap")
+				err = r.Update(context.TODO(), foundConfigMap)
+			}
 		}
 		if err != nil {
 			return requeueOrNot, err
@@ -395,28 +410,33 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	if !blockReconciliationOfStatefulSet {
 		// Generate StatefulSet
 		statefulSet := util.GenerateStatefulSet(instance, &newStatus, hostNameIpMap, reconcileConfigInfo, needsPkcs12InitContainer, tlsCertMd5)
-		if err := controllerutil.SetControllerReference(instance, statefulSet, r.scheme); err != nil {
-			return requeueOrNot, err
-		}
 
 		// Check if the StatefulSet already exists
 		statefulSetLogger := logger.WithValues("statefulSet", statefulSet.Name)
 		foundStatefulSet := &appsv1.StatefulSet{}
 		err = r.Get(context.TODO(), types.NamespacedName{Name: statefulSet.Name, Namespace: statefulSet.Namespace}, foundStatefulSet)
 		if err != nil && errors.IsNotFound(err) {
 			statefulSetLogger.Info("Creating StatefulSet")
-			err = r.Create(context.TODO(), statefulSet)
+			if err = controllerutil.SetControllerReference(instance, statefulSet, r.scheme); err == nil {
+				err = r.Create(context.TODO(), statefulSet)
+			}
 			// Find which labels the PVCs will be using, to use for the finalizer
 			pvcLabelSelector = statefulSet.Spec.Selector.MatchLabels
 		} else if err == nil {
 			statefulSetStatus = foundStatefulSet.Status
-			if util.CopyStatefulSetFields(statefulSet, foundStatefulSet, statefulSetLogger) {
-				// Update the found StatefulSet and write the result back if there are any changes
+			// Find which labels the PVCs will be using, to use for the finalizer
+			pvcLabelSelector = foundStatefulSet.Spec.Selector.MatchLabels
+
+			// Check to see if the StatefulSet needs an update
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(instance, foundStatefulSet, r.scheme)
+			needsUpdate = util.CopyStatefulSetFields(statefulSet, foundStatefulSet, statefulSetLogger) || needsUpdate
+
+			// Update the found StatefulSet and write the result back if there are any changes
+			if needsUpdate && err == nil {
 				statefulSetLogger.Info("Updating StatefulSet")
 				err = r.Update(context.TODO(), foundStatefulSet)
 			}
-			// Find which labels the PVCs will be using, to use for the finalizer
-			pvcLabelSelector = foundStatefulSet.Spec.Selector.MatchLabels
 		}
 		if err != nil {
 			return requeueOrNot, err
@@ -493,21 +513,26 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	if extAddressabilityOpts != nil && extAddressabilityOpts.Method == solr.Ingress {
 		// Generate Ingress
 		ingress := util.GenerateIngress(instance, solrNodeNames)
-		if err := controllerutil.SetControllerReference(instance, ingress, r.scheme); err != nil {
-			return requeueOrNot, err
-		}
 
 		// Check if the Ingress already exists
 		ingressLogger := logger.WithValues("ingress", ingress.Name)
 		foundIngress := &netv1.Ingress{}
 		err = r.Get(context.TODO(), types.NamespacedName{Name: ingress.Name, Namespace: ingress.Namespace}, foundIngress)
 		if err != nil && errors.IsNotFound(err) {
 			ingressLogger.Info("Creating Ingress")
-			err = r.Create(context.TODO(), ingress)
-		} else if err == nil && util.CopyIngressFields(ingress, foundIngress, ingressLogger) {
+			if err = controllerutil.SetControllerReference(instance, ingress, r.scheme); err == nil {
+				err = r.Create(context.TODO(), ingress)
+			}
+		} else if err == nil {
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(instance, foundIngress, r.scheme)
+			needsUpdate = util.CopyIngressFields(ingress, foundIngress, ingressLogger) || needsUpdate
+
 			// Update the found Ingress and write the result back if there are any changes
-			ingressLogger.Info("Updating Ingress")
-			err = r.Update(context.TODO(), foundIngress)
+			if needsUpdate && err == nil {
+				ingressLogger.Info("Updating Ingress")
+				err = r.Update(context.TODO(), foundIngress)
+			}
 		}
 		if err != nil {
 			return requeueOrNot, err
@@ -651,24 +676,29 @@ func reconcileCloudStatus(r *SolrCloudReconciler, solrCloud *solr.SolrCloud, new
 func reconcileNodeService(r *SolrCloudReconciler, logger logr.Logger, instance *solr.SolrCloud, nodeName string) (err error, ip string) {
 	// Generate Node Service
 	service := util.GenerateNodeService(instance, nodeName)
-	if err := controllerutil.SetControllerReference(instance, service, r.scheme); err != nil {
-		return err, ip
-	}
 
 	// Check if the Node Service already exists
 	nodeServiceLogger := logger.WithValues("service", service.Name)
 	foundService := &corev1.Service{}
 	err = r.Get(context.TODO(), types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, foundService)
 	if err != nil && errors.IsNotFound(err) {
 		nodeServiceLogger.Info("Creating Node Service")
-		err = r.Create(context.TODO(), service)
+		if err = controllerutil.SetControllerReference(instance, service, r.scheme); err == nil {
+			err = r.Create(context.TODO(), service)
+		}
 	} else if err == nil {
-		if util.CopyServiceFields(service, foundService, nodeServiceLogger) {
+		ip = foundService.Spec.ClusterIP
+
+		// Check to see if the Service needs an update
+		var needsUpdate bool
+		needsUpdate, err = util.OvertakeControllerRef(instance, foundService, r.scheme)
+		needsUpdate = util.CopyServiceFields(service, foundService, nodeServiceLogger) || needsUpdate
+
+		if needsUpdate && err == nil {
 			// Update the found Node service because there are differences between our version and the existing version
 			nodeServiceLogger.Info("Updating Node Service")
 			err = r.Update(context.TODO(), foundService)
 		}
-		ip = foundService.Spec.ClusterIP
 	}
 	if err != nil {
 		return err, ip
@@ -689,20 +719,23 @@ func reconcileZk(r *SolrCloudReconciler, logger logr.Logger, instance *solr.Solr
 			return errors.NewBadRequest("Cannot create a Zookeeper Cluster, as the Solr Operator is not configured to use the Zookeeper CRD")
 		}
 		zkCluster := util.GenerateZookeeperCluster(instance, pzk)
-		if err := controllerutil.SetControllerReference(instance, zkCluster, r.scheme); err != nil {
-			return err
-		}
 
 		// Check if the ZookeeperCluster already exists
 		zkLogger := logger.WithValues("zookeeperCluster", zkCluster.Name)
 		foundZkCluster := &zk.ZookeeperCluster{}
 		err := r.Get(context.TODO(), types.NamespacedName{Name: zkCluster.Name, Namespace: zkCluster.Namespace}, foundZkCluster)
 		if err != nil && errors.IsNotFound(err) {
 			zkLogger.Info("Creating Zookeeer Cluster")
-			err = r.Create(context.TODO(), zkCluster)
+			if err = controllerutil.SetControllerReference(instance, zkCluster, r.scheme); err == nil {
+				err = r.Create(context.TODO(), zkCluster)
+			}
 		} else if err == nil {
-			if util.CopyZookeeperClusterFields(zkCluster, foundZkCluster, zkLogger) {
-				// Update the found ZookeeperCluster and write the result back if there are any changes
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(instance, foundZkCluster, r.scheme)
+			needsUpdate = util.CopyZookeeperClusterFields(zkCluster, foundZkCluster, zkLogger) || needsUpdate
+
+			// Update the found ZookeeperCluster and write the result back if there are any changes
+			if needsUpdate && err == nil {
 				zkLogger.Info("Updating Zookeeer Cluster")
 				err = r.Update(context.TODO(), foundZkCluster)
 			}

controllers/solrprometheusexporter_controller.go

@@ -111,9 +111,6 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
 	if prometheusExporter.Spec.Config != "" {
 		// Generate ConfigMap
 		configMap := util.GenerateMetricsConfigMap(prometheusExporter)
-		if err := controllerutil.SetControllerReference(prometheusExporter, configMap, r.scheme); err != nil {
-			return ctrl.Result{}, err
-		}
 
 		// capture the MD5 for the default config XML, otherwise we already computed it above
 		if configXmlMd5 == "" {
@@ -126,11 +123,19 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
 		err = r.Get(context.TODO(), types.NamespacedName{Name: configMap.Name, Namespace: configMap.Namespace}, foundConfigMap)
 		if err != nil && errors.IsNotFound(err) {
 			configMapLogger.Info("Creating ConfigMap")
-			err = r.Create(context.TODO(), configMap)
-		} else if err == nil && util.CopyConfigMapFields(configMap, foundConfigMap, configMapLogger) {
+			if err = controllerutil.SetControllerReference(prometheusExporter, configMap, r.scheme); err == nil {
+				err = r.Create(context.TODO(), configMap)
+			}
+		} else if err == nil {
+			var needsUpdate bool
+			needsUpdate, err = util.OvertakeControllerRef(prometheusExporter, foundConfigMap, r.scheme)
+			needsUpdate = util.CopyConfigMapFields(configMap, foundConfigMap, configMapLogger) || needsUpdate
+
 			// Update the found ConfigMap and write the result back if there are any changes
-			configMapLogger.Info("Updating ConfigMap")
-			err = r.Update(context.TODO(), foundConfigMap)
+			if needsUpdate && err == nil {
+				configMapLogger.Info("Updating ConfigMap")
+				err = r.Update(context.TODO(), foundConfigMap)
+			}
 		}
 		if err != nil {
 			return ctrl.Result{}, err
@@ -139,21 +144,26 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
 
 	// Generate Metrics Service
 	metricsService := util.GenerateSolrMetricsService(prometheusExporter)
-	if err := controllerutil.SetControllerReference(prometheusExporter, metricsService, r.scheme); err != nil {
-		return ctrl.Result{}, err
-	}
 
 	// Check if the Metrics Service already exists
 	serviceLogger := logger.WithValues("service", metricsService.Name)
 	foundMetricsService := &corev1.Service{}
 	err = r.Get(context.TODO(), types.NamespacedName{Name: metricsService.Name, Namespace: metricsService.Namespace}, foundMetricsService)
 	if err != nil && errors.IsNotFound(err) {
 		serviceLogger.Info("Creating Service")
-		err = r.Create(context.TODO(), metricsService)
-	} else if err == nil && util.CopyServiceFields(metricsService, foundMetricsService, serviceLogger) {
+		if err = controllerutil.SetControllerReference(prometheusExporter, metricsService, r.scheme); err == nil {
+			err = r.Create(context.TODO(), metricsService)
+		}
+	} else if err == nil {
+		var needsUpdate bool
+		needsUpdate, err = util.OvertakeControllerRef(prometheusExporter, foundMetricsService, r.scheme)
+		needsUpdate = util.CopyServiceFields(metricsService, foundMetricsService, serviceLogger) || needsUpdate
+
 		// Update the found Metrics Service and write the result back if there are any changes
-		serviceLogger.Info("Updating Service")
-		err = r.Update(context.TODO(), foundMetricsService)
+		if needsUpdate && err == nil {
+			serviceLogger.Info("Updating Service")
+			err = r.Update(context.TODO(), foundMetricsService)
+		}
 	}
 	if err != nil {
 		return ctrl.Result{}, err
@@ -225,9 +235,6 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
 	}
 
 	deploy := util.GenerateSolrPrometheusExporterDeployment(prometheusExporter, solrConnectionInfo, configXmlMd5, tlsClientOptions, basicAuthMd5)
-	if err := controllerutil.SetControllerReference(prometheusExporter, deploy, r.scheme); err != nil {
-		return ctrl.Result{}, err
-	}
 
 	ready := false
 	// Check if the Metrics Deployment already exists
@@ -236,9 +243,16 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
 	err = r.Get(context.TODO(), types.NamespacedName{Name: deploy.Name, Namespace: deploy.Namespace}, foundDeploy)
 	if err != nil && errors.IsNotFound(err) {
 		deploymentLogger.Info("Creating Deployment")
-		err = r.Create(context.TODO(), deploy)
+		if err = controllerutil.SetControllerReference(prometheusExporter, deploy, r.scheme); err == nil {
+			err = r.Create(context.TODO(), deploy)
+		}
 	} else if err == nil {
-		if util.CopyDeploymentFields(deploy, foundDeploy, deploymentLogger) {
+		var needsUpdate bool
+		needsUpdate, err = util.OvertakeControllerRef(prometheusExporter, foundDeploy, r.scheme)
+		needsUpdate = util.CopyDeploymentFields(deploy, foundDeploy, deploymentLogger) || needsUpdate
+
+		// Update the found Metrics Service and write the result back if there are any changes
+		if needsUpdate && err == nil {
 			deploymentLogger.Info("Updating Deployment")
 			err = r.Update(context.TODO(), foundDeploy)
 		}

controllers/util/common.go

@@ -23,7 +23,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	netv1 "k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/utils/pointer"
 	"reflect"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"strconv"
 	"strings"
 )
@@ -536,3 +539,17 @@ func CopyPodContainers(fromPtr, toPtr *[]corev1.Container, basePath string, logg
 	}
 	return requireUpdate
 }
+
+// OvertakeControllerRef makes sure that the controlled object has the owner as the controller ref.
+// If the object has a different controller, then that ref will be downgraded to an "owner" and the new controller ref will be added
+func OvertakeControllerRef(owner metav1.Object, controlled metav1.Object, scheme *runtime.Scheme) (needsUpdate bool, err error) {
+	if !metav1.IsControlledBy(controlled, owner) {
+		if otherController := metav1.GetControllerOfNoCopy(controlled); otherController != nil {
+			otherController.Controller = pointer.BoolPtr(false)
+			otherController.BlockOwnerDeletion = pointer.BoolPtr(false)
+		}
+		err = controllerutil.SetControllerReference(owner, controlled, scheme)
+		needsUpdate = true
+	}
+	return needsUpdate, err
+}

go.mod

@@ -22,5 +22,6 @@ require (
 	k8s.io/apimachinery v0.19.0
 	k8s.io/client-go v0.19.0
 	k8s.io/klog/v2 v2.3.0 // indirect
+	k8s.io/utils v0.0.0-20200729134348-d5654de09c73
 	sigs.k8s.io/controller-runtime v0.6.2
 )