OCPBUGS-45421: Linter fixes (#1253)

* Linter fixes

Golangci-linter configuration ".ci-operator.yaml" was configured in a way
which was showing a lot of false positives and also missing a lot of issues.

Fix both the linter configuration and the missing issues that found its way
into the codebase.

* Remove misspelt word coming from openshift/kubernetes

* Keep e2e-gcp-pao-updating-profile happy

Tests in 7_performance_kubelet_node/kubelet.go are broken and need
fixing, keep the current semantics and make the linter pass.

---------

Co-authored-by: Jiri Mencak <[email protected]>

Author: jmencak

.ci-operator.yaml

@@ -1,4 +1,4 @@
 build_root_image:
   name: release
   namespace: openshift
-  tag: rhel-9-release-golang-1.22-openshift-4.18
+  tag: rhel-9-release-golang-1.23-openshift-4.19

.golangci.yaml

@@ -1,16 +1,19 @@
 run:
   timeout: 5m
-  tests: false
-  skip-dirs:
-    - vendor
-    - test
   modules-download-mode: vendor
+issues:
+  exclude-dirs:
+    - vendor
+  exclude-rules:
+    # Temporarily disable the deprecation warnings until we rewrite the code based on deprecated functionality.
+    - linters:
+        - staticcheck
+      text: "SA1019: .* deprecated"
 linters:
   disable-all: true
   enable:
     - errcheck
     - bodyclose
-    - exportloopref
     - gosimple
     - govet
     - ineffassign
@@ -25,8 +28,14 @@ linters:
     - wastedassign
     - whitespace
 linters-settings:
+  errcheck:
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+    # default is false: such cases aren't reported by default.
+    check-blank: false
   misspell:
-    locale: US
+    # Do not set locale explicitly, the default is to use a neutral variety of English.
+    # Setting locale to US will cause correcting the British spelling of 'colour' to 'color'.
+    # locale: US
     ignore-words:
       - NTO
       - nto

Dockerfile.rhel9

@@ -1,9 +1,9 @@
-FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.22-openshift-4.18 AS builder
+FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.23-openshift-4.19 AS builder
 WORKDIR /go/src/github.com/openshift/cluster-node-tuning-operator
 COPY . .
 RUN make build
 
-FROM registry.ci.openshift.org/ocp/4.18:base-rhel9
+FROM registry.ci.openshift.org/ocp/4.19:base-rhel9
 COPY --from=builder /go/src/github.com/openshift/cluster-node-tuning-operator/_output/cluster-node-tuning-operator /usr/bin/
 COPY --from=builder /go/src/github.com/openshift/cluster-node-tuning-operator/_output/performance-profile-creator /usr/bin/
 COPY --from=builder /go/src/github.com/openshift/cluster-node-tuning-operator/_output/gather-sysinfo /usr/bin/

Makefile

@@ -55,7 +55,7 @@ PAO_CRD_APIS :=$(addprefix ./$(API_TYPES_DIR)/performanceprofile/,v2 v1 v1alpha1
 PAO_E2E_SUITES := $(shell hack/list-test-bin.sh)
 
 # golangci-lint variables
-GOLANGCI_LINT_VERSION=1.54.2
+GOLANGCI_LINT_VERSION=1.62.2
 GOLANGCI_LINT_BIN=$(OUT_DIR)/golangci-lint
 GOLANGCI_LINT_VERSION_TAG=v${GOLANGCI_LINT_VERSION}
 

cmd/gather-sysinfo/gather-sysinfo_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"io/ioutil"
 	"os"
 	"strings"
 	"testing"
@@ -63,7 +62,7 @@ func TestCollectMachineInfo(t *testing.T) {
 		t.Errorf("Collection of machine info failed: %v", err)
 	}
 
-	content, err := ioutil.ReadFile(destFile)
+	content, err := os.ReadFile(destFile)
 	if err != nil {
 		t.Errorf("Reading of generated output failed: %v", err)
 	}

pkg/apis/performanceprofile/performance_test.go

@@ -1,7 +1,7 @@
 package performance
 
 import (
-	"io/ioutil"
+	"os"
 	"strings"
 
 	"github.com/RHsyseng/operator-utils/pkg/validation"
@@ -53,7 +53,7 @@ var _ = Describe("PerformanceProfile CR(D) Schema", func() {
 // getSchema reads in & returns CRD schema file as openAPIV3Schema{} for validation usage.
 // See references operator-utils/validation/schema & go-openapi/spec/schema
 func getSchema(crdPath string) (validation.Schema, error) {
-	bytes, err := ioutil.ReadFile(crdPath)
+	bytes, err := os.ReadFile(crdPath)
 	if err != nil {
 		return nil, err
 	}
@@ -66,7 +66,7 @@ func getSchema(crdPath string) (validation.Schema, error) {
 
 // getCR unmarshals a *_cr.yaml file and returns the representing struct
 func getCR(crPath string) (map[string]interface{}, error) {
-	bytes, err := ioutil.ReadFile(crPath)
+	bytes, err := os.ReadFile(crPath)
 	if err != nil {
 		return nil, err
 	}

pkg/apis/performanceprofile/v2/performanceprofile_validation.go

@@ -216,11 +216,11 @@ func validateNoIntersectionExists(lists *components.CPULists, allErrs field.Erro
 func (r *PerformanceProfile) validateSelectors() field.ErrorList {
 	var allErrs field.ErrorList
 
-	if r.Spec.MachineConfigLabel != nil && len(r.Spec.MachineConfigLabel) > 1 {
+	if len(r.Spec.MachineConfigLabel) > 1 {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.machineConfigLabel"), r.Spec.MachineConfigLabel, "you should provide only 1 MachineConfigLabel"))
 	}
 
-	if r.Spec.MachineConfigPoolSelector != nil && len(r.Spec.MachineConfigPoolSelector) > 1 {
+	if len(r.Spec.MachineConfigPoolSelector) > 1 {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.machineConfigPoolSelector"), r.Spec.MachineConfigLabel, "you should provide only 1 MachineConfigPoolSelector"))
 	}
 

pkg/apis/performanceprofile/v2/performanceprofile_validation_test.go

@@ -12,7 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 const (
@@ -126,13 +126,13 @@ func NewPerformanceProfile(name string) *PerformanceProfile {
 				},
 			},
 			RealTimeKernel: &RealTimeKernel{
-				Enabled: pointer.Bool(true),
+				Enabled: ptr.To(true),
 			},
 			NUMA: &NUMA{
 				TopologyPolicy: &numaPolicy,
 			},
 			Net: &Net{
-				UserLevelNetworking: pointer.Bool(true),
+				UserLevelNetworking: ptr.To(true),
 				Devices: []Device{
 					{
 						InterfaceName: &netDeviceName,
@@ -381,7 +381,7 @@ var _ = Describe("PerformanceProfile", func() {
 			setValidNodeSelector(profile)
 
 			errors = profile.validateSelectors()
-			Expect(profile.validateSelectors()).To(BeEmpty(), "should not have validation errors when machine config selector nil")
+			Expect(errors).To(BeEmpty(), "should not have validation errors when machine config selector nil")
 		})
 
 		It("should should have 0 or 1 MachineConfigPoolSelector labels", func() {
@@ -397,7 +397,7 @@ var _ = Describe("PerformanceProfile", func() {
 			setValidNodeSelector(profile)
 
 			errors = profile.validateSelectors()
-			Expect(profile.validateSelectors()).To(BeEmpty(), "should not have validation errors when machine config pool selector nil")
+			Expect(errors).To(BeEmpty(), "should not have validation errors when machine config pool selector nil")
 		})
 
 		It("should have sensible NodeSelector in case MachineConfigLabel or MachineConfigPoolSelector is empty", func() {
@@ -583,7 +583,7 @@ var _ = Describe("PerformanceProfile", func() {
 
 			profile.Spec.HugePages.Pages = append(profile.Spec.HugePages.Pages, HugePage{
 				Count: 128,
-				Node:  pointer.Int32(0),
+				Node:  ptr.To(int32(0)),
 				Size:  "14M",
 			})
 			errors := profile.validateHugePages(nodes)
@@ -604,7 +604,7 @@ var _ = Describe("PerformanceProfile", func() {
 
 			profile.Spec.HugePages.Pages = append(profile.Spec.HugePages.Pages, HugePage{
 				Count: 128,
-				Node:  pointer.Int32(0),
+				Node:  ptr.To(int32(0)),
 				Size:  "14M",
 			})
 			errors := profile.validateHugePages(nodes)
@@ -635,7 +635,7 @@ var _ = Describe("PerformanceProfile", func() {
 
 			profile.Spec.HugePages.Pages = append(profile.Spec.HugePages.Pages, HugePage{
 				Count: 128,
-				Node:  pointer.Int32(0),
+				Node:  ptr.To(int32(0)),
 				Size:  "14M",
 			})
 
@@ -661,12 +661,12 @@ var _ = Describe("PerformanceProfile", func() {
 					profile.Spec.HugePages.Pages = append(profile.Spec.HugePages.Pages, HugePage{
 						Count: 128,
 						Size:  hugepagesSize1G,
-						Node:  pointer.Int32(0),
+						Node:  ptr.To(int32(0)),
 					})
 					profile.Spec.HugePages.Pages = append(profile.Spec.HugePages.Pages, HugePage{
 						Count: 64,
 						Size:  hugepagesSize1G,
-						Node:  pointer.Int32(0),
+						Node:  ptr.To(int32(0)),
 					})
 					errors := profile.validateHugePages(nodes)
 					Expect(errors).NotTo(BeEmpty())
@@ -729,33 +729,33 @@ var _ = Describe("PerformanceProfile", func() {
 			It("should raise the validation syntax errors", func() {
 				invalidVendor := "123"
 				invalidDevice := "0x12345"
-				profile.Spec.Net.Devices[0].InterfaceName = pointer.String("")
-				profile.Spec.Net.Devices[0].VendorID = pointer.String(invalidVendor)
-				profile.Spec.Net.Devices[0].DeviceID = pointer.String(invalidDevice)
+				profile.Spec.Net.Devices[0].InterfaceName = ptr.To("")
+				profile.Spec.Net.Devices[0].VendorID = ptr.To(invalidVendor)
+				profile.Spec.Net.Devices[0].DeviceID = ptr.To(invalidDevice)
 				errors := profile.validateNet()
 				Expect(len(errors)).To(Equal(3))
-				Expect(errors[0].Error()).To(ContainSubstring(fmt.Sprintf("device name cannot be empty")))
+				Expect(errors[0].Error()).To(ContainSubstring("device name cannot be empty"))
 				Expect(errors[1].Error()).To(ContainSubstring(fmt.Sprintf("device vendor ID %s has an invalid format. Vendor ID should be represented as 0x<4 hexadecimal digits> (16 bit representation)", invalidVendor)))
 				Expect(errors[2].Error()).To(ContainSubstring(fmt.Sprintf("device model ID %s has an invalid format. Model ID should be represented as 0x<4 hexadecimal digits> (16 bit representation)", invalidDevice)))
 
 			})
 			It("should raise the validation errors for missing fields", func() {
 				profile.Spec.Net.Devices[0].VendorID = nil
-				profile.Spec.Net.Devices[0].DeviceID = pointer.String("0x1")
+				profile.Spec.Net.Devices[0].DeviceID = ptr.To("0x1")
 				errors := profile.validateNet()
 				Expect(errors).NotTo(BeEmpty())
-				Expect(errors[0].Error()).To(ContainSubstring(fmt.Sprintf("device model ID can not be used without specifying the device vendor ID.")))
+				Expect(errors[0].Error()).To(ContainSubstring("device model ID can not be used without specifying the device vendor ID."))
 			})
 		})
 
 		Describe("Workload hints validation", func() {
 			When("realtime kernel is enabled and realtime workload hint is explicitly disabled", func() {
 				It("should raise validation error", func() {
 					profile.Spec.WorkloadHints = &WorkloadHints{
-						RealTime: pointer.Bool(false),
+						RealTime: ptr.To(false),
 					}
 					profile.Spec.RealTimeKernel = &RealTimeKernel{
-						Enabled: pointer.Bool(true),
+						Enabled: ptr.To(true),
 					}
 					errors := profile.validateWorkloadHints()
 					Expect(errors).NotTo(BeEmpty())
@@ -765,8 +765,8 @@ var _ = Describe("PerformanceProfile", func() {
 			When("HighPowerConsumption hint is enabled and PerPodPowerManagement hint is enabled", func() {
 				It("should raise validation error", func() {
 					profile.Spec.WorkloadHints = &WorkloadHints{
-						HighPowerConsumption:  pointer.Bool(true),
-						PerPodPowerManagement: pointer.Bool(true),
+						HighPowerConsumption:  ptr.To(true),
+						PerPodPowerManagement: ptr.To(true),
 					}
 					errors := profile.validateWorkloadHints()
 					Expect(errors).NotTo(BeEmpty())
@@ -776,7 +776,7 @@ var _ = Describe("PerformanceProfile", func() {
 			When("MixedCPUs hint is enabled but no shared CPUs are specified", func() {
 				It("should raise validation error", func() {
 					profile.Spec.WorkloadHints = &WorkloadHints{
-						MixedCpus: pointer.Bool(true),
+						MixedCpus: ptr.To(true),
 					}
 					errors := profile.validateWorkloadHints()
 					Expect(errors).NotTo(BeEmpty())
@@ -807,17 +807,17 @@ var _ = Describe("PerformanceProfile", func() {
 			profile.Spec.HugePages.DefaultHugePagesSize = &incorrectDefaultSize
 
 			profile.Spec.WorkloadHints = &WorkloadHints{
-				RealTime: pointer.Bool(false),
+				RealTime: ptr.To(false),
 			}
 			profile.Spec.RealTimeKernel = &RealTimeKernel{
-				Enabled: pointer.Bool(true),
+				Enabled: ptr.To(true),
 			}
 
 			invalidVendor := "123"
 			invalidDevice := "0x12345"
-			profile.Spec.Net.Devices[0].InterfaceName = pointer.String("")
-			profile.Spec.Net.Devices[0].VendorID = pointer.String(invalidVendor)
-			profile.Spec.Net.Devices[0].DeviceID = pointer.String(invalidDevice)
+			profile.Spec.Net.Devices[0].InterfaceName = ptr.To("")
+			profile.Spec.Net.Devices[0].VendorID = ptr.To(invalidVendor)
+			profile.Spec.Net.Devices[0].DeviceID = ptr.To(invalidDevice)
 
 			errors := profile.ValidateBasicFields()
 

pkg/operator/status.go

@@ -2,14 +2,15 @@ package operator
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/klog/v2"
@@ -80,7 +81,7 @@ func (c *Controller) syncOperatorStatus(tuned *tunedv1.Tuned) error {
 func (c *Controller) getOrCreateOperatorStatus() (*configv1.ClusterOperator, error) {
 	co, err := c.listers.ClusterOperators.Get(tunedv1.TunedClusterOperatorResourceName)
 	if err != nil {
-		if errors.IsNotFound(err) {
+		if apierrors.IsNotFound(err) {
 			// Cluster operator not found, create it
 			co = &configv1.ClusterOperator{ObjectMeta: metav1.ObjectMeta{Name: tunedv1.TunedClusterOperatorResourceName}}
 			co, err = c.clients.ConfigV1Client.ClusterOperators().Create(context.TODO(), co, metav1.CreateOptions{})
@@ -205,7 +206,7 @@ func (c *Controller) computeStatus(tuned *tunedv1.Tuned, conditions []configv1.C
 		ds, err := c.listers.DaemonSets.Get(dsMf.Name)
 		if err != nil {
 			// There was a problem fetching Tuned daemonset
-			if errors.IsNotFound(err) {
+			if apierrors.IsNotFound(err) {
 				// Tuned daemonset has not been created yet
 				if len(conditions) == 0 {
 					// This looks like a fresh install => initialize
@@ -238,7 +239,7 @@ func (c *Controller) computeStatus(tuned *tunedv1.Tuned, conditions []configv1.C
 		} else {
 			if ds.Status.ObservedGeneration != ds.Generation {
 				// Do not base the conditions on stale information
-				return conditions, "", fmt.Errorf(errGenerationMismatch)
+				return conditions, "", errors.New(errGenerationMismatch)
 			}
 
 			dsReleaseVersion := c.getDaemonSetReleaseVersion(ds)
@@ -303,14 +304,14 @@ func (c *Controller) computeStatus(tuned *tunedv1.Tuned, conditions []configv1.C
 		}
 
 		if numDegradedProfiles > 0 {
-			klog.Infof(fmt.Sprintf("%v/%v Profiles failed to be applied", numDegradedProfiles, len(profileList)))
+			klog.Infof("%v/%v Profiles failed to be applied", numDegradedProfiles, len(profileList))
 			availableCondition.Reason = "ProfileDegraded"
 			availableCondition.Message = fmt.Sprintf("%v/%v Profiles failed to be applied", numDegradedProfiles, len(profileList))
 		}
 
 		numConflict := c.numProfilesWithBootcmdlineConflict(profileList)
 		if numConflict > 0 {
-			klog.Infof(fmt.Sprintf("%v/%v Profiles with bootcmdline conflict", numConflict, len(profileList)))
+			klog.Infof("%v/%v Profiles with bootcmdline conflict", numConflict, len(profileList))
 			degradedCondition.Status = configv1.ConditionTrue
 			degradedCondition.Reason = "ProfileConflict"
 			degradedCondition.Message = fmt.Sprintf("%v/%v Profiles with bootcmdline conflict", numConflict, len(profileList))