chore(lint): enable contextcheck and containedctx (opendatahub-io#1070)

* chore(lint): enable contextcheck Signed-off-by: Luca Burgazzoli <[email protected]> * chore(lint): enable containedctx Signed-off-by: Luca Burgazzoli <[email protected]> * Fix PR review findings * Fix rebase --------- Signed-off-by: Luca Burgazzoli <[email protected]> (cherry picked from commit 47ea7d1)
red-hat-data-services · Jul 24, 2024 · 06e21a4 · 06e21a4
1 parent 4578d74
commit 06e21a4
Show file tree

Hide file tree

Showing 59 changed files with 797 additions and 595 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -55,7 +55,6 @@ linters-settings:
 linters:
   enable-all: true
   disable:
-    - containedctx # detects struct contained context.Context field
     - depguard # [replaced by gomodguard] checks if package imports are in a list of acceptable packages
     - exhaustruct # Prevents empty struct. We use a lot of these so I think it is safe to disable.c
     - forbidigo
@@ -75,7 +74,6 @@ linters:
     # Need to check
     - nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity
     - err113 # [too strict] checks the errors handling expressions
-    - contextcheck # Requires to pass context to all function.
 
     # To be fixed
     - gocognit # https://github.com/opendatahub-io/opendatahub-operator/issues/709

diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go
@@ -35,11 +35,11 @@ type CodeFlare struct {
 	components.Component `json:""`
 }
 
-func (c *CodeFlare) OverrideManifests(_ string) error {
+func (c *CodeFlare) OverrideManifests(ctx context.Context, _ string) error {
 	// If devflags are set, update default manifests path
 	if len(c.DevFlags.Manifests) != 0 {
 		manifestConfig := c.DevFlags.Manifests[0]
-		if err := deploy.DownloadManifests(ComponentName, manifestConfig); err != nil {
+		if err := deploy.DownloadManifests(ctx, ComponentName, manifestConfig); err != nil {
 			return err
 		}
 		// If overlay is defined, update paths
@@ -76,15 +76,15 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	if enabled {
 		if c.DevFlags != nil {
 			// Download manifests and update paths
-			if err := c.OverrideManifests(string(platform)); err != nil {
+			if err := c.OverrideManifests(ctx, string(platform)); err != nil {
 				return err
 			}
 		}
 		// check if the CodeFlare operator is installed: it should not be installed
 		// Both ODH and RHOAI should have the same operator name
 		dependentOperator := CodeflareOperator
 
-		if found, err := cluster.OperatorExists(cli, dependentOperator); err != nil {
+		if found, err := cluster.OperatorExists(ctx, cli, dependentOperator); err != nil {
 			return fmt.Errorf("operator exists throws error %w", err)
 		} else if found {
 			return fmt.Errorf("operator %s is found. Please uninstall the operator before enabling %s component",
@@ -100,7 +100,7 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	}
 
 	// Deploy Codeflare
-	if err := deploy.DeployManifestsFromPath(cli, owner, //nolint:revive,nolintlint
+	if err := deploy.DeployManifestsFromPath(ctx, cli, owner, //nolint:revive,nolintlint
 		CodeflarePath,
 		dscispec.ApplicationsNamespace,
 		ComponentName, enabled); err != nil {
@@ -121,7 +121,7 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 		if err := c.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
 			return err
 		}
-		if err := deploy.DeployManifestsFromPath(cli, owner,
+		if err := deploy.DeployManifestsFromPath(ctx, cli, owner,
 			filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
 			dscispec.Monitoring.Namespace,
 			"prometheus", true); err != nil {

diff --git a/components/component.go b/components/component.go
@@ -44,11 +44,7 @@ func (c *Component) GetManagementState() operatorv1.ManagementState {
 	return c.ManagementState
 }
 
-func (c *Component) SetImageParamsMap(imageMap map[string]string) map[string]string {
-	return imageMap
-}
-
-func (c *Component) Cleanup(_ client.Client, _ *dsciv1.DSCInitializationSpec) error {
+func (c *Component) Cleanup(_ context.Context, _ client.Client, _ *dsciv1.DSCInitializationSpec) error {
 	// noop
 	return nil
 }
@@ -85,10 +81,10 @@ type ManifestsConfig struct {
 type ComponentInterface interface {
 	ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
 		owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error
-	Cleanup(cli client.Client, DSCISpec *dsciv1.DSCInitializationSpec) error
+	Cleanup(ctx context.Context, cli client.Client, DSCISpec *dsciv1.DSCInitializationSpec) error
 	GetComponentName() string
 	GetManagementState() operatorv1.ManagementState
-	OverrideManifests(platform string) error
+	OverrideManifests(ctx context.Context, platform cluster.Platform) error
 	UpdatePrometheusConfig(cli client.Client, enable bool, component string) error
 	ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger
 }

diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
-
 	corev1 "k8s.io/api/core/v1"
 	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -43,11 +42,11 @@ type Dashboard struct {
 	components.Component `json:""`
 }
 
-func (d *Dashboard) OverrideManifests(_ string) error {
+func (d *Dashboard) OverrideManifests(ctx context.Context, platform cluster.Platform) error {
 	// If devflags are set, update default manifests path
 	if len(d.DevFlags.Manifests) != 0 {
 		manifestConfig := d.DevFlags.Manifests[0]
-		if err := deploy.DownloadManifests(ComponentNameUpstream, manifestConfig); err != nil {
+		if err := deploy.DownloadManifests(ctx, ComponentNameUpstream, manifestConfig); err != nil {
 			return err
 		}
 		if manifestConfig.SourcePath != "" {
@@ -94,7 +93,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 		}
 		if d.DevFlags != nil {
 			// Download manifests and update paths
-			if err := d.OverrideManifests(string(platform)); err != nil {
+			if err := d.OverrideManifests(ctx, platform); err != nil {
 				return err
 			}
 			if OverridePath != "" {
@@ -119,7 +118,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 		}
 
 		// 4. Append or Update variable for component to consume
-		extraParamsMap, err := updateKustomizeVariable(cli, platform, dscispec)
+		extraParamsMap, err := updateKustomizeVariable(ctx, cli, platform, dscispec)
 		if err != nil {
 			return errors.New("failed to set variable for extraParamsMap")
 		}
@@ -141,7 +140,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 			return fmt.Errorf("failed to create access-secret for anaconda: %w", err)
 		}
 		// Deploy RHOAI manifests
-		if err := deploy.DeployManifestsFromPath(cli, owner, entryPath, dscispec.ApplicationsNamespace, ComponentNameDownstream, enabled); err != nil {
+		if err := deploy.DeployManifestsFromPath(ctx, cli, owner, entryPath, dscispec.ApplicationsNamespace, ComponentNameDownstream, enabled); err != nil {
 			return fmt.Errorf("failed to apply manifests from %s: %w", PathDownstream, err)
 		}
 		l.Info("apply manifests done")
@@ -159,7 +158,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 			if err := d.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentNameDownstream); err != nil {
 				return err
 			}
-			if err := deploy.DeployManifestsFromPath(cli, owner,
+			if err := deploy.DeployManifestsFromPath(ctx, cli, owner,
 				filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
 				dscispec.Monitoring.Namespace,
 				"prometheus", true); err != nil {
@@ -171,15 +170,15 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 
 	default:
 		// Deploy ODH manifests
-		if err := deploy.DeployManifestsFromPath(cli, owner, entryPath, dscispec.ApplicationsNamespace, ComponentNameUpstream, enabled); err != nil {
+		if err := deploy.DeployManifestsFromPath(ctx, cli, owner, entryPath, dscispec.ApplicationsNamespace, ComponentNameUpstream, enabled); err != nil {
 			return err
 		}
 		l.Info("apply manifests done")
 		return nil
 	}
 }
 
-func updateKustomizeVariable(cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) {
+func updateKustomizeVariable(ctx context.Context, cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) {
 	adminGroups := map[cluster.Platform]string{
 		cluster.SelfManagedRhods: "rhods-admins",
 		cluster.ManagedRhods:     "dedicated-admins",
@@ -194,7 +193,7 @@ func updateKustomizeVariable(cli client.Client, platform cluster.Platform, dscis
 		cluster.Unknown:          "OpenShift Open Data Hub",
 	}[platform]
 
-	consoleLinkDomain, err := cluster.GetDomain(cli)
+	consoleLinkDomain, err := cluster.GetDomain(ctx, cli)
 	if err != nil {
 		return nil, fmt.Errorf("error getting console route URL %s : %w", consoleLinkDomain, err)
 	}

diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go
@@ -41,11 +41,11 @@ type DataSciencePipelines struct {
 	components.Component `json:""`
 }
 
-func (d *DataSciencePipelines) OverrideManifests(_ string) error {
+func (d *DataSciencePipelines) OverrideManifests(ctx context.Context, _ string) error {
 	// If devflags are set, update default manifests path
 	if len(d.DevFlags.Manifests) != 0 {
 		manifestConfig := d.DevFlags.Manifests[0]
-		if err := deploy.DownloadManifests(ComponentName, manifestConfig); err != nil {
+		if err := deploy.DownloadManifests(ctx, ComponentName, manifestConfig); err != nil {
 			return err
 		}
 		// If overlay is defined, update paths
@@ -97,7 +97,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 	if enabled {
 		if d.DevFlags != nil {
 			// Download manifests and update paths
-			if err := d.OverrideManifests(string(platform)); err != nil {
+			if err := d.OverrideManifests(ctx, string(platform)); err != nil {
 				return err
 			}
 		}
@@ -119,7 +119,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 	if platform == cluster.OpenDataHub || platform == "" {
 		manifestsPath = filepath.Join(OverlayPath, "odh")
 	}
-	if err := deploy.DeployManifestsFromPath(cli, owner, manifestsPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
+	if err := deploy.DeployManifestsFromPath(ctx, cli, owner, manifestsPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
 		return err
 	}
 	l.Info("apply manifests done")
@@ -138,7 +138,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 		if err := d.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
 			return err
 		}
-		if err := deploy.DeployManifestsFromPath(cli, owner,
+		if err := deploy.DeployManifestsFromPath(ctx, cli, owner,
 			filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
 			dscispec.Monitoring.Namespace,
 			"prometheus", true); err != nil {

diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go
@@ -56,13 +56,13 @@ type Kserve struct {
 	DefaultDeploymentMode DefaultDeploymentMode `json:"defaultDeploymentMode,omitempty"`
 }
 
-func (k *Kserve) OverrideManifests(_ string) error {
+func (k *Kserve) OverrideManifests(ctx context.Context, _ string) error {
 	// Download manifests if defined by devflags
 	// Go through each manifest and set the overlays if defined
 	for _, subcomponent := range k.DevFlags.Manifests {
 		if strings.Contains(subcomponent.URI, DependentComponentName) {
 			// Download subcomponent
-			if err := deploy.DownloadManifests(DependentComponentName, subcomponent); err != nil {
+			if err := deploy.DownloadManifests(ctx, DependentComponentName, subcomponent); err != nil {
 				return err
 			}
 			// If overlay is defined, update paths
@@ -75,7 +75,7 @@ func (k *Kserve) OverrideManifests(_ string) error {
 
 		if strings.Contains(subcomponent.URI, ComponentName) {
 			// Download subcomponent
-			if err := deploy.DownloadManifests(ComponentName, subcomponent); err != nil {
+			if err := deploy.DownloadManifests(ctx, ComponentName, subcomponent); err != nil {
 				return err
 			}
 			// If overlay is defined, update paths
@@ -108,17 +108,17 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 
 	if !enabled {
-		if err := k.removeServerlessFeatures(dscispec); err != nil {
+		if err := k.removeServerlessFeatures(ctx, dscispec); err != nil {
 			return err
 		}
 	} else {
 		// Configure dependencies
-		if err := k.configureServerless(dscispec); err != nil {
+		if err := k.configureServerless(ctx, dscispec); err != nil {
 			return err
 		}
 		if k.DevFlags != nil {
 			// Download manifests and update paths
-			if err := k.OverrideManifests(string(platform)); err != nil {
+			if err := k.OverrideManifests(ctx, string(platform)); err != nil {
 				return err
 			}
 		}
@@ -131,11 +131,11 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
 		}
 	}
 
-	if err := k.configureServiceMesh(cli, dscispec); err != nil {
+	if err := k.configureServiceMesh(ctx, cli, dscispec); err != nil {
 		return fmt.Errorf("failed configuring service mesh while reconciling kserve component. cause: %w", err)
 	}
 
-	if err := deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
+	if err := deploy.DeployManifestsFromPath(ctx, cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
 		return fmt.Errorf("failed to apply manifests from %s : %w", Path, err)
 	}
 
@@ -158,7 +158,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
 		}
 	}
 
-	if err := deploy.DeployManifestsFromPath(cli, owner, DependentPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
+	if err := deploy.DeployManifestsFromPath(ctx, cli, owner, DependentPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
 		if !strings.Contains(err.Error(), "spec.selector") || !strings.Contains(err.Error(), "field is immutable") {
 			// explicitly ignore error if error contains keywords "spec.selector" and "field is immutable" and return all other error.
 			return err
@@ -184,10 +184,10 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
 	return nil
 }
 
-func (k *Kserve) Cleanup(cli client.Client, instance *dsciv1.DSCInitializationSpec) error {
-	if removeServerlessErr := k.removeServerlessFeatures(instance); removeServerlessErr != nil {
+func (k *Kserve) Cleanup(ctx context.Context, cli client.Client, instance *dsciv1.DSCInitializationSpec) error {
+	if removeServerlessErr := k.removeServerlessFeatures(ctx, instance); removeServerlessErr != nil {
 		return removeServerlessErr
 	}
 
-	return k.removeServiceMeshConfigurations(cli, instance)
+	return k.removeServiceMeshConfigurations(ctx, cli, instance)
 }
diff --git a/components/kserve/kserve_config_handler.go b/components/kserve/kserve_config_handler.go
@@ -117,14 +117,14 @@ func (k *Kserve) setDefaultDeploymentMode(ctx context.Context, cli client.Client
 	return nil
 }
 
-func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) error {
+func (k *Kserve) configureServerless(ctx context.Context, instance *dsciv1.DSCInitializationSpec) error {
 	switch k.Serving.ManagementState {
 	case operatorv1.Unmanaged: // Bring your own CR
 		fmt.Println("Serverless CR is not configured by the operator, we won't do anything")
 
 	case operatorv1.Removed: // we remove serving CR
 		fmt.Println("existing Serverless CR (owned by operator) will be removed")
-		if err := k.removeServerlessFeatures(instance); err != nil {
+		if err := k.removeServerlessFeatures(ctx, instance); err != nil {
 			return err
 		}
 
@@ -136,17 +136,17 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
 
 		serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), instance, k.configureServerlessFeatures())
 
-		if err := serverlessFeatures.Apply(); err != nil {
+		if err := serverlessFeatures.Apply(ctx); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error {
+func (k *Kserve) removeServerlessFeatures(ctx context.Context, instance *dsciv1.DSCInitializationSpec) error {
 	serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), instance, k.configureServerlessFeatures())
 
-	return serverlessFeatures.Delete()
+	return serverlessFeatures.Delete(ctx)
 }
 
 func checkDependentOperators(cli client.Client) *multierror.Error {

diff --git a/components/kserve/serverless_setup.go b/components/kserve/serverless_setup.go
@@ -1,6 +1,7 @@
 package kserve
 
 import (
+	"context"
 	"path"
 
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
@@ -70,7 +71,7 @@ func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
 }
 
 func PopulateComponentSettings(k *Kserve) feature.Action {
-	return func(f *feature.Feature) error {
+	return func(_ context.Context, f *feature.Feature) error {
 		f.Spec.Serving = &k.Serving
 		return nil
 	}