Modify pulp resources reconciliation logic

closes: #1041

Author: git-hyagi

CHANGES/1041.misc

@@ -0,0 +1 @@
+Modified the way to check if a Pulp resource has changed.

controllers/deployment.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
 	"os"
 	"reflect"
 	"strconv"
@@ -112,6 +113,7 @@ func (d CommonDeployment) Deploy(resources any, pulpcoreType string) client.Obje
 		},
 	}
 
+	AddHashLabel(resources.(FunctionResources), dep)
 	// Set Pulp instance as the owner and controller
 	ctrl.SetControllerReference(pulp, dep, resources.(FunctionResources).Scheme)
 	return dep
@@ -1019,6 +1021,17 @@ func (d *CommonDeployment) setTelemetryConfig(resources any, pulpcoreType string
 	d.containers, d.volumes = telemetryConfig(resources, d.envVars, d.containers, d.volumes, pulpcoreType)
 }
 
+// AddHashLabel creates a label with the calculated hash from the mutated deployment
+func AddHashLabel(r FunctionResources, deployment *appsv1.Deployment) {
+	// if the object does not exist yet we need to mutate the object to get the
+	// default values (I think they are added by the admission controller)
+	if err := r.Create(context.TODO(), deployment, client.DryRunAll); err != nil {
+		SetHashLabel(HashFromMutated(deployment, r), deployment)
+	} else {
+		SetHashLabel(CalculateHash(deployment.Spec), deployment)
+	}
+}
+
 // build constructs the fields used in the deployment specification
 func (d *CommonDeployment) build(resources any, pulpcoreType string) {
 	pulp := resources.(FunctionResources).Pulp

controllers/ocp/deployment.go

@@ -50,6 +50,10 @@ func (d DeploymentAPIOCP) Deploy(resources controllers.FunctionResources) client
 	// get the current pulpcore-api common deployment definition
 	deployment := d.DeploymentAPICommon.Deploy(resources).(*appsv1.Deployment)
 	defaultsForOCPDeployment(deployment, resources.Pulp)
+
+	// update the hash label
+	controllers.AddHashLabel(resources, deployment)
+
 	return deployment
 }
 
@@ -64,6 +68,10 @@ func (d DeploymentContentOCP) Deploy(resources controllers.FunctionResources) cl
 	// get the current pulpcore-content common deployment definition
 	deployment := d.DeploymentContentCommon.Deploy(resources).(*appsv1.Deployment)
 	defaultsForOCPDeployment(deployment, resources.Pulp)
+
+	// update the hash label
+	controllers.AddHashLabel(resources, deployment)
+
 	return deployment
 }
 
@@ -78,5 +86,9 @@ func (d DeploymentWorkerOCP) Deploy(resources controllers.FunctionResources) cli
 	// get the current pulpcore-worker common deployment definition
 	deployment := d.DeploymentWorkerCommon.Deploy(resources).(*appsv1.Deployment)
 	defaultsForOCPDeployment(deployment, resources.Pulp)
+
+	// update the hash label
+	controllers.AddHashLabel(resources, deployment)
+
 	return deployment
 }

controllers/ocp/ingress.go

@@ -121,7 +121,7 @@ func deployOCPIngressRedirect(resources controllers.FunctionResources, plugins [
 	}
 
 	// Ensure ingress specs are as expected
-	if _, err := controllers.ReconcileObject(controllers.FunctionResources{Context: resources.Context, Client: resources.Client, Pulp: pulp, Scheme: resources.Scheme, Logger: log}, expectedIngress, currentIngress, conditionType); err != nil {
+	if _, err := controllers.ReconcileObject(controllers.FunctionResources{Context: resources.Context, Client: resources.Client, Pulp: pulp, Scheme: resources.Scheme, Logger: log}, expectedIngress, currentIngress, conditionType, controllers.PulpIngress{}); err != nil {
 		return err
 	}
 

controllers/ocp/route.go

@@ -177,7 +177,7 @@ func PulpRouteController(resources controllers.FunctionResources, restClient res
 			}
 
 			// Ensure route specs are as expected
-			if requeue, err := controllers.ReconcileObject(resources, expectedRoute, currentRoute, conditionType); err != nil || requeue {
+			if requeue, err := controllers.ReconcileObject(resources, expectedRoute, currentRoute, conditionType, controllers.PulpRoute{}); err != nil || requeue {
 				c <- statusReturn{ctrl.Result{Requeue: requeue}, err}
 				return
 			}

controllers/repo_manager/api.go

@@ -141,7 +141,7 @@ func (r *RepoManagerReconciler) pulpApiController(ctx context.Context, pulp *rep
 	apiDeployment := &appsv1.Deployment{}
 	r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-api", Namespace: pulp.Namespace}, apiDeployment)
 	expected := deploymentForPulpApi(funcResources)
-	if requeue, err := controllers.ReconcileObject(funcResources, expected, apiDeployment, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(funcResources, expected, apiDeployment, conditionType, controllers.PulpDeployment{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 
@@ -156,15 +156,15 @@ func (r *RepoManagerReconciler) pulpApiController(ctx context.Context, pulp *rep
 	apiSvc := &corev1.Service{}
 	r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-api-svc", Namespace: pulp.Namespace}, apiSvc)
 	expectedSvc := serviceForAPI(funcResources)
-	if requeue, err := controllers.ReconcileObject(funcResources, expectedSvc, apiSvc, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(funcResources, expectedSvc, apiSvc, conditionType, controllers.PulpService{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 
 	// Ensure the secret data is as expected
 	serverSecret := &corev1.Secret{}
 	r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-server", Namespace: pulp.Namespace}, serverSecret)
 	expectedServerSecret := pulpServerSecret(funcResources)
-	if requeue, err := controllers.ReconcileObject(funcResources, expectedServerSecret, serverSecret, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(funcResources, expectedServerSecret, serverSecret, conditionType, controllers.PulpSecret{}); err != nil || requeue {
 		log.Info("Reprovisioning pulpcore-api pods to get the new settings ...")
 		// when requeue==true it means the secret changed so we need to redeploy api and content pods to get the new settings.py
 		r.restartPods(pulp, apiDeployment)
@@ -182,15 +182,15 @@ func (r *RepoManagerReconciler) pulpApiController(ctx context.Context, pulp *rep
 		telemetryConfigMap := &corev1.ConfigMap{}
 		r.Get(ctx, types.NamespacedName{Name: controllers.OtelConfigName, Namespace: pulp.Namespace}, telemetryConfigMap)
 		expectedTelemetryConfigMap := controllers.OtelConfigMap(funcResources)
-		if requeue, err := controllers.ReconcileObject(funcResources, expectedTelemetryConfigMap, telemetryConfigMap, conditionType); err != nil || requeue {
+		if requeue, err := controllers.ReconcileObject(funcResources, expectedTelemetryConfigMap, telemetryConfigMap, conditionType, controllers.PulpConfigMap{}); err != nil || requeue {
 			return ctrl.Result{Requeue: requeue}, err
 		}
 
 		// Ensure otelService is as expected
 		telemetryService := &corev1.Service{}
 		r.Get(ctx, types.NamespacedName{Name: controllers.OtelServiceName, Namespace: pulp.Namespace}, telemetryService)
 		expectedTelemetryService := controllers.ServiceOtel(funcResources)
-		if requeue, err := controllers.ReconcileObject(funcResources, expectedTelemetryService, telemetryService, conditionType); err != nil || requeue {
+		if requeue, err := controllers.ReconcileObject(funcResources, expectedTelemetryService, telemetryService, conditionType, controllers.PulpService{}); err != nil || requeue {
 			return ctrl.Result{Requeue: requeue}, err
 		}
 	}

controllers/repo_manager/content.go

@@ -70,15 +70,15 @@ func (r *RepoManagerReconciler) pulpContentController(ctx context.Context, pulp
 	deployment := &appsv1.Deployment{}
 	r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-content", Namespace: pulp.Namespace}, deployment)
 	expected := deploymentForPulpContent(funcResources)
-	if requeue, err := controllers.ReconcileObject(funcResources, expected, deployment, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(funcResources, expected, deployment, conditionType, controllers.PulpDeployment{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 
 	// Reconcile Service
 	cntSvc := &corev1.Service{}
 	r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-content-svc", Namespace: pulp.Namespace}, cntSvc)
 	newCntSvc := serviceForContent(funcResources)
-	if requeue, err := controllers.ReconcileObject(funcResources, newCntSvc, cntSvc, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(funcResources, newCntSvc, cntSvc, conditionType, controllers.PulpService{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 

controllers/repo_manager/ingress.go

@@ -146,7 +146,7 @@ func (r *RepoManagerReconciler) pulpIngressController(ctx context.Context, pulp
 	}
 
 	// Ensure ingress specs are as expected
-	if requeue, err := controllers.ReconcileObject(controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}, expectedIngress, currentIngress, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}, expectedIngress, currentIngress, conditionType, controllers.PulpIngress{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 

controllers/repo_manager/redis.go

@@ -8,6 +8,8 @@ import (
 	"github.com/go-logr/logr"
 	repomanagerpulpprojectorgv1beta2 "github.com/pulp/pulp-operator/apis/repo-manager.pulpproject.org/v1beta2"
 	"github.com/pulp/pulp-operator/controllers"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -21,6 +23,10 @@ import (
 
 func (r *RepoManagerReconciler) pulpCacheController(ctx context.Context, pulp *repomanagerpulpprojectorgv1beta2.Pulp, log logr.Logger) (ctrl.Result, error) {
 
+	// conditionType is used to update .status.conditions with the current resource state
+	conditionType := cases.Title(language.English, cases.Compact).String(pulp.Spec.DeploymentType) + "-API-Ready"
+	funcResources := controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}
+
 	// pulp-redis-data PVC
 	// the PVC will be created only if a StorageClassName is provided
 	if _, storageType := controllers.MultiStorageConfigured(pulp, "Cache"); storageType[0] == controllers.SCNameType {
@@ -99,9 +105,8 @@ func (r *RepoManagerReconciler) pulpCacheController(ctx context.Context, pulp *r
 	// redis Deployment
 	deploymentFound := &appsv1.Deployment{}
 	err = r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-redis", Namespace: pulp.Namespace}, deploymentFound)
-	dep := redisDeployment(pulp)
+	dep := redisDeployment(pulp, funcResources)
 	if err != nil && errors.IsNotFound(err) {
-		ctrl.SetControllerReference(pulp, dep, r.Scheme)
 		log.Info("Creating a new Pulp Redis Deployment", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name)
 		err = r.Create(ctx, dep)
 		if err != nil {
@@ -117,19 +122,9 @@ func (r *RepoManagerReconciler) pulpCacheController(ctx context.Context, pulp *r
 		return ctrl.Result{}, err
 	}
 
-	// Reconcile Deployment
-	if controllers.CheckDeploymentSpec(dep.Spec, deploymentFound.Spec) {
-		log.Info("The Redis Deployment has been modified! Reconciling ...")
-		ctrl.SetControllerReference(pulp, dep, r.Scheme)
-		r.recorder.Event(pulp, corev1.EventTypeNormal, "Updating", "Reconciling Redis Deployment")
-		err = r.Update(ctx, dep)
-		if err != nil {
-			log.Error(err, "Error trying to update the Redis Deployment object ... ")
-			r.recorder.Event(pulp, corev1.EventTypeWarning, "Failed", "Failed to reconcile Redis Deployment")
-			return ctrl.Result{}, err
-		}
-		r.recorder.Event(pulp, corev1.EventTypeNormal, "Updated", "Redis Deployment reconciled")
-		return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, nil
+	// Ensure the deployment spec is as expected
+	if requeue, err := controllers.ReconcileObject(funcResources, dep, deploymentFound, conditionType, controllers.PulpDeployment{}); err != nil || requeue {
+		return ctrl.Result{Requeue: requeue}, err
 	}
 
 	r.recorder.Event(pulp, corev1.EventTypeNormal, "RedisReady", "All Redis tasks ran successfully")
@@ -220,7 +215,7 @@ func redisSvc(m *repomanagerpulpprojectorgv1beta2.Pulp) *corev1.Service {
 }
 
 // redisDeployment returns a Redis Deployment object
-func redisDeployment(m *repomanagerpulpprojectorgv1beta2.Pulp) *appsv1.Deployment {
+func redisDeployment(m *repomanagerpulpprojectorgv1beta2.Pulp, funcResources controllers.FunctionResources) *appsv1.Deployment {
 
 	replicas := int32(1)
 
@@ -359,7 +354,7 @@ func redisDeployment(m *repomanagerpulpprojectorgv1beta2.Pulp) *appsv1.Deploymen
 	removeStorageDefinition(&resources)
 
 	// deployment definition
-	return &appsv1.Deployment{
+	dep := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      m.Name + "-redis",
 			Namespace: m.Namespace,
@@ -422,6 +417,10 @@ func redisDeployment(m *repomanagerpulpprojectorgv1beta2.Pulp) *appsv1.Deploymen
 			},
 		},
 	}
+
+	controllers.AddHashLabel(funcResources, dep)
+	ctrl.SetControllerReference(m, dep, funcResources.Scheme)
+	return dep
 }
 
 // removeStorageDefinition ensures that no storage definition is present in resourceRequirements

controllers/repo_manager/web.go

@@ -37,6 +37,7 @@ import (
 )
 
 func (r *RepoManagerReconciler) pulpWebController(ctx context.Context, pulp *repomanagerpulpprojectorgv1beta2.Pulp, log logr.Logger) (ctrl.Result, error) {
+	funcResources := controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}
 
 	// conditionType is used to update .status.conditions with the current resource state
 	conditionType := cases.Title(language.English, cases.Compact).String(pulp.Spec.DeploymentType) + "-Web-Ready"
@@ -66,7 +67,7 @@ func (r *RepoManagerReconciler) pulpWebController(ctx context.Context, pulp *rep
 	// pulp-web Deployment
 	webDeployment := &appsv1.Deployment{}
 	err = r.Get(ctx, types.NamespacedName{Name: pulp.Name + "-web", Namespace: pulp.Namespace}, webDeployment)
-	newWebDeployment := r.deploymentForPulpWeb(pulp)
+	newWebDeployment := r.deploymentForPulpWeb(pulp, funcResources)
 	if err != nil && errors.IsNotFound(err) {
 		log.Info("Creating a new Pulp Web Deployment", "Deployment.Namespace", newWebDeployment.Namespace, "Deployment.Name", newWebDeployment.Name)
 		controllers.UpdateStatus(ctx, r.Client, pulp, metav1.ConditionFalse, conditionType, "CreatingWebDeployment", "Creating "+pulp.Name+"-web deployment resource")
@@ -86,7 +87,7 @@ func (r *RepoManagerReconciler) pulpWebController(ctx context.Context, pulp *rep
 	}
 
 	// Reconcile Deployment
-	if controllers.CheckDeploymentSpec(newWebDeployment.Spec, webDeployment.Spec) {
+	if controllers.CheckDeploymentSpec(*newWebDeployment, *webDeployment, funcResources) {
 		log.Info("The Web Deployment has been modified! Reconciling ...")
 		controllers.UpdateStatus(ctx, r.Client, pulp, metav1.ConditionFalse, conditionType, "UpdatingWebDeployment", "Reconciling "+pulp.Name+"-web deployment resource")
 		r.recorder.Event(pulp, corev1.EventTypeNormal, "Updating", "Reconciling Web Deployment")
@@ -125,15 +126,15 @@ func (r *RepoManagerReconciler) pulpWebController(ctx context.Context, pulp *rep
 	}
 
 	// Reconcile Service
-	if requeue, err := controllers.ReconcileObject(controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}, newWebSvc, webSvc, conditionType); err != nil || requeue {
+	if requeue, err := controllers.ReconcileObject(controllers.FunctionResources{Context: ctx, Client: r.Client, Pulp: pulp, Scheme: r.Scheme, Logger: log}, newWebSvc, webSvc, conditionType, controllers.PulpService{}); err != nil || requeue {
 		return ctrl.Result{Requeue: requeue}, err
 	}
 
 	return ctrl.Result{}, nil
 }
 
 // deploymentForPulpWeb returns a pulp-web Deployment object
-func (r *RepoManagerReconciler) deploymentForPulpWeb(m *repomanagerpulpprojectorgv1beta2.Pulp) *appsv1.Deployment {
+func (r *RepoManagerReconciler) deploymentForPulpWeb(m *repomanagerpulpprojectorgv1beta2.Pulp, funcResources controllers.FunctionResources) *appsv1.Deployment {
 
 	ls := labelsForPulpWeb(m)
 	replicas := m.Spec.Web.Replicas
@@ -257,6 +258,8 @@ func (r *RepoManagerReconciler) deploymentForPulpWeb(m *repomanagerpulpprojector
 			},
 		},
 	}
+
+	controllers.AddHashLabel(funcResources, dep)
 	// Set Pulp instance as the owner and controller
 	ctrl.SetControllerReference(m, dep, r.Scheme)
 	return dep