chore: minor clean-ups (#536)

* fix: handles err when checking managed RHODS

* chore: fixes typos and godoc convention

* chore: removes redundant pattern

* chore: removes dead code

* chore(deps): removes unused module

* chore: fixes godoc to use documented function name first

* chore: unifies receiver names

* chore: wraps error instead of constructing new one

* fix: removes rendundant _ exp from for loop

* fix: removes redundant import alias

* fix: removes unneeded string conversion

* chore: switches file operations from depracated ioutil to os pkg

* chore: swaps deprecated AddToScheme with Install

* chore: handles ignored errors

* Revert "chore: wraps error instead of constructing new one"

This reverts commit da574c4.

* chore: wraps errors using %w directive

* chore: simplifies component error handling using multierr

* chore: cleans up imports

* chore: update docs in pkg/deploy/deploy.go

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

---------

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

.gitignore

@@ -18,7 +18,6 @@ vendor/
 flymd.*
 
 # vim .swp files
-*~
 *.swp
 *.swo
 

README.md

@@ -31,7 +31,7 @@ and installed from source manually, see the Developer guide for further instruct
 
 ## Dev Preview
 
-Developer Preview of the new Open Data Hub operator codebase is now avaible.
+Developer Preview of the new Open Data Hub operator codebase is now available.
 Refer [Dev-Preview.md](./docs/Dev-Preview.md) for testing preview features.
 
 ### Developer Guide

apis/datasciencecluster/v1/datasciencecluster_types.go

@@ -30,7 +30,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches"
 )
 
