From 8401c70930be79af13e1d78f5ea55c5f397aa8ce Mon Sep 17 00:00:00 2001 From: Nitish Bhat Date: Wed, 2 Sep 2026 22:33:20 +0000 Subject: [PATCH] build: upgrade golangci-lint to v2.13.1 and fix resulting lint issues `make lint` does not currently work in this repo. It fails before reporting anything: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.25.8) golangci-lint@v1.63.4 has a toolchain directive forcing it to be built with go1.23.6, while go.mod targets Go 1.25.8, and golangci-lint refuses that combination. Staying on v1 is not an option: every v1 release up to and including the final v1.64.8 is built with Go <= 1.24. Bump to v2.13.1 (built with go1.26.7). Note the module path gains a /v2 suffix. There is no .golangci.yml in this repo, so there is no config to migrate - only the default linter set changes slightly (gosimple folds into staticcheck). Fix the 34 issues this surfaces. Note that golangci-lint caps duplicate messages at 3 by default, which reports only 21 of them; the full set was found with --max-same-issues=0. errcheck 7 unchecked Close() / os.Setenv / os.Unsetenv returns staticcheck 13 ST1005 capitalized error strings staticcheck 5 QF1004 strings.Replace(..., -1) -> strings.ReplaceAll staticcheck 4 QF1008 redundant embedded-field selectors staticcheck 2 QF1003 if/else-if chain -> tagged switch staticcheck 2 SA1019 deprecated NodeSystemInfo.KubeProxyVersion staticcheck 1 ST1001 dot import Notes on the two judgement calls: - SA1019: KubeProxyVersion appears only as test fixture data and is never read by non-test code, so the field is dropped rather than suppressed. - ST1001: the dot import is gocheck's documented idiom and there are 66 call sites across the e2e suite that would need qualifying. Suppressed with a nolint carrying that rationale rather than churning the suite. All changes are behaviour preserving. Verified: make manager, make vet, make unit-test and make lint all pass, and lint reports 0 issues uncapped. --- Makefile | 3 ++- internal/config/config.go | 2 +- .../controllers/network_config_reconciler.go | 20 +++++++++---------- .../network_config_reconciler_test.go | 1 - internal/controllers/upgrademgr.go | 7 ++++--- internal/kmmmodule/kmmmodule.go | 12 +++++------ internal/kmmmodule/kmmmodule_test.go | 5 ++--- internal/validator/specValidators.go | 4 ++-- internal/validator/utils.go | 6 +++--- tests/e2e/client/client.go | 2 +- tests/e2e/e2e_test.go | 5 +++-- tests/e2e/suite.go | 1 + tests/e2e/utils/utils.go | 8 ++++---- 13 files changed, 39 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index f0842162..efed6646 100644 --- a/Makefile +++ b/Makefile @@ -379,9 +379,10 @@ controller-gen: ## Download controller-gen locally if necessary. $(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.0) GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint +GOLANGCI_LINT_VERSION ?= v2.13.1 .PHONY: golangci-lint golangci-lint: ## Download golangci-lint locally if necessary. - $(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@v1.63.4) + $(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)) HELMDOCS = $(shell pwd)/bin/helm-docs .PHONY: helm-docs diff --git a/internal/config/config.go b/internal/config/config.go index 5b3bbaa2..f7b01def 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,7 +57,7 @@ func ParseFile(path string) (*Config, error) { if err != nil { return nil, fmt.Errorf("could not open the configuration file: %v", err) } - defer fd.Close() + defer func() { _ = fd.Close() }() cfg := Config{} diff --git a/internal/controllers/network_config_reconciler.go b/internal/controllers/network_config_reconciler.go index 1d5ef830..6fa70196 100644 --- a/internal/controllers/network_config_reconciler.go +++ b/internal/controllers/network_config_reconciler.go @@ -207,7 +207,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if k8serrors.IsNotFound(err) || strings.Contains(err.Error(), "not found") { logger.Info("NetworkConfig CR deleted") - r.helper.updateNodeAssignments(req.NamespacedName.String(), nil, true) + r.helper.updateNodeAssignments(req.String(), nil, true) return ctrl.Result{}, nil } return res, fmt.Errorf("failed to get the requested %s CR: %v", req.NamespacedName, err) @@ -232,13 +232,13 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques } // Verify that the NetworkConfig does not select nodes covered by other NetworkConfigs - err = r.helper.validateNodeAssignments(req.NamespacedName.String(), nodes) + err = r.helper.validateNodeAssignments(req.String(), nodes) if err != nil { if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeError, nwConfig, metav1.ConditionTrue, conditions.ValidationError, fmt.Sprintf("Validation failed: %v", err)); errSet != nil { - logger.Error(fmt.Errorf("Failed to set error condition: %v", errSet), "") + logger.Error(fmt.Errorf("failed to set error condition: %v", errSet), "") } if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeReady, nwConfig, metav1.ConditionFalse, conditions.ReadyStatus, ""); errSet != nil { - logger.Error(fmt.Errorf("Failed to set ready condition: %v", errSet), "") + logger.Error(fmt.Errorf("failed to set ready condition: %v", errSet), "") } return res, err } @@ -248,10 +248,10 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques if len(result) != 0 { // Update status Conditions here if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeError, nwConfig, metav1.ConditionTrue, conditions.ValidationError, fmt.Sprintf("Validation failed: %v", result)); errSet != nil { - logger.Error(fmt.Errorf("Failed to set error condition: %v", errSet), "") + logger.Error(fmt.Errorf("failed to set error condition: %v", errSet), "") } if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeReady, nwConfig, metav1.ConditionFalse, conditions.ReadyStatus, ""); errSet != nil { - logger.Error(fmt.Errorf("Failed to set ready condition: %v", errSet), "") + logger.Error(fmt.Errorf("failed to set ready condition: %v", errSet), "") } return res, fmt.Errorf("validation failed for NetworkConfig %s: %v", req.NamespacedName, result) } @@ -270,7 +270,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques logger.Info("start module install/upgrade reconciliation") res, err = r.helper.handleModuleUpgrade(ctx, nwConfig, nodes, false) if err != nil { - return res, fmt.Errorf("Failed to fetch nodes for NetworkConfig %s: %v", req.NamespacedName, err) + return res, fmt.Errorf("failed to fetch nodes for NetworkConfig %s: %v", req.NamespacedName, err) } logger.Info("start KMM reconciliation") @@ -326,7 +326,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques } // Update nodeAssignments after NetworkConfig status update - r.helper.updateNodeAssignments(req.NamespacedName.String(), nodes, false) + r.helper.updateNodeAssignments(req.String(), nodes, false) return res, nil } @@ -1539,7 +1539,7 @@ func (dcrh *networkConfigReconcilerHelper) setCondition(ctx context.Context, con dcrh.conditionUpdater.SetErrorCondition(nwConfig, status, reason, message) return dcrh.updateNetworkConfigStatus(ctx, nwConfig) } - return fmt.Errorf("Condition %s not supported", condition) + return fmt.Errorf("condition %s not supported", condition) } func (dcrh *networkConfigReconcilerHelper) deleteCondition(ctx context.Context, condition string, nwConfig *amdv1alpha1.NetworkConfig) error { @@ -1551,7 +1551,7 @@ func (dcrh *networkConfigReconcilerHelper) deleteCondition(ctx context.Context, dcrh.conditionUpdater.DeleteErrorCondition(nwConfig) return dcrh.updateNetworkConfigStatus(ctx, nwConfig) } - return fmt.Errorf("Condition %s not supported", condition) + return fmt.Errorf("condition %s not supported", condition) } func (dcrh *networkConfigReconcilerHelper) validateNetworkConfig(ctx context.Context, nwConfig *amdv1alpha1.NetworkConfig) []string { diff --git a/internal/controllers/network_config_reconciler_test.go b/internal/controllers/network_config_reconciler_test.go index c903eb0b..208dd433 100644 --- a/internal/controllers/network_config_reconciler_test.go +++ b/internal/controllers/network_config_reconciler_test.go @@ -74,7 +74,6 @@ var ( Architecture: "amd64", ContainerRuntimeVersion: "containerd://1.7.19", KernelVersion: "6.8.0-40-generic", - KubeProxyVersion: "v1.30.3", KubeletVersion: "v1.30.3", OperatingSystem: "linux", OSImage: "Ubuntu 22.04.3 LTS", diff --git a/internal/controllers/upgrademgr.go b/internal/controllers/upgrademgr.go index e2618e54..5327d8dd 100644 --- a/internal/controllers/upgrademgr.go +++ b/internal/controllers/upgrademgr.go @@ -137,7 +137,8 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph initInternalNodeStates := func(networkConfig *amdv1alpha1.NetworkConfig) { for nodeName, moduleStatus := range networkConfig.Status.NodeModuleStatus { - if moduleStatus.Status == amdv1alpha1.UpgradeStateStarted { + switch moduleStatus.Status { + case amdv1alpha1.UpgradeStateStarted: if networkConfig.Spec.Driver.UpgradePolicy.RebootRequired != nil && *networkConfig.Spec.Driver.UpgradePolicy.RebootRequired { nodeObj, err := n.helper.getNode(ctx, nodeName) if err == nil { @@ -158,7 +159,7 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph log.FromContext(ctx).Info(fmt.Sprintf("Node: %v: Resetting Upgrade State to UpgradeStateEmpty", nodeName)) n.helper.setNodeStatus(ctx, nodeName, amdv1alpha1.UpgradeStateEmpty) } - } else if moduleStatus.Status == amdv1alpha1.UpgradeStateRebootInProgress { + case amdv1alpha1.UpgradeStateRebootInProgress: // Operator restarted during upgrade operation. Schedule the reboot pod deletion log.FromContext(ctx).Info(fmt.Sprintf("Node: %v: Reboot is in progress, scheduling reboot pod deletion", nodeName)) // If the pod is still present, schedule reboot pod deletion, else, move ahead to Upgrade-In-Progress @@ -171,7 +172,7 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph n.helper.setNodeStatus(ctx, nodeName, moduleStatus.Status) go n.helper.deleteRebootPod(ctx, nodeName, *networkConfig, false) } - } else { + default: n.helper.setNodeStatus(ctx, nodeName, moduleStatus.Status) } } diff --git a/internal/kmmmodule/kmmmodule.go b/internal/kmmmodule/kmmmodule.go index c0eaf0e7..f2bfc5ff 100644 --- a/internal/kmmmodule/kmmmodule.go +++ b/internal/kmmmodule/kmmmodule.go @@ -216,7 +216,7 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri if !present { return "", fmt.Errorf("invalid ubuntu version, expected to be one of %v", maps.Keys(driverLabels)) } - dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$DRIVER_LABEL", driverLabel, -1) + dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$DRIVER_LABEL", driverLabel) // trigger to pull the internal ROCM dev build if internalArtifactoryURL, ok := os.LookupEnv("INTERNAL_ARTIFACTORY"); ok && @@ -227,9 +227,9 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri return "", fmt.Errorf("please provide internal build info, required 4 items: artifactory URL, installer deb file name, amdionic build number and rocm build tag, got: %+v", nwConfig.Spec.Driver.AMDNetworkInstallerRepoURL) } nwConfig.Spec.Driver.AMDNetworkInstallerRepoURL = devBuildinfo[0] - dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$DEV_DEB", devBuildinfo[1], -1) - dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$AMDNetwork_BUILD", devBuildinfo[2], -1) - dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$ROCM_BUILD", devBuildinfo[3], -1) + dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$DEV_DEB", devBuildinfo[1]) + dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$AMDNetwork_BUILD", devBuildinfo[2]) + dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$ROCM_BUILD", devBuildinfo[3]) } case "coreos": dockerfileTemplate = dockerfileTemplateCoreOSFromRPM @@ -251,7 +251,7 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri default: return "", fmt.Errorf("not supported OS: %s", osDistro) } - resolvedDockerfile := strings.Replace(dockerfileTemplate, "$$VERSION", version, -1) + resolvedDockerfile := strings.ReplaceAll(dockerfileTemplate, "$$VERSION", version) return resolvedDockerfile, nil } @@ -339,7 +339,7 @@ func getKernelMappings(nwConfig *amdv1alpha1.NetworkConfig, isOpenshift bool, no } if nodes == nil || len(nodes.Items) == 0 { - return nil, "", fmt.Errorf("No nodes found for the label selector %s", MapToLabelSelector(nwConfig.Spec.Selector)) + return nil, "", fmt.Errorf("no nodes found for the label selector %s", MapToLabelSelector(nwConfig.Spec.Selector)) } kernelMappings := []kmmv1beta1.KernelMapping{} kmSet := map[string]bool{} diff --git a/internal/kmmmodule/kmmmodule_test.go b/internal/kmmmodule/kmmmodule_test.go index af53b122..0002f967 100644 --- a/internal/kmmmodule/kmmmodule_test.go +++ b/internal/kmmmodule/kmmmodule_test.go @@ -64,7 +64,6 @@ var ( Architecture: "amd64", ContainerRuntimeVersion: "containerd://1.7.19", KernelVersion: "6.8.0-40-generic", - KubeProxyVersion: "v1.30.3", KubeletVersion: "v1.30.3", OperatingSystem: "linux", OSImage: "Ubuntu 22.04.3 LTS", @@ -187,8 +186,8 @@ var _ = Describe("BaseImageRegistry and BaseImageRegistryTLS", func() { } // Set CI_ENV - os.Setenv("CI_ENV", "1") - defer os.Unsetenv("CI_ENV") + _ = os.Setenv("CI_ENV", "1") + defer func() { _ = os.Unsetenv("CI_ENV") }() km, _, err := getKM(nwConfig, node, "", false) diff --git a/internal/validator/specValidators.go b/internal/validator/specValidators.go index d25a33b8..3d3d3830 100644 --- a/internal/validator/specValidators.go +++ b/internal/validator/specValidators.go @@ -102,7 +102,7 @@ func ValidateDevicePluginSpec(ctx context.Context, client client.Client, nwConfi for key, val := range devicePluginArguments { validValues, validKey := supportedFlagValues[key] if !validKey { - return fmt.Errorf("Invalid flag: %s", key) + return fmt.Errorf("invalid flag: %s", key) } validKeyValue := false @@ -114,7 +114,7 @@ func ValidateDevicePluginSpec(ctx context.Context, client client.Client, nwConfi } if !validKeyValue { - return fmt.Errorf("Invalid flag value: %s=%s. Supported values: %v", key, val, supportedFlagValues[key]) + return fmt.Errorf("invalid flag value: %s=%s. Supported values: %v", key, val, supportedFlagValues[key]) } } diff --git a/internal/validator/utils.go b/internal/validator/utils.go index 7b0fb121..21e2c7cd 100644 --- a/internal/validator/utils.go +++ b/internal/validator/utils.go @@ -35,14 +35,14 @@ const ( func validateSecret(ctx context.Context, client client.Client, secretRef *v1.LocalObjectReference, namespace string) error { if secretRef == nil || secretRef.Name == "" { - return fmt.Errorf("Secret reference is nil or empty") + return fmt.Errorf("secret reference is nil or empty") } secret := &v1.Secret{} err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretRef.Name}, secret) if err != nil { if k8serrors.IsNotFound(err) { - return fmt.Errorf("Secret %s not found in namespace %s", secretRef.Name, namespace) + return fmt.Errorf("secret %s not found in namespace %s", secretRef.Name, namespace) } return fmt.Errorf("failed to get Secret %s: %v", secretRef.Name, err) } @@ -52,7 +52,7 @@ func validateSecret(ctx context.Context, client client.Client, secretRef *v1.Loc func validateConfigMap(ctx context.Context, client client.Client, mapRef string, namespace string) error { if mapRef == "" { - return fmt.Errorf("No ConfigMap name provided for validation") + return fmt.Errorf("no ConfigMap name provided for validation") } configMap := &v1.ConfigMap{} diff --git a/tests/e2e/client/client.go b/tests/e2e/client/client.go index 92b9ca11..2e4f696b 100644 --- a/tests/e2e/client/client.go +++ b/tests/e2e/client/client.go @@ -37,7 +37,7 @@ type NetworkConfigClient struct { func Client(c *rest.Config) (*NetworkConfigClient, error) { config := *c - config.ContentConfig.GroupVersion = &v1alpha1.GroupVersion + config.GroupVersion = &v1alpha1.GroupVersion config.APIPath = "/apis" config.NegotiatedSerializer = scheme.Codecs.WithoutConversion() config.UserAgent = rest.DefaultKubernetesUserAgent() diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index e0bae0c3..edbea1eb 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -280,11 +280,12 @@ func (s *E2ESuite) verifyMetricsExporterServiceStatus(nc *v1alpha1.NetworkConfig } // Validate port configuration based on service type - if ncSvcType == v1alpha1.ServiceTypeNodePort { + switch ncSvcType { + case v1alpha1.ServiceTypeNodePort: if svc.Spec.Ports[0].NodePort != nc.Spec.MetricsExporter.NodePort { return false, fmt.Errorf("NodePort service port mismatch, expected %d, got %d", nc.Spec.MetricsExporter.NodePort, svc.Spec.Ports[0].NodePort) } - } else if ncSvcType == v1alpha1.ServiceTypeClusterIP { + case v1alpha1.ServiceTypeClusterIP: if svc.Spec.Ports[0].Port != nc.Spec.MetricsExporter.Port { return false, fmt.Errorf("ClusterIP service port mismatch, expected %d, got %d", nc.Spec.MetricsExporter.Port, svc.Spec.Ports[0].Port) } diff --git a/tests/e2e/suite.go b/tests/e2e/suite.go index 37aa3f8e..62105673 100644 --- a/tests/e2e/suite.go +++ b/tests/e2e/suite.go @@ -32,6 +32,7 @@ import ( monitoringClient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + //nolint:staticcheck // ST1001: gocheck's dot-import is the library's documented idiom; qualifying it would touch 66 call sites across the e2e suite. . "gopkg.in/check.v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextClient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" diff --git a/tests/e2e/utils/utils.go b/tests/e2e/utils/utils.go index d18e2512..5b4680a8 100644 --- a/tests/e2e/utils/utils.go +++ b/tests/e2e/utils/utils.go @@ -486,7 +486,7 @@ func RunCommandOnNode(ctx context.Context, cl *kubernetes.Clientset, nodeName, c if err != nil { return "", err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() data, err := io.ReadAll(resp.Body) log.Infof("runcommand resp status=%s err=%v", resp.Status, err) if err != nil { @@ -723,7 +723,7 @@ func DeployResourcesFromFile(pathOrURL string, cl *kubernetes.Clientset, apiCl * if err != nil { return fmt.Errorf("HTTP GET %s: %w", pathOrURL, err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { return fmt.Errorf("HTTP GET %s returned status %d (%s)", pathOrURL, resp.StatusCode, resp.Status) } @@ -879,7 +879,7 @@ func IsNodeHealthy(cl *kubernetes.Clientset, nodeip string) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() body, _ := io.ReadAll(resp.Body) log.Infof("health resp: %s body=%s", resp.Status, string(body)) if resp.StatusCode != http.StatusOK || string(body) != "healthy" { @@ -900,7 +900,7 @@ func RebootNode(cl *kubernetes.Clientset, nodeip string) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) return fmt.Errorf("reboot failed status=%s body=%s", resp.Status, string(body))