Refactor reconcilor so each component is returning status

Signed-off-by: Anish Asthana <[email protected]>

controllers/apiserver.go

@@ -21,6 +21,7 @@ import (
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
 	v1 "github.com/openshift/api/route/v1"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -39,50 +40,50 @@ var samplePipelineTemplates = map[string]string{
 	"sample-config":   "apiserver/sample-pipeline/sample-config.yaml.tmpl",
 }
 
-func (r *DSPAReconciler) ReconcileAPIServer(ctx context.Context, dsp *dspav1.DataSciencePipelinesApplication, params *DSPAParams) error {
+func (r *DSPAReconciler) ReconcileAPIServer(ctx context.Context, dsp *dspav1.DataSciencePipelinesApplication, params *DSPAParams) (status metav1.ConditionStatus, description string) {
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if !dsp.Spec.APIServer.Deploy {
 		r.Log.Info("Skipping Application of APIServer Resources")
-		return nil
+		return metav1.ConditionTrue, "APIServer Application Skipped"
 	}
 
 	log.Info("Applying APIServer Resources")
 	err := r.ApplyDir(dsp, params, apiServerTemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	if dsp.Spec.APIServer.EnableRoute {
 		err := r.Apply(dsp, params, serverRoute)
 		if err != nil {
-			return err
+			return metav1.ConditionFalse, err.Error()
 		}
 	} else {
 		route := &v1.Route{}
 		namespacedNamed := types.NamespacedName{Name: "ds-pipeline-" + dsp.Name, Namespace: dsp.Namespace}
 		err := r.DeleteResourceIfItExists(ctx, route, namespacedNamed)
 		if err != nil {
-			return err
+			return metav1.ConditionFalse, err.Error()
 		}
 	}
 
 	for cmName, template := range samplePipelineTemplates {
 		if dsp.Spec.APIServer.EnableSamplePipeline {
 			err := r.Apply(dsp, params, template)
 			if err != nil {
-				return err
+				return metav1.ConditionFalse, err.Error()
 			}
 		} else {
 			cm := &corev1.ConfigMap{}
 			namespacedNamed := types.NamespacedName{Name: cmName + "-" + dsp.Name, Namespace: dsp.Namespace}
 			err := r.DeleteResourceIfItExists(ctx, cm, namespacedNamed)
 			if err != nil {
-				return err
+				return metav1.ConditionFalse, err.Error()
 			}
 		}
 	}
 
 	log.Info("Finished applying APIServer Resources")
-	return nil
+	return metav1.ConditionTrue, "APIServer Resources Applied"
 }

controllers/common.go

@@ -17,27 +17,28 @@ package controllers
 
 import (
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var commonTemplatesDir = "common/default"
 
 const commonCusterRolebindingTemplate = "common/no-owner/clusterrolebinding.yaml.tmpl"
 
-func (r *DSPAReconciler) ReconcileCommon(dsp *dspav1.DataSciencePipelinesApplication, params *DSPAParams) error {
+func (r *DSPAReconciler) ReconcileCommon(dsp *dspav1.DataSciencePipelinesApplication, params *DSPAParams) (status metav1.ConditionStatus, description string) {
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	log.Info("Applying Common Resources")
 	err := r.ApplyDir(dsp, params, commonTemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 	err = r.ApplyWithoutOwner(params, commonCusterRolebindingTemplate)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	log.Info("Finished applying Common Resources")
-	return nil
+	return metav1.ConditionTrue, "Common Resources Applied"
 }
 
 func (r *DSPAReconciler) CleanUpCommon(params *DSPAParams) error {

controllers/dspipeline_controller.go

@@ -18,7 +18,9 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
+
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/dspastatus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -286,53 +288,53 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	if dspaPrereqsReady {
 		// Manage Common Manifests
-		err = r.ReconcileCommon(dspa, params)
-		if err != nil {
-			return ctrl.Result{}, err
+		status, description := r.ReconcileCommon(dspa, params)
+		if status != metav1.ConditionTrue {
+			return ctrl.Result{}, errors.New(description)
 		}
 
-		err = r.ReconcileAPIServer(ctx, dspa, params)
-		if err != nil {
-			r.setStatusAsNotReady(config.APIServerReady, err, dspaStatus.SetApiServerStatus)
-			return ctrl.Result{}, err
+		status, description = r.ReconcileAPIServer(ctx, dspa, params)
+		if status != metav1.ConditionTrue {
+			r.setStatusAsNotReady(config.APIServerReady, description, dspaStatus.SetApiServerStatus)
+			return ctrl.Result{}, errors.New(description)
 		} else {
 			r.setStatus(ctx, params.APIServerDefaultResourceName, config.APIServerReady, dspa,
 				dspaStatus.SetApiServerStatus, log)
 		}
 
-		err = r.ReconcilePersistenceAgent(dspa, params)
-		if err != nil {
-			r.setStatusAsNotReady(config.PersistenceAgentReady, err, dspaStatus.SetPersistenceAgentStatus)
-			return ctrl.Result{}, err
+		status, description = r.ReconcilePersistenceAgent(dspa, params)
+		if status != metav1.ConditionTrue {
+			r.setStatusAsNotReady(config.PersistenceAgentReady, description, dspaStatus.SetPersistenceAgentStatus)
+			return ctrl.Result{}, errors.New(description)
 		} else {
 			r.setStatus(ctx, params.PersistentAgentDefaultResourceName, config.PersistenceAgentReady, dspa,
 				dspaStatus.SetPersistenceAgentStatus, log)
 		}
 
-		err = r.ReconcileScheduledWorkflow(dspa, params)
-		if err != nil {
-			r.setStatusAsNotReady(config.ScheduledWorkflowReady, err, dspaStatus.SetScheduledWorkflowStatus)
-			return ctrl.Result{}, err
+		status, description = r.ReconcileScheduledWorkflow(dspa, params)
+		if status != metav1.ConditionTrue {
+			r.setStatusAsNotReady(config.ScheduledWorkflowReady, description, dspaStatus.SetScheduledWorkflowStatus)
+			return ctrl.Result{}, errors.New(description)
 		} else {
 			r.setStatus(ctx, params.ScheduledWorkflowDefaultResourceName, config.ScheduledWorkflowReady, dspa,
 				dspaStatus.SetScheduledWorkflowStatus, log)
 		}
 
-		err = r.ReconcileUI(dspa, params)
-		if err != nil {
-			return ctrl.Result{}, err
+		status, description = r.ReconcileUI(dspa, params)
+		if status != metav1.ConditionTrue {
+			return ctrl.Result{}, errors.New(description)
 		}
 
-		err = r.ReconcileWorkflowController(dspa, params)
-		if err != nil {
-			return ctrl.Result{}, err
+		status, description = r.ReconcileWorkflowController(dspa, params)
+		if status != metav1.ConditionTrue {
+			return ctrl.Result{}, errors.New(description)
 		}
 
 		// MLMD should be the last to reconcile because it can cause an early exit due to the lack of the TLS secret, which may not have been created yet.
-		err = r.ReconcileMLMD(ctx, dspa, params)
-		if err != nil {
-			r.setStatusAsNotReady(config.MLMDProxyReady, err, dspaStatus.SetMLMDProxyStatus)
-			return ctrl.Result{}, err
+		status, description = r.ReconcileMLMD(ctx, dspa, params)
+		if status != metav1.ConditionTrue {
+			r.setStatusAsNotReady(config.MLMDProxyReady, description, dspaStatus.SetMLMDProxyStatus)
+			return ctrl.Result{}, errors.New(description)
 		} else {
 			r.setStatus(ctx, params.MlmdProxyDefaultResourceName, config.MLMDProxyReady, dspa,
 				dspaStatus.SetMLMDProxyStatus, log)
@@ -364,8 +366,8 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	return ctrl.Result{}, nil
 }
 
-func (r *DSPAReconciler) setStatusAsNotReady(conditionType string, err error, setStatus func(metav1.Condition)) {
-	condition := dspastatus.BuildFalseCondition(conditionType, config.FailingToDeploy, err.Error())
+func (r *DSPAReconciler) setStatusAsNotReady(conditionType string, description string, setStatus func(metav1.Condition)) {
+	condition := dspastatus.BuildFalseCondition(conditionType, config.FailingToDeploy, description)
 	setStatus(condition)
 }
 

controllers/mlmd.go

@@ -17,8 +17,9 @@ package controllers
 
 import (
 	"context"
-	"errors"
+
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
@@ -29,47 +30,47 @@ const (
 )
 
 func (r *DSPAReconciler) ReconcileMLMD(ctx context.Context, dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status metav1.ConditionStatus, description string) {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if (params.MLMD == nil || !params.MLMD.Deploy) && (dsp.Spec.MLMD == nil || !dsp.Spec.MLMD.Deploy) {
 		r.Log.Info("Skipping Application of ML-Metadata (MLMD) Resources")
-		return nil
+		return metav1.ConditionTrue, "ML-Metadata (MLMD) Resources Application Skipped"
 	}
 
 	log.Info("Applying ML-Metadata (MLMD) Resources")
 
 	// We need to create the service first so OpenShift creates the certificate that we'll use later.
 	err := r.ApplyDir(dsp, params, mlmdTemplatesDir+"/"+mlmdGrpcService)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	if params.PodToPodTLS {
 		var certificatesExist bool
 		certificatesExist, err = params.LoadMlmdCertificates(ctx, r.Client)
 		if err != nil {
-			return err
+			return metav1.ConditionFalse, err.Error()
 		}
 
 		if !certificatesExist {
-			return errors.New("secret containing the certificate for MLMD gRPC Server was not created yet")
+			return metav1.ConditionFalse, "secret containing the certificate for MLMD gRPC Server was not created yet"
 		}
 	}
 
 	err = r.ApplyDir(dsp, params, mlmdTemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	if dsp.Spec.MLMD == nil || dsp.Spec.MLMD.Envoy == nil || dsp.Spec.MLMD.Envoy.DeployRoute {
 		err = r.Apply(dsp, params, mlmdEnvoyRoute)
 		if err != nil {
-			return err
+			return metav1.ConditionFalse, err.Error()
 		}
 	}
 
 	log.Info("Finished applying MLMD Resources")
-	return nil
+	return metav1.ConditionTrue, "MLMD Resources Applied"
 }

controllers/mlpipeline_ui.go

@@ -18,26 +18,27 @@ package controllers
 
 import (
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var mlPipelineUITemplatesDir = "mlpipelines-ui"
 
 func (r *DSPAReconciler) ReconcileUI(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status metav1.ConditionStatus, description string) {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if dsp.Spec.MlPipelineUI == nil || !dsp.Spec.MlPipelineUI.Deploy {
 		log.Info("Skipping Application of MlPipelineUI Resources")
-		return nil
+		return metav1.ConditionTrue, "MLPipelineUI Application Skipped"
 	}
 
 	log.Info("Applying MlPipelineUI Resources")
 	err := r.ApplyDir(dsp, params, mlPipelineUITemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	log.Info("Finished applying MlPipelineUI Resources")
-	return nil
+	return metav1.ConditionTrue, "MLPipelineUI Resources Applied"
 }

controllers/persistence_agent.go

@@ -18,29 +18,30 @@ package controllers
 
 import (
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var persistenceAgentTemplatesDir = "persistence-agent"
 
 const persistenceAgentDefaultResourceNamePrefix = "ds-pipeline-persistenceagent-"
 
 func (r *DSPAReconciler) ReconcilePersistenceAgent(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status metav1.ConditionStatus, description string) {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if !dsp.Spec.PersistenceAgent.Deploy {
 		log.Info("Skipping Application of PersistenceAgent Resources")
-		return nil
+		return metav1.ConditionTrue, "PersistenceAgent Application Skipped"
 	}
 
 	log.Info("Applying PersistenceAgent Resources")
 
 	err := r.ApplyDir(dsp, params, persistenceAgentTemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	log.Info("Finished applying PersistenceAgent Resources")
-	return nil
+	return metav1.ConditionTrue, "PersistenceAgent Resources Applied"
 }

controllers/scheduled_workflow.go

@@ -18,29 +18,30 @@ package controllers
 
 import (
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var scheduledWorkflowTemplatesDir = "scheduled-workflow"
 
 const scheduledWorkflowDefaultResourceNamePrefix = "ds-pipeline-scheduledworkflow-"
 
 func (r *DSPAReconciler) ReconcileScheduledWorkflow(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status metav1.ConditionStatus, description string) {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if !dsp.Spec.ScheduledWorkflow.Deploy {
 		log.Info("Skipping Application of ScheduledWorkflow Resources")
-		return nil
+		return metav1.ConditionTrue, "ScheduledWorkflow Application Skipped"
 	}
 
 	log.Info("Applying ScheduledWorkflow Resources")
 
 	err := r.ApplyDir(dsp, params, scheduledWorkflowTemplatesDir)
 	if err != nil {
-		return err
+		return metav1.ConditionFalse, err.Error()
 	}
 
 	log.Info("Finished applying ScheduledWorkflow Resources")
-	return nil
+	return metav1.ConditionTrue, "ScheduledWorkflow Resources Applied"
 }