Skip to content

Commit

Permalink
chore(lint): enable contextcheck and containedctx (opendatahub-io#1070)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
lburgazzoli authored Jun 25, 2024
1 parent ea827dd commit 47ea7d1
Show file tree
Hide file tree
Showing 59 changed files with 650 additions and 598 deletions.
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

0 comments on commit 47ea7d1

Please sign in to comment.