Refactor reconcilor so each component returns status

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

controllers/apiserver.go

@@ -39,50 +39,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 string, err error) {
 	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 "Skipped Application of APIServer Resources", nil
 	}
 
 	log.Info("Applying APIServer Resources")
-	err := r.ApplyDir(dsp, params, apiServerTemplatesDir)
+	err = r.ApplyDir(dsp, params, apiServerTemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply APIServer Resources", err
 	}
 
 	if dsp.Spec.APIServer.EnableRoute {
 		err := r.Apply(dsp, params, serverRoute)
 		if err != nil {
-			return err
+			return "Failed to apply APIServer route", err
 		}
 	} 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 "Failed to delete APIServer route", err
 		}
 	}
 
 	for cmName, template := range samplePipelineTemplates {
 		if dsp.Spec.APIServer.EnableSamplePipeline {
 			err := r.Apply(dsp, params, template)
 			if err != nil {
-				return err
+				return "Failed to apply sample pipeline", err
 			}
 		} 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 "Failed to delete sample pipeline", err
 			}
 		}
 	}
 
 	log.Info("Finished applying APIServer Resources")
-	return nil
+	return "APIServer Resources Applied", nil
 }

controllers/common.go

@@ -23,21 +23,21 @@ 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 string, err error) {
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	log.Info("Applying Common Resources")
-	err := r.ApplyDir(dsp, params, commonTemplatesDir)
+	err = r.ApplyDir(dsp, params, commonTemplatesDir)
 	if err != nil {
-		return err
+		return "Error Applying Common Resources", err
 	}
 	err = r.ApplyWithoutOwner(params, commonCusterRolebindingTemplate)
 	if err != nil {
-		return err
+		return "Error Applying clusterrolebinding", err
 	}
 
 	log.Info("Finished applying Common Resources")
-	return nil
+	return "Common Resources Applied", nil
 }
 
 func (r *DSPAReconciler) CleanUpCommon(params *DSPAParams) error {

controllers/dspipeline_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"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,55 +287,55 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	if dspaPrereqsReady {
 		// Manage Common Manifests
-		err = r.ReconcileCommon(dspa, params)
+		var statusMessage string
+		_, err = r.ReconcileCommon(dspa, params)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 
-		err = r.ReconcileAPIServer(ctx, dspa, params)
+		statusMessage, err = r.ReconcileAPIServer(ctx, dspa, params)
 		if err != nil {
 			r.setStatusAsNotReady(config.APIServerReady, err, dspaStatus.SetApiServerStatus)
 			return ctrl.Result{}, err
 		} else {
-			r.setStatus(ctx, params.APIServerDefaultResourceName, config.APIServerReady, dspa,
+			r.setStatus(ctx, params.APIServerDefaultResourceName, config.APIServerReady, statusMessage, dspa,
 				dspaStatus.SetApiServerStatus, log)
 		}
 
-		err = r.ReconcilePersistenceAgent(dspa, params)
+		statusMessage, err = r.ReconcilePersistenceAgent(dspa, params)
 		if err != nil {
 			r.setStatusAsNotReady(config.PersistenceAgentReady, err, dspaStatus.SetPersistenceAgentStatus)
 			return ctrl.Result{}, err
 		} else {
-			r.setStatus(ctx, params.PersistentAgentDefaultResourceName, config.PersistenceAgentReady, dspa,
+			r.setStatus(ctx, params.PersistentAgentDefaultResourceName, config.PersistenceAgentReady, statusMessage, dspa,
 				dspaStatus.SetPersistenceAgentStatus, log)
 		}
 
-		err = r.ReconcileScheduledWorkflow(dspa, params)
+		statusMessage, err = r.ReconcileScheduledWorkflow(dspa, params)
 		if err != nil {
 			r.setStatusAsNotReady(config.ScheduledWorkflowReady, err, dspaStatus.SetScheduledWorkflowStatus)
 			return ctrl.Result{}, err
 		} else {
-			r.setStatus(ctx, params.ScheduledWorkflowDefaultResourceName, config.ScheduledWorkflowReady, dspa,
+			r.setStatus(ctx, params.ScheduledWorkflowDefaultResourceName, config.ScheduledWorkflowReady, statusMessage, dspa,
 				dspaStatus.SetScheduledWorkflowStatus, log)
 		}
 
-		err = r.ReconcileUI(dspa, params)
+		_, err = r.ReconcileUI(dspa, params)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 
-		err = r.ReconcileWorkflowController(dspa, params)
+		_, err = r.ReconcileWorkflowController(dspa, params)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 
-		// 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)
+		statusMessage, err = r.ReconcileMLMD(ctx, dspa, params)
 		if err != nil {
 			r.setStatusAsNotReady(config.MLMDProxyReady, err, dspaStatus.SetMLMDProxyStatus)
 			return ctrl.Result{}, err
 		} else {
-			r.setStatus(ctx, params.MlmdProxyDefaultResourceName, config.MLMDProxyReady, dspa,
+			r.setStatus(ctx, params.MlmdProxyDefaultResourceName, config.MLMDProxyReady, statusMessage, dspa,
 				dspaStatus.SetMLMDProxyStatus, log)
 		}
 	}
@@ -374,10 +375,11 @@ func (r *DSPAReconciler) setStatusAsUnsupported(conditionType string, err error,
 	setStatus(condition)
 }
 
-func (r *DSPAReconciler) setStatus(ctx context.Context, resourceName string, conditionType string,
+func (r *DSPAReconciler) setStatus(ctx context.Context, resourceName string, conditionType string, statusMessage string,
 	dspa *dspav1.DataSciencePipelinesApplication, setStatus func(metav1.Condition),
 	log logr.Logger) {
 	condition, err := r.evaluateCondition(ctx, dspa, resourceName, conditionType)
+	condition.Message = statusMessage
 	setStatus(condition)
 	if err != nil {
 		log.Error(err, fmt.Sprintf("Encountered error when creating the %s readiness condition", conditionType))

controllers/dspipeline_controller_func_test.go

@@ -20,12 +20,13 @@ package controllers
 
 import (
 	"fmt"
+	"testing"
+
 	mfc "github.com/manifestival/controller-runtime-client"
 	mf "github.com/manifestival/manifestival"
 	util "github.com/opendatahub-io/data-science-pipelines-operator/controllers/testutil"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 var uc = util.UtilContext{}

controllers/mlmd.go

@@ -17,7 +17,7 @@ package controllers
 
 import (
 	"context"
-	"errors"
+
 	dspav1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1"
 )
 
@@ -29,47 +29,47 @@ const (
 )
 
 func (r *DSPAReconciler) ReconcileMLMD(ctx context.Context, dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status string, err error) {
 
 	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 "MLMD Resource Application Skipped", nil
 	}
 
 	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)
+	err = r.ApplyDir(dsp, params, mlmdTemplatesDir+"/"+mlmdGrpcService)
 	if err != nil {
-		return err
+		return "MLMD Service Failed to create", err
 	}
 
 	if params.PodToPodTLS {
 		var certificatesExist bool
 		certificatesExist, err = params.LoadMlmdCertificates(ctx, r.Client)
 		if err != nil {
-			return err
+			return "Failed to load MLMD Certificate", err
 		}
 
 		if !certificatesExist {
-			return errors.New("secret containing the certificate for MLMD gRPC Server was not created yet")
+			return "Secret containing the certificate for MLMD gRPC Server was not created yet", nil
 		}
 	}
 
 	err = r.ApplyDir(dsp, params, mlmdTemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply MLMD Resources", err
 	}
 
 	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 "Failed to apply MLMD Envoy Route", err
 		}
 	}
 
 	log.Info("Finished applying MLMD Resources")
-	return nil
+	return "MLMD Resources Applied", nil
 }

controllers/mlpipeline_ui.go

@@ -23,21 +23,21 @@ import (
 var mlPipelineUITemplatesDir = "mlpipelines-ui"
 
 func (r *DSPAReconciler) ReconcileUI(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status string, err error) {
 
 	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 "Skipped application of MlPipelineUI Resources", nil
 	}
 
 	log.Info("Applying MlPipelineUI Resources")
-	err := r.ApplyDir(dsp, params, mlPipelineUITemplatesDir)
+	err = r.ApplyDir(dsp, params, mlPipelineUITemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply MlPipelineUI Resources", err
 	}
 
 	log.Info("Finished applying MlPipelineUI Resources")
-	return nil
+	return "MlPipelineUI Resources Applied", nil
 }

controllers/persistence_agent.go

@@ -25,22 +25,22 @@ var persistenceAgentTemplatesDir = "persistence-agent"
 const persistenceAgentDefaultResourceNamePrefix = "ds-pipeline-persistenceagent-"
 
 func (r *DSPAReconciler) ReconcilePersistenceAgent(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status string, err error) {
 
 	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 "Skipped Application PersistenceAgent Resources", nil
 	}
 
 	log.Info("Applying PersistenceAgent Resources")
 
-	err := r.ApplyDir(dsp, params, persistenceAgentTemplatesDir)
+	err = r.ApplyDir(dsp, params, persistenceAgentTemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply PersistenceAgent Resources", err
 	}
 
 	log.Info("Finished applying PersistenceAgent Resources")
-	return nil
+	return "PersistenceAgent Resources Applied", nil
 }

controllers/scheduled_workflow.go

@@ -25,22 +25,22 @@ var scheduledWorkflowTemplatesDir = "scheduled-workflow"
 const scheduledWorkflowDefaultResourceNamePrefix = "ds-pipeline-scheduledworkflow-"
 
 func (r *DSPAReconciler) ReconcileScheduledWorkflow(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status string, err error) {
 
 	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 "Skipped Application of ScheduledWorkflow Resources", nil
 	}
 
 	log.Info("Applying ScheduledWorkflow Resources")
 
-	err := r.ApplyDir(dsp, params, scheduledWorkflowTemplatesDir)
+	err = r.ApplyDir(dsp, params, scheduledWorkflowTemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply ScheduledWorkflow Resources", err
 	}
 
 	log.Info("Finished applying ScheduledWorkflow Resources")
-	return nil
+	return "ScheduledWorkflow Resources Applied", nil
 }

controllers/workflow_controller.go

@@ -23,22 +23,22 @@ import (
 var workflowControllerTemplatesDir = "workflow-controller"
 
 func (r *DSPAReconciler) ReconcileWorkflowController(dsp *dspav1.DataSciencePipelinesApplication,
-	params *DSPAParams) error {
+	params *DSPAParams) (status string, err error) {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
 
 	if dsp.Spec.WorkflowController == nil || !dsp.Spec.WorkflowController.Deploy {
 		log.Info("Skipping Application of WorkflowController Resources")
-		return nil
+		return "Skipped application of WorkflowController Resources", nil
 	}
 
 	log.Info("Applying WorkflowController Resources")
 
-	err := r.ApplyDir(dsp, params, workflowControllerTemplatesDir)
+	err = r.ApplyDir(dsp, params, workflowControllerTemplatesDir)
 	if err != nil {
-		return err
+		return "Failed to apply WorkflowController Resources", err
 	}
 
 	log.Info("Finished applying WorkflowController Resources")
-	return nil
+	return "WorkflowController Resources Applied", nil
 }