Merge pull request #6286 from subhamkrai/golanci-lint-staticcheck

ceph: handle golangci-lint linter ineffassign

.github/workflows/golangci-lint.yaml

@@ -24,6 +24,6 @@ jobs:
           # working-directory: somedir
 
           # Optional: golangci-lint command line arguments.
-          args: --disable-all -E gosimple,unused,deadcode --timeout=4m
+          args: --disable-all -E gosimple,unused,deadcode,ineffassign --timeout=4m
           # Optional: show only new issues if it's a pull request. The default value is `false`.
           # only-new-issues: true

cmd/rook/ceph/osd_test.go

@@ -137,6 +137,7 @@ func TestParseDesiredDevices(t *testing.T) {
 	devices = string(marshalledDevices)
 
 	result, err = parseDevices(devices)
+	assert.NoError(t, err)
 	assert.Equal(t, "sdd", result[0].Name)
 	assert.Equal(t, "sde", result[1].Name)
 	assert.Equal(t, "sdf", result[2].Name)

pkg/apis/edgefs.rook.io/v1/utils.go

@@ -40,7 +40,6 @@ func ByteCountBinary(b uint64) string {
 func GetModifiedRookImagePath(originRookImage, addon string) string {
 	imageParts := strings.Split(originRookImage, "/")
 	latestImagePartIndex := len(imageParts) - 1
-	modifiedImageName := "edgefs"
 	modifiedImageTag := "latest"
 
 	latestImagePart := imageParts[latestImagePartIndex]
@@ -49,6 +48,7 @@ func GetModifiedRookImagePath(originRookImage, addon string) string {
 		modifiedImageTag = imageVersionParts[1]
 	}
 
+	var modifiedImageName string
 	if len(addon) > 0 {
 		modifiedImageName = fmt.Sprintf("%s-%s", imageVersionParts[0], addon)
 	} else {

pkg/daemon/ceph/agent/flexvolume/server_test.go

@@ -58,15 +58,15 @@ func TestConfigureFlexVolume(t *testing.T) {
 
 func TestGetFlexDriverInfo(t *testing.T) {
 	// empty string, can't do anything with that, this is an error
-	vendor, driver, err := getFlexDriverInfo("")
+	_, _, err := getFlexDriverInfo("")
 	assert.NotNil(t, err)
 
 	// no driver dir found, this is an error
-	vendor, driver, err = getFlexDriverInfo("/a/b/c")
+	_, _, err = getFlexDriverInfo("/a/b/c")
 	assert.NotNil(t, err)
 
 	// well formed flex driver path, driver dir is last dir
-	vendor, driver, err = getFlexDriverInfo("/usr/libexec/kubernetes/kubelet-plugins/volume/exec/foo.bar.baz~biz")
+	vendor, driver, err := getFlexDriverInfo("/usr/libexec/kubernetes/kubelet-plugins/volume/exec/foo.bar.baz~biz")
 	assert.Nil(t, err)
 	assert.Equal(t, "foo.bar.baz", vendor)
 	assert.Equal(t, "biz", driver)
@@ -84,6 +84,6 @@ func TestGetFlexDriverInfo(t *testing.T) {
 	assert.Equal(t, "biz", driver)
 
 	// more flex volume info items than expected, this is an error
-	vendor, driver, err = getFlexDriverInfo("/usr/libexec/kubernetes/kubelet-plugins/volume/exec/foo.bar.baz~biz~buzz/")
+	_, _, err = getFlexDriverInfo("/usr/libexec/kubernetes/kubelet-plugins/volume/exec/foo.bar.baz~biz~buzz/")
 	assert.NotNil(t, err)
 }

pkg/daemon/ceph/client/erasure-code-profile_test.go

@@ -76,7 +76,6 @@ func testCreateProfile(t *testing.T, failureDomain, crushRoot, deviceClass strin
 				}
 				if deviceClass != "" {
 					assert.Equal(t, fmt.Sprintf("crush-device-class=%s", deviceClass), args[nextArg])
-					nextArg++
 				}
 				return "", nil
 			}

pkg/daemon/ceph/client/filesystem.go

@@ -128,12 +128,11 @@ func CreateFilesystem(context *clusterd.Context, clusterInfo *ClusterInfo, name,
 	}
 
 	logger.Infof("creating filesystem %q with metadata pool %q and data pools %v", name, metadataPool, dataPools)
-	args := []string{}
 	var err error
 
 	if IsMultiFSEnabled() {
 		// enable multiple file systems in case this is not the first
-		args = []string{"fs", "flag", "set", "enable_multiple", "true", confirmFlag}
+		args := []string{"fs", "flag", "set", "enable_multiple", "true", confirmFlag}
 		_, err = NewCephCommand(context, clusterInfo, args).Run()
 		if err != nil {
 			// continue if this fails
@@ -142,7 +141,7 @@ func CreateFilesystem(context *clusterd.Context, clusterInfo *ClusterInfo, name,
 	}
 
 	// create the filesystem
-	args = []string{"fs", "new", name, metadataPool, dataPools[0]}
+	args := []string{"fs", "new", name, metadataPool, dataPools[0]}
 	// Force to use pre-existing pools
 	if force {
 		args = append(args, "--force")

pkg/daemon/ceph/client/upgrade_test.go

@@ -152,7 +152,7 @@ func TestDaemonMapEntry(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, dummyVersions.Mon, m)
 
-	m, err = daemonMapEntry(&dummyVersions, "dummy")
+	_, err = daemonMapEntry(&dummyVersions, "dummy")
 	assert.Error(t, err)
 }
 

pkg/daemon/ceph/osd/volume_test.go

@@ -310,7 +310,7 @@ func TestInitializeBlockPVC(t *testing.T) {
 		},
 	}
 
-	blockPath, metadataBlockPath, walBlockPath, err = a.initializeBlockPVC(context, devices, false)
+	_, _, _, err = a.initializeBlockPVC(context, devices, false)
 	assert.NotNil(t, err)
 
 	executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
@@ -351,7 +351,7 @@ func TestInitializeBlockPVC(t *testing.T) {
 		},
 	}
 
-	blockPath, metadataBlockPath, walBlockPath, err = a.initializeBlockPVC(context, devices, false)
+	_, _, _, err = a.initializeBlockPVC(context, devices, false)
 	assert.NotNil(t, err)
 
 	executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {

pkg/operator/ceph/cluster/mgr/dashboard_test.go

@@ -65,6 +65,7 @@ func TestGetOrGeneratePassword(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, 1, len(secret.Data))
 	passwordFromSecret, err := decodeSecret(secret)
+	assert.NoError(t, err)
 	assert.Equal(t, password, passwordFromSecret)
 
 	// We should retrieve the same password on the second call

pkg/operator/ceph/cluster/mon/mon_test.go

@@ -317,6 +317,7 @@ func TestWaitForQuorum(t *testing.T) {
 		return clienttest.MonInQuorumResponseFromMons(mons), nil
 	}
 	context, err := newTestStartClusterWithQuorumResponse(t, namespace, quorumResponse)
+	assert.NoError(t, err)
 	requireAllInQuorum := false
 	expectedMons := []string{"a"}
 	clusterInfo := &client.ClusterInfo{Namespace: namespace}

pkg/operator/ceph/config/network_test.go

@@ -44,7 +44,7 @@ func TestGenerateNetworkSettings(t *testing.T) {
 		"public": "public-network-attach-def",
 	}
 
-	cephNetwork, err := generateNetworkSettings(ctx, ns, netSelector)
+	_, err := generateNetworkSettings(ctx, ns, netSelector)
 	assert.Error(t, err)
 
 	//
@@ -87,7 +87,7 @@ func TestGenerateNetworkSettings(t *testing.T) {
 	// Create public network definition
 	ctx.NetworkClient.NetworkAttachmentDefinitions(ns).Create(network)
 
-	cephNetwork, err = generateNetworkSettings(ctx, ns, netSelector)
+	cephNetwork, err := generateNetworkSettings(ctx, ns, netSelector)
 	assert.NoError(t, err)
 	assert.ElementsMatch(t, cephNetwork, expectedNetworks, fmt.Sprintf("networks: %+v", cephNetwork))
 

pkg/operator/ceph/file/filesystem_test.go

@@ -153,9 +153,6 @@ func TestCreateFilesystem(t *testing.T) {
 func TestCreateNopoolFilesystem(t *testing.T) {
 	clientset := testop.New(t, 3)
 	configDir, _ := ioutil.TempDir("", "")
-	// Output to check multiple filesystem creation
-	fses := `[{"name":"myfs"}]`
-
 	executor := &exectest.MockExecutor{
 		MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) {
 			return "{\"key\":\"mysecurekey\"}", nil
@@ -193,16 +190,6 @@ func TestCreateNopoolFilesystem(t *testing.T) {
 	err = createFilesystem(context, clusterInfo, fs, &cephv1.ClusterSpec{}, metav1.OwnerReference{}, "/var/lib/rook/", scheme.Scheme)
 	assert.Nil(t, err)
 	validateStart(t, context, fs)
-
-	// Test multiple filesystem creation
-	executor = &exectest.MockExecutor{
-		MockExecuteCommandWithOutputFile: func(command string, outFileArg string, args ...string) (string, error) {
-			if contains(args, "ls") {
-				return fses, nil
-			}
-			return "{\"key\":\"mysecurekey\"}", errors.New("multiple fs")
-		},
-	}
 }
 func contains(arr []string, str string) bool {
 	for _, a := range arr {

pkg/operator/ceph/nfs/controller.go

@@ -223,7 +223,7 @@ func (r *ReconcileCephNFS) reconcile(request reconcile.Request) (reconcile.Resul
 
 	// CREATE/UPDATE
 	logger.Debug("reconciling ceph nfs deployments")
-	reconcileResponse, err = r.reconcileCreateCephNFS(cephNFS)
+	_, err = r.reconcileCreateCephNFS(cephNFS)
 	if err != nil {
 		updateStatus(r.client, request.NamespacedName, k8sutil.FailedStatus)
 		return reconcile.Result{}, errors.Wrap(err, "failed to create ceph nfs deployments")

pkg/operator/ceph/object/bucket/provisioner.go

@@ -518,7 +518,6 @@ func (p Provisioner) setAdditionalSettings(options *apibkt.BucketOptions) error
 		}
 	}
 	if maxSize != "" {
-		_, err = cephObject.SetQuotaUserMaxSize(p.objectContext, p.cephUserName, maxSize)
 		if _, err = cephObject.SetQuotaUserMaxSize(p.objectContext, p.cephUserName, maxSize); err != nil {
 			logger.Errorf("Setting MaxSize failed %v", err)
 			return err

pkg/operator/ceph/object/controller.go

@@ -271,7 +271,7 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci
 	}
 
 	// CREATE/UPDATE
-	reconcileResponse, err = r.reconcileCreateObjectStore(cephObjectStore, request.NamespacedName)
+	_, err = r.reconcileCreateObjectStore(cephObjectStore, request.NamespacedName)
 	if err != nil {
 		return r.setFailedStatus(request.NamespacedName, "failed to create object store deployments", err)
 	}

pkg/operator/ceph/object/controller_test.go

@@ -393,7 +393,8 @@ func TestCephObjectStoreController(t *testing.T) {
 			Kind: "CephObjectStoreUser",
 		},
 	}
-	objectUser, _ = r.context.RookClientset.CephV1().CephObjectStoreUsers(objectStore.Namespace).Create(objectUser)
+	_, err = r.context.RookClientset.CephV1().CephObjectStoreUsers(objectStore.Namespace).Create(objectUser)
+	assert.NoError(t, err)
 	_, okToDelete = r.verifyObjectUserCleanup(objectStore)
 	assert.False(t, okToDelete)
 	logger.Info("PHASE 4 DONE")

pkg/operator/ceph/object/objectstore.go

@@ -108,6 +108,10 @@ func removeObjectStoreFromMultisite(objContext *Context, spec cephv1.ObjectStore
 	endpointArg := fmt.Sprintf("--endpoints=%s", zoneEndpoints)
 
 	zoneIsMaster, err := checkZoneIsMaster(objContext)
+	if err != nil {
+		return errors.Wrap(err, "failed to find out zone in Master")
+	}
+
 	zoneGroupIsMaster := false
 	if zoneIsMaster {
 		_, err = RunAdminCommandNoMultisite(objContext, "zonegroup", "modify", realmArg, zoneGroupArg, endpointArg)

pkg/operator/ceph/object/objectstore_test.go

@@ -243,6 +243,7 @@ func TestDashboard(t *testing.T) {
 	context := &clusterd.Context{Executor: executor}
 	objContext := NewContext(context, &client.ClusterInfo{Namespace: "mycluster"}, storeName)
 	checkdashboard, err := checkDashboardUser(objContext)
+	assert.NoError(t, err)
 	assert.False(t, checkdashboard)
 	err = enableRGWDashboard(objContext)
 	assert.Nil(t, err)
@@ -256,6 +257,7 @@ func TestDashboard(t *testing.T) {
 	}
 	objContext.Context.Executor = executor
 	checkdashboard, err = checkDashboardUser(objContext)
+	assert.NoError(t, err)
 	assert.True(t, checkdashboard)
 	err = disableRGWDashboard(objContext)
 	assert.Nil(t, err)

pkg/operator/ceph/object/realm/controller.go

@@ -184,17 +184,17 @@ func (r *ReconcileObjectRealm) reconcile(request reconcile.Request) (reconcile.R
 	// Create/Pull Ceph Realm
 	if cephObjectRealm.Spec.IsPullRealm() {
 		logger.Debug("pull section in spec found")
-		reconcileResponse, err = r.pullCephRealm(cephObjectRealm)
+		_, err = r.pullCephRealm(cephObjectRealm)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 	} else {
-		reconcileResponse, err = r.createRealmKeys(cephObjectRealm)
+		_, err = r.createRealmKeys(cephObjectRealm)
 		if err != nil {
 			return r.setFailedStatus(request.NamespacedName, "failed to create keys for realm", err)
 		}
 
-		reconcileResponse, err = r.createCephRealm(cephObjectRealm)
+		_, err = r.createCephRealm(cephObjectRealm)
 		if err != nil {
 			return r.setFailedStatus(request.NamespacedName, "failed to create ceph realm", err)
 		}

pkg/operator/ceph/object/user/controller_test.go

@@ -292,6 +292,7 @@ func TestCephObjectStoreUserController(t *testing.T) {
 	assert.NoError(t, err)
 	assert.False(t, res.Requeue)
 	err = r.client.Get(context.TODO(), req.NamespacedName, objectUser)
+	assert.NoError(t, err)
 	assert.Equal(t, "Ready", objectUser.Status.Phase, objectUser)
 	logger.Info("PHASE 5 DONE")
 }

pkg/operator/ceph/object/zone/controller.go

@@ -190,7 +190,7 @@ func (r *ReconcileObjectZone) reconcile(request reconcile.Request) (reconcile.Re
 	}
 
 	// Create Ceph Zone
-	reconcileResponse, err = r.createCephZone(cephObjectZone, realmName)
+	_, err = r.createCephZone(cephObjectZone, realmName)
 	if err != nil {
 		return r.setFailedStatus(request.NamespacedName, "failed to create ceph zone", err)
 	}

pkg/operator/ceph/object/zonegroup/controller.go

@@ -188,7 +188,7 @@ func (r *ReconcileObjectZoneGroup) reconcile(request reconcile.Request) (reconci
 	}
 
 	// Create/Update Ceph Zone Group
-	reconcileResponse, err = r.createCephZoneGroup(cephObjectZoneGroup)
+	_, err = r.createCephZoneGroup(cephObjectZoneGroup)
 	if err != nil {
 		return r.setFailedStatus(request.NamespacedName, "failed to create ceph zone group", err)
 	}

pkg/operator/ceph/version/version.go

@@ -50,7 +50,6 @@ var (
 	// supportedVersions are production-ready versions that rook supports
 	supportedVersions   = []CephVersion{Nautilus, Octopus}
 	unsupportedVersions = []CephVersion{Pacific}
-	// allVersions includes all supportedVersions as well as unreleased versions that are being tested with rook
 
 	// for parsing the output of `ceph --version`
 	versionPattern = regexp.MustCompile(`ceph version (\d+)\.(\d+)\.(\d+)`)

pkg/operator/k8sutil/delete_test.go

@@ -64,6 +64,7 @@ func TestDeleteResource(t *testing.T) {
 	deleteError = nil
 	verifyCount = 0
 	err = DeleteResource(stubDelete, stubVerify, "test resource name", &DeleteOptions{}, defaultWaitOpts)
+	assert.NoError(t, err)
 	assert.Zero(t, verifyCount)
 
 	// Verify that the default wait options are picked up

pkg/operator/test/volumes.go

@@ -21,7 +21,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 /*
@@ -86,7 +86,7 @@ func HumanReadableVolumes(volumes []v1.Volume) string {
 // HumanReadableVolume returns a string representation of a Kubernetes volume which is more compact
 // and human readable than the default string go prints.
 func HumanReadableVolume(v *v1.Volume) string {
-	sourceString := "something went wrong parsing the volume source in HumanReadableVolume()"
+	var sourceString string
 	if v.VolumeSource.EmptyDir != nil {
 		sourceString = "EmptyDir"
 	} else if v.VolumeSource.HostPath != nil {

pkg/operator/yugabytedb/controller_test.go

@@ -418,7 +418,7 @@ func TestOnAdd(t *testing.T) {
 	assert.Equal(t, 0, (&limCPU).Cmp(resource.MustParse(podCPULimitDefault)))
 
 	reqMem, reqOk := container.Resources.Requests[v1.ResourceMemory]
-	limMem, limOk := container.Resources.Limits[v1.ResourceMemory]
+	limMem := container.Resources.Limits[v1.ResourceMemory]
 	assert.True(t, reqOk)
 	assert.True(t, reqOk)
 	assert.Equal(t, 0, (&reqMem).Cmp(resource.MustParse(masterMemLimitDefault)))
@@ -508,7 +508,7 @@ func TestOnAdd(t *testing.T) {
 
 	reqMem, reqOk = container.Resources.Requests[v1.ResourceMemory]
 	limMem, limOk = container.Resources.Limits[v1.ResourceMemory]
-	assert.True(t, reqOk)
+	assert.True(t, limOk)
 	assert.True(t, reqOk)
 	assert.Equal(t, 0, (&reqMem).Cmp(resource.MustParse(tserverMemLimitDefault)))
 	assert.Equal(t, 0, (&limMem).Cmp(resource.MustParse(tserverMemLimitDefault)))
@@ -1046,6 +1046,7 @@ func verifyAllComponentsExist(t *testing.T, clientset *fake.Clientset, namespace
 
 	// verify Master statefulSet is created
 	statefulSets, err := clientset.AppsV1().StatefulSets(namespace).Get(addCRNameSuffix(masterName), metav1.GetOptions{})
+	assert.NoError(t, err)
 	assert.NotNil(t, statefulSets)
 
 	// verify Master pods are created

pkg/util/sys/device_test.go

@@ -169,11 +169,11 @@ NAME="ceph--89fa04fa--b93a--4874--9364--c95be3ec01c6-osd--data--70847bdb--2ec1--
 	assert.Equal(t, uint64(0x400000), unused)
 	assert.Equal(t, 7, len(partitions))
 
-	partitions, unused, err = GetDevicePartitions("dm-0", executor)
+	partitions, _, err = GetDevicePartitions("dm-0", executor)
 	assert.Nil(t, err)
 	assert.Equal(t, 1, len(partitions))
 
-	partitions, unused, err = GetDevicePartitions("sdx", executor)
+	partitions, _, err = GetDevicePartitions("sdx", executor)
 	assert.Nil(t, err)
 	assert.Equal(t, 0, len(partitions))
 }

tests/framework/installer/cassandra_installer.go

@@ -22,6 +22,7 @@ import (
 
 	cassandrav1alpha1 "github.com/rook/rook/pkg/apis/cassandra.rook.io/v1alpha1"
 	"github.com/rook/rook/tests/framework/utils"
+	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -130,6 +131,7 @@ func (ci *CassandraInstaller) DeleteCassandraCluster(namespace string) {
 		return err
 	}
 	err = ci.k8sHelper.WaitForCustomResourceDeletion(namespace, crdCheckerFunc)
+	assert.NoError(ci.T(), err)
 
 	// Delete Namespace
 	logger.Infof("Deleting Cassandra Cluster namespace %s", namespace)

tests/framework/installer/ceph_installer.go

@@ -571,6 +571,9 @@ func (h *CephInstaller) UninstallRookFromMultipleNS(systemNamespace string, name
 
 		roles := h.Manifests.GetClusterRoles(namespace, systemNamespace)
 		_, err = h.k8shelper.KubectlWithStdin(roles, deleteFromStdinArgs...)
+		if err != nil {
+			logger.Errorf("failed to delete cluster roles. %v ", err)
+		}
 
 		crdCheckerFunc := func() error {
 			_, err := h.k8shelper.RookClientset.CephV1().CephClusters(namespace).Get(h.clusterName, metav1.GetOptions{})

tests/integration/ceph_base_block_test.go

@@ -396,9 +396,11 @@ func restartOSDPods(k8sh *utils.K8sHelper, s suite.Suite, namespace string) {
 	// Delete the osd pod(s)
 	logger.Infof("Deleting osd pod(s)")
 	pods, err := k8sh.Clientset.CoreV1().Pods(namespace).List(metav1.ListOptions{LabelSelector: osdLabel})
+	assert.NoError(s.T(), err)
+
 	for _, pod := range pods.Items {
 		options := metav1.DeleteOptions{}
-		err = k8sh.Clientset.CoreV1().Pods(namespace).Delete(pod.Name, &options)
+		err := k8sh.Clientset.CoreV1().Pods(namespace).Delete(pod.Name, &options)
 		assert.NoError(s.T(), err)
 
 		logger.Infof("Waiting for osd pod %s to be deleted", pod.Name)

tests/integration/ceph_smoke_test.go

@@ -250,6 +250,7 @@ func (suite *SmokeSuite) TestPoolResize() {
 
 	// Verify the Kubernetes Secret has been created (bootstrap peer token)
 	pool, err := suite.k8sh.RookClientset.CephV1().CephBlockPools(suite.namespace).Get(poolName, metav1.GetOptions{})
+	assert.NoError(suite.T(), err)
 	if pool.Spec.Mirroring.Enabled {
 		secretName := pool.Status.Info[oppool.RBDMirrorBootstrapPeerSecretName]
 		assert.NotEmpty(suite.T(), secretName)

tests/integration/ceph_upgrade_test.go

@@ -192,8 +192,8 @@ func (s *UpgradeSuite) TestUpgradeToMaster() {
 	// Verify reading and writing to the test clients
 	newFile = "post-octopus-upgrade-file"
 	s.verifyFilesAfterUpgrade(filesystemName, newFile, message, rbdFilesToRead, cephfsFilesToRead)
-	rbdFilesToRead = append(rbdFilesToRead, newFile)
-	cephfsFilesToRead = append(cephfsFilesToRead, newFile)
+	_ = append(rbdFilesToRead, newFile)
+	_ = append(cephfsFilesToRead, newFile)
 	logger.Infof("Verified upgrade from nautilus to octopus")
 }