Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(lint): enable contextcheck and containedctx #1070

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions components/codeflare/codeflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c *Component) GetManagementState() operatorv1.ManagementState {
return c.ManagementState
}

func (c *Component) Cleanup(_ client.Client, _ *dsciv1.DSCInitializationSpec) error {
func (c *Component) Cleanup(_ context.Context, _ client.Client, _ *dsciv1.DSCInitializationSpec) error {
// noop
return nil
}
Expand Down Expand Up @@ -81,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 string) error
UpdatePrometheusConfig(cli client.Client, enable bool, component string) error
ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger
}
Expand Down
30 changes: 15 additions & 15 deletions components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ type Dashboard struct {
components.Component `json:""`
}

func (d *Dashboard) OverrideManifests(platform string) error {
func (d *Dashboard) OverrideManifests(ctx context.Context, platform 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
Expand Down Expand Up @@ -113,12 +113,12 @@ 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, string(platform)); err != nil {
return err
}
}
// 1. Deploy CRDs
if err := d.deployCRDsForPlatform(cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
if err := d.deployCRDsForPlatform(ctx, cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
return fmt.Errorf("failed to deploy Dashboard CRD: %w", err)
}

Expand Down Expand Up @@ -154,12 +154,12 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
return fmt.Errorf("failed to create access-secret for anaconda: %w", err)
}
// overlay which including ../../base + anaconda-ce-validator
if err := deploy.DeployManifestsFromPath(cli, owner, PathSupported, dscispec.ApplicationsNamespace, ComponentNameSupported, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, PathSupported, dscispec.ApplicationsNamespace, ComponentNameSupported, enabled); err != nil {
return fmt.Errorf("failed to apply manifests from %s: %w", PathSupported, err)
}

// Apply RHOAI specific configs, e.g anaconda screct and cronjob and ISV
if err := d.applyRHOAISpecificConfigs(cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
if err := d.applyRHOAISpecificConfigs(ctx, cli, owner, dscispec.ApplicationsNamespace, platform); err != nil {
return err
}
// consolelink
Expand All @@ -181,7 +181,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
if err := d.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentNameSupported); 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 {
Expand All @@ -192,11 +192,11 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
return nil
default:
// base
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 err
}
// ISV
if err := deploy.DeployManifestsFromPath(cli, owner, PathISV, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, PathISV, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
return err
}
// consolelink
Expand All @@ -208,16 +208,16 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
}
}

func (d *Dashboard) deployCRDsForPlatform(cli client.Client, owner metav1.Object, namespace string, platform cluster.Platform) error {
func (d *Dashboard) deployCRDsForPlatform(ctx context.Context, cli client.Client, owner metav1.Object, namespace string, platform cluster.Platform) error {
componentName := ComponentName
if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods {
componentName = ComponentNameSupported
}
// we only deploy CRD, we do not remove CRD
return deploy.DeployManifestsFromPath(cli, owner, PathCRDs, namespace, componentName, true)
return deploy.DeployManifestsFromPath(ctx, cli, owner, PathCRDs, namespace, componentName, true)
}

func (d *Dashboard) applyRHOAISpecificConfigs(cli client.Client, owner metav1.Object, namespace string, platform cluster.Platform) error {
func (d *Dashboard) applyRHOAISpecificConfigs(ctx context.Context, cli client.Client, owner metav1.Object, namespace string, platform cluster.Platform) error {
enabled := d.ManagementState == operatorv1.Managed

// set proper group name
Expand All @@ -230,15 +230,15 @@ func (d *Dashboard) applyRHOAISpecificConfigs(cli client.Client, owner metav1.Ob
if err := common.ReplaceStringsInFile(dashboardConfig, map[string]string{"<admin_groups>": adminGroups}); err != nil {
return err
}
if err := deploy.DeployManifestsFromPath(cli, owner, PathODHDashboardConfig, namespace, ComponentNameSupported, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, PathODHDashboardConfig, namespace, ComponentNameSupported, enabled); err != nil {
return fmt.Errorf("failed to create OdhDashboardConfig from %s: %w", PathODHDashboardConfig, err)
}
// ISV
path := PathISVSM
if platform == cluster.ManagedRhods {
path = PathISVAddOn
}
if err := deploy.DeployManifestsFromPath(cli, owner, path, namespace, ComponentNameSupported, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, path, namespace, ComponentNameSupported, enabled); err != nil {
return fmt.Errorf("failed to set dashboard ISV from %s : %w", Path, err)
}
return nil
Expand Down Expand Up @@ -278,7 +278,7 @@ func (d *Dashboard) deployConsoleLink(ctx context.Context, cli client.Client, ow
}

enabled := d.ManagementState == operatorv1.Managed
if err := deploy.DeployManifestsFromPath(cli, owner, PathConsoleLink, namespace, componentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, PathConsoleLink, namespace, componentName, enabled); err != nil {
return fmt.Errorf("failed to set dashboard consolelink %s : %w", pathConsoleLink, err)
}

Expand Down
10 changes: 5 additions & 5 deletions components/datasciencepipelines/datasciencepipelines.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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")
Expand All @@ -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 {
Expand Down
24 changes: 12 additions & 12 deletions components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -109,17 +109,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
}
}
Expand All @@ -132,11 +132,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)
}

Expand All @@ -159,7 +159,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
Expand All @@ -185,10 +185,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)
}
10 changes: 5 additions & 5 deletions components/kserve/kserve_config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,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
}

Expand All @@ -134,15 +134,15 @@ 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)
}
3 changes: 2 additions & 1 deletion components/kserve/serverless_setup.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package kserve

import (
"context"
"path"

"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
Expand Down Expand Up @@ -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
}
Expand Down
Loading
Loading