-// Defines the desired state of DataScienceCluster
+// DataScienceCluster defines the desired state of the cluster.
 type DataScienceClusterSpec struct {
 	// Override and fine tune specific component configurations.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,order=1

components/README.md

@@ -39,7 +39,7 @@ can be found [here](https://github.com/opendatahub-io/opendatahub-operator/tree/
     ```
 ### Add reconcile and Events
 
-- Once you setup the new component module, add the component to [Reconcile](https://github.com/opendatahub-io/opendatahub-operator/blob/acaaf31f43e371456363f3fd272aec91ba413482/controllers/datasciencecluster/datasciencecluster_controller.go#L135) 
+- Once you set up the new component module, add the component to [Reconcile](https://github.com/opendatahub-io/opendatahub-operator/blob/acaaf31f43e371456363f3fd272aec91ba413482/controllers/datasciencecluster/datasciencecluster_controller.go#L135) 
   function in order to deploy manifests.
 - This will also enable/add status updates of the component in the operator.
 

components/codeflare/codeflare.go

@@ -41,8 +41,8 @@ func (c *CodeFlare) GetComponentName() string {
 // Verifies that CodeFlare implements ComponentInterface
 var _ components.ComponentInterface = (*CodeFlare)(nil)
 
-func (d *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsci.DSCInitializationSpec) error {
-	enabled := d.GetManagementState() == operatorv1.Managed
+func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsci.DSCInitializationSpec) error {
+	enabled := c.GetManagementState() == operatorv1.Managed
 
 	if enabled {
 		// check if the CodeFlare operator is installed
@@ -73,15 +73,15 @@ func (d *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 	}
 
 	// Special handling to delete MCAD InstaScale ImageStream resources
-	if d.GetManagementState() == operatorv1.Removed {
+	if c.GetManagementState() == operatorv1.Removed {
 		// Fetch the MCAD resource based on the request
 		mcad := &codeflarev1alpha1.MCAD{}
 		err := cli.Get(context.TODO(), client.ObjectKey{
 			Name: "mcad",
 		}, mcad)
 		if err != nil {
 			if apierrs.IsNotFound(err) {
-				return fmt.Errorf("failed to get MCAD instance mcad: %v", err)
+				return fmt.Errorf("failed to get MCAD instance mcad: %w", err)
 			}
 		}
 		err = cli.Delete(context.TODO(), mcad)
@@ -93,7 +93,7 @@ func (d *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 		}, instascale)
 		if err != nil {
 			if apierrs.IsNotFound(err) {
-				return fmt.Errorf("failed to get InstaScale instance instascale: %v", err)
+				return fmt.Errorf("failed to get InstaScale instance instascale: %w", err)
 			}
 		}
 		err = cli.Delete(context.TODO(), instascale)
@@ -105,14 +105,14 @@ func (d *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 		}, imagestream)
 		if err != nil {
 			if apierrs.IsNotFound(err) {
-				return fmt.Errorf("failed to get Imagestream instance codeflare-notebook: %v", err)
+				return fmt.Errorf("failed to get Imagestream instance codeflare-notebook: %w", err)
 			}
 		}
 		err = cli.Delete(context.TODO(), imagestream)
 	}
 
 	// Deploy Codeflare
-	err := deploy.DeployManifestsFromPath(owner, cli, d.GetComponentName(),
+	err := deploy.DeployManifestsFromPath(owner, cli, c.GetComponentName(),
 		CodeflarePath,
 		dscispec.ApplicationsNamespace,
 		cli.Scheme(), enabled)
@@ -121,7 +121,7 @@ func (d *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 
 }
 
-func (in *CodeFlare) DeepCopyInto(out *CodeFlare) {
-	*out = *in
-	out.Component = in.Component
+func (c *CodeFlare) DeepCopyInto(target *CodeFlare) {
+	*target = *c
+	target.Component = c.Component
 }

components/dashboard/dashboard.go

@@ -2,10 +2,9 @@
 package dashboard
 
 import (
+	"context"
 	"fmt"
 	operatorv1 "github.com/openshift/api/operator/v1"
-
-	"context"
 	"strings"
 
 	dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
@@ -101,7 +100,7 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 				dscispec.ApplicationsNamespace,
 				cli.Scheme(), enabled)
 			if err != nil {
-				return fmt.Errorf("failed to set dashboard config from %s: %v", PathODHDashboardConfig, err)
+				return fmt.Errorf("failed to set dashboard config from %s: %w", PathODHDashboardConfig, err)
 			}
 
 			// Apply modelserving config
@@ -110,20 +109,20 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 				dscispec.ApplicationsNamespace,
 				cli.Scheme(), enabled)
 			if err != nil {
-				return fmt.Errorf("failed to set dashboard OVMS from %s: %v", PathOVMS, err)
+				return fmt.Errorf("failed to set dashboard OVMS from %s: %w", PathOVMS, err)
 			}
 
 			// Apply anaconda config
 			err = common.CreateSecret(cli, "anaconda-ce-access", dscispec.ApplicationsNamespace)
 			if err != nil {
-				return fmt.Errorf("failed to create access-secret for anaconda: %v", err)
+				return fmt.Errorf("failed to create access-secret for anaconda: %w", err)
 			}
 			err = deploy.DeployManifestsFromPath(owner, cli, ComponentNameSupported,
 				PathAnaconda,
 				dscispec.ApplicationsNamespace,
 				cli.Scheme(), enabled)
 			if err != nil {
-				return fmt.Errorf("failed to deploy anaconda resources from %s: %v", PathAnaconda, err)
+				return fmt.Errorf("failed to deploy anaconda resources from %s: %w", PathAnaconda, err)
 			}
 		}
 
@@ -163,13 +162,13 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			dscispec.ApplicationsNamespace,
 			cli.Scheme(), enabled)
 		if err != nil {
-			return fmt.Errorf("failed to set dashboard ISV from %s: %v", PathISVSM, err)
+			return fmt.Errorf("failed to set dashboard ISV from %s: %w", PathISVSM, err)
 		}
 		// ConsoleLink handling
 		consoleRoute := &routev1.Route{}
 		err = cli.Get(context.TODO(), client.ObjectKey{Name: NameConsoleLink, Namespace: NamespaceConsoleLink}, consoleRoute)
 		if err != nil {
-			return fmt.Errorf("error getting console route URL : %v", err)
+			return fmt.Errorf("error getting console route URL : %w", err)
 		}
 		domainIndex := strings.Index(consoleRoute.Spec.Host, ".")
 		consolelinkDomain := consoleRoute.Spec.Host[domainIndex+1:]
@@ -178,14 +177,14 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			"<section-title>":       "OpenShift Self Managed Services",
 		})
 		if err != nil {
-			return fmt.Errorf("error replacing with correct dashboard url for ConsoleLink: %v", err)
+			return fmt.Errorf("error replacing with correct dashboard url for ConsoleLink: %w", err)
 		}
 		err = deploy.DeployManifestsFromPath(owner, cli, ComponentNameSupported,
 			PathConsoleLink,
 			dscispec.ApplicationsNamespace,
 			cli.Scheme(), enabled)
 		if err != nil {
-			return fmt.Errorf("failed to set dashboard consolelink from %s: %v", PathConsoleLink, err)
+			return fmt.Errorf("failed to set dashboard consolelink from %s: %w", PathConsoleLink, err)
 		}
 		return err
 	case deploy.ManagedRhods:
@@ -194,13 +193,13 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			dscispec.ApplicationsNamespace,
 			cli.Scheme(), enabled)
 		if err != nil {
-			return fmt.Errorf("failed to set dashboard ISV from %s: %v", PathISVAddOn, err)
+			return fmt.Errorf("failed to set dashboard ISV from %s: %w", PathISVAddOn, err)
 		}
 		// ConsoleLink handling
 		consoleRoute := &routev1.Route{}
 		err = cli.Get(context.TODO(), client.ObjectKey{Name: NameConsoleLink, Namespace: NamespaceConsoleLink}, consoleRoute)
 		if err != nil {
-			return fmt.Errorf("error getting console route URL : %v", err)
+			return fmt.Errorf("error getting console route URL : %w", err)
 		}
 		domainIndex := strings.Index(consoleRoute.Spec.Host, ".")
 		consolelinkDomain := consoleRoute.Spec.Host[domainIndex+1:]
@@ -209,7 +208,7 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d
 			"<section-title>":       "OpenShift Managed Services",
 		})
 		if err != nil {
-			return fmt.Errorf("Error replacing with correct dashboard url for ConsoleLink: %v", err)
+			return fmt.Errorf("failed replacing with correct dashboard url for ConsoleLink: %w", err)
 		}
 		err = deploy.DeployManifestsFromPath(owner, cli, ComponentNameSupported,
 			PathConsoleLink,

components/datasciencepipelines/datasciencepipelines.go

@@ -61,7 +61,7 @@ func (d *DataSciencePipelines) ReconcileComponent(cli client.Client, owner metav
 	return err
 }
 
-func (in *DataSciencePipelines) DeepCopyInto(out *DataSciencePipelines) {
-	*out = *in
-	out.Component = in.Component
+func (d *DataSciencePipelines) DeepCopyInto(target *DataSciencePipelines) {
+	*target = *d
+	target.Component = d.Component
 }

components/kserve/kserve.go

@@ -87,7 +87,7 @@ func (k *Kserve) ReconcileComponent(cli client.Client, owner metav1.Object, dsci
 		if err != nil {
 			return err
 		}
-		// Update image parameters for keserve
+		// Update image parameters for kserve
 		if dscispec.DevFlags.ManifestsUri == "" {
 			if err := deploy.ApplyImageParams(Path, dependentImageParamMap); err != nil {
 				return err
@@ -104,7 +104,7 @@ func (k *Kserve) ReconcileComponent(cli client.Client, owner metav1.Object, dsci
 	return nil
 }
 
-func (in *Kserve) DeepCopyInto(out *Kserve) {
-	*out = *in
-	out.Component = in.Component
+func (k *Kserve) DeepCopyInto(target *Kserve) {
+	*target = *k
+	target.Component = k.Component
 }

components/modelmeshserving/modelmeshserving.go

@@ -93,7 +93,7 @@ func (m *ModelMeshServing) ReconcileComponent(cli client.Client, owner metav1.Ob
 	return err
 }
 
-func (in *ModelMeshServing) DeepCopyInto(out *ModelMeshServing) {
-	*out = *in
-	out.Component = in.Component
+func (m *ModelMeshServing) DeepCopyInto(target *ModelMeshServing) {
+	*target = *m
+	target.Component = m.Component
 }

components/ray/ray.go

@@ -54,7 +54,7 @@ func (r *Ray) ReconcileComponent(cli client.Client, owner metav1.Object, dscispe
 
 }
 
-func (in *Ray) DeepCopyInto(out *Ray) {
-	*out = *in
-	out.Component = in.Component
+func (r *Ray) DeepCopyInto(target *Ray) {
+	*target = *r
+	target.Component = r.Component
 }

components/workbenches/workbenches.go

@@ -109,7 +109,7 @@ func (w *Workbenches) ReconcileComponent(cli client.Client, owner metav1.Object,
 
 }
 
-func (in *Workbenches) DeepCopyInto(out *Workbenches) {
-	*out = *in
-	out.Component = in.Component
+func (w *Workbenches) DeepCopyInto(target *Workbenches) {
+	*target = *w
+	target.Component = w.Component
 }

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -19,7 +19,9 @@ package datasciencecluster
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"github.com/hashicorp/go-multierror"
 	v1 "github.com/openshift/api/operator/v1"
 	"time"
 
@@ -33,13 +35,6 @@ import (
 	dsc "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
 	dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/components"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/codeflare"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/dashboard"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/datasciencepipelines"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/kserve"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/modelmeshserving"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/ray"
-	"github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches"
 	appsv1 "k8s.io/api/apps/v1"
 	netv1 "k8s.io/api/networking/v1"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -82,7 +77,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 
 	if len(instances.Items) > 1 {
 		message := fmt.Sprintf("only one instance of DataScienceCluster object is allowed. Update existing instance on namespace %s and name %s", req.Namespace, req.Name)
-		err := fmt.Errorf(message)
+		err := errors.New(message)
 		_ = r.reportError(err, &instances.Items[0], message)
 
 		return ctrl.Result{}, err
@@ -142,7 +137,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 		r.DataScienceCluster.DSCISpec.ApplicationsNamespace = dsciInstances.Items[0].Spec.ApplicationsNamespace
 		r.DataScienceCluster.DSCISpec.DevFlags.ManifestsUri = dsciInstances.Items[0].Spec.DevFlags.ManifestsUri
 	} else {
-		return ctrl.Result{}, fmt.Errorf(fmt.Sprintf("only one instance of DSCInitialization object is allowed."))
+		return ctrl.Result{}, errors.New("only one instance of DSCInitialization object is allowed")
 	}
 
 	// Ensure all omitted components show up as explicitly disabled
@@ -153,61 +148,61 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 	}
 
 	// Initialize error list, instead of returning errors after every component is deployed
-	componentErrorList := make(map[string]error)
+	var componentErrors *multierror.Error
 
 	// reconcile dashboard component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.Dashboard)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[dashboard.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile DataSciencePipelines component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.DataSciencePipelines)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[datasciencepipelines.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile Workbench component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.Workbenches)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[workbenches.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile Kserve component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.Kserve)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[kserve.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile ModelMesh component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.ModelMeshServing)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[modelmeshserving.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile CodeFlare component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.CodeFlare)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[codeflare.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// reconcile Ray component
 	if instance, err = r.reconcileSubComponent(ctx, instance, &(instance.Spec.Components.Ray)); err != nil {
 		// no need to log any errors as this is done in the reconcileSubComponent method
-		componentErrorList[ray.ComponentName] = err
+		componentErrors = multierror.Append(componentErrors, err)
 	}
 
 	// Process errors for components
-	if componentErrorList != nil && len(componentErrorList) != 0 {
+	if componentErrors != nil {
 		r.Log.Info("DataScienceCluster Deployment Incomplete.")
 		instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
 			status.SetCompleteCondition(&saved.Status.Conditions, status.ReconcileCompletedWithComponentErrors,
-				fmt.Sprintf("DataScienceCluster resource reconciled with component errors: %v", fmt.Sprint(componentErrorList)))
+				fmt.Sprintf("DataScienceCluster resource reconciled with component errors: %v", componentErrors))
 			saved.Status.Phase = status.PhaseReady
 		})
 		r.Recorder.Eventf(instance, corev1.EventTypeNormal, "DataScienceClusterComponentFailures",
-			"DataScienceCluster instance %s created, but have some failures in component %v", instance.Name, fmt.Sprint(componentErrorList))
-		return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf(fmt.Sprint(componentErrorList))
+			"DataScienceCluster instance %s created, but have some failures in component %v", instance.Name, componentErrors)
+		return ctrl.Result{RequeueAfter: time.Second * 10}, componentErrors
 	}
 
 	// finalize reconciliation
@@ -216,7 +211,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 		saved.Status.Phase = status.PhaseReady
 	})
 	if err != nil {
-		r.Log.Error(err, "failed to update DataScienceCluster conditions after successfuly completed reconciliation")
+		r.Log.Error(err, "failed to update DataScienceCluster conditions after successfully completed reconciliation")
 		return ctrl.Result{}, err
 	}