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 <lburgazzoli@gmail.com>

* chore(lint): enable containedctx

Signed-off-by: Luca Burgazzoli <lburgazzoli@gmail.com>

* Fix PR review findings

* Fix rebase

---------

Signed-off-by: Luca Burgazzoli <lburgazzoli@gmail.com>
(cherry picked from commit 47ea7d1)
  • Loading branch information
lburgazzoli authored and VaishnaviHire committed Jul 24, 2024
1 parent 4578d74 commit 06e21a4
Show file tree
Hide file tree
Showing 59 changed files with 797 additions and 595 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
10 changes: 3 additions & 7 deletions components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 9 additions & 10 deletions components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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")
}
Expand All @@ -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")
Expand All @@ -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 {
Expand All @@ -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",
Expand All @@ -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)
}
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 @@ -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
}
}
Expand All @@ -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)
}

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

Expand All @@ -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 {
Expand Down
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 06e21a4

Please sign in to comment.