From 8c4a38eec69ae9e973a56c61c8004641d36d6313 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 4 Apr 2024 10:52:46 +0200 Subject: [PATCH] rbd: address golangci-lint issues addressing golangci-lint issues in rbd related code. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 32 +++++++++++++++--------------- internal/rbd/driver/driver_test.go | 3 +-- internal/rbd/encryption.go | 2 +- internal/rbd/nodeserver.go | 9 +++++---- internal/rbd/nodeserver_test.go | 8 ++++---- internal/rbd/rbd_attach.go | 12 +++++------ internal/rbd/rbd_healer.go | 3 ++- internal/rbd/rbd_journal.go | 2 +- internal/rbd/rbd_util.go | 6 ++---- internal/rbd/rbd_util_test.go | 6 +++--- 10 files changed, 41 insertions(+), 42 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index e82f1e894..3b6c58d06 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -68,10 +68,10 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea return err } // Check sanity of request Name, Volume Capabilities - if req.Name == "" { + if req.GetName() == "" { return status.Error(codes.InvalidArgument, "volume Name cannot be empty") } - if req.VolumeCapabilities == nil { + if req.GetVolumeCapabilities() == nil { return status.Error(codes.InvalidArgument, "volume Capabilities cannot be empty") } options := req.GetParameters() @@ -105,7 +105,7 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea return err } - err = validateStriping(req.Parameters) + err = validateStriping(req.GetParameters()) if err != nil { return status.Error(codes.InvalidArgument, err.Error()) } @@ -156,13 +156,13 @@ func (cs *ControllerServer) parseVolCreateRequest( // below capability check indicates that we support both {SINGLE_NODE or MULTI_NODE} WRITERs and the `isMultiWriter` // flag has been set accordingly. - isMultiWriter, isBlock := csicommon.IsBlockMultiWriter(req.VolumeCapabilities) + isMultiWriter, isBlock := csicommon.IsBlockMultiWriter(req.GetVolumeCapabilities()) // below return value has set, if it is RWO mode File PVC. - isRWOFile := csicommon.IsFileRWO(req.VolumeCapabilities) + isRWOFile := csicommon.IsFileRWO(req.GetVolumeCapabilities()) // below return value has set, if it is ReadOnly capability. - isROOnly := csicommon.IsReaderOnly(req.VolumeCapabilities) + isROOnly := csicommon.IsReaderOnly(req.GetVolumeCapabilities()) // We want to fail early if the user is trying to create a RWX on a non-block type device if !isRWOFile && !isBlock && !isROOnly { return nil, status.Error( @@ -782,13 +782,13 @@ func checkContentSource( req *csi.CreateVolumeRequest, cr *util.Credentials, ) (*rbdVolume, *rbdSnapshot, error) { - if req.VolumeContentSource == nil { + if req.GetVolumeContentSource() == nil { return nil, nil, nil } - volumeSource := req.VolumeContentSource - switch volumeSource.Type.(type) { + volumeSource := req.GetVolumeContentSource() + switch volumeSource.GetType().(type) { case *csi.VolumeContentSource_Snapshot: - snapshot := req.VolumeContentSource.GetSnapshot() + snapshot := req.GetVolumeContentSource().GetSnapshot() if snapshot == nil { return nil, nil, status.Error(codes.NotFound, "volume Snapshot cannot be empty") } @@ -808,7 +808,7 @@ func checkContentSource( return nil, rbdSnap, nil case *csi.VolumeContentSource_Volume: - vol := req.VolumeContentSource.GetVolume() + vol := req.GetVolumeContentSource().GetVolume() if vol == nil { return nil, nil, status.Error(codes.NotFound, "volume cannot be empty") } @@ -1066,11 +1066,11 @@ func (cs *ControllerServer) ValidateVolumeCapabilities( return nil, status.Error(codes.InvalidArgument, "empty volume ID in request") } - if len(req.VolumeCapabilities) == 0 { + if len(req.GetVolumeCapabilities()) == 0 { return nil, status.Error(codes.InvalidArgument, "empty volume capabilities in request") } - for _, capability := range req.VolumeCapabilities { + for _, capability := range req.GetVolumeCapabilities() { if capability.GetAccessMode().GetMode() != csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER { return &csi.ValidateVolumeCapabilitiesResponse{Message: ""}, nil } @@ -1078,7 +1078,7 @@ func (cs *ControllerServer) ValidateVolumeCapabilities( return &csi.ValidateVolumeCapabilitiesResponse{ Confirmed: &csi.ValidateVolumeCapabilitiesResponse_Confirmed{ - VolumeCapabilities: req.VolumeCapabilities, + VolumeCapabilities: req.GetVolumeCapabilities(), }, }, nil } @@ -1297,10 +1297,10 @@ func (cs *ControllerServer) validateSnapshotReq(ctx context.Context, req *csi.Cr } // Check sanity of request Snapshot Name, Source Volume Id - if req.Name == "" { + if req.GetName() == "" { return status.Error(codes.InvalidArgument, "snapshot Name cannot be empty") } - if req.SourceVolumeId == "" { + if req.GetSourceVolumeId() == "" { return status.Error(codes.InvalidArgument, "source Volume ID cannot be empty") } diff --git a/internal/rbd/driver/driver_test.go b/internal/rbd/driver/driver_test.go index b1c5d884f..ea59f00a8 100644 --- a/internal/rbd/driver/driver_test.go +++ b/internal/rbd/driver/driver_test.go @@ -20,7 +20,6 @@ import ( "os" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ceph/ceph-csi/internal/util" @@ -44,7 +43,7 @@ func TestSetupCSIAddonsServer(t *testing.T) { // verify the socket file has been created _, err = os.Stat(tmpDir + "/csi-addons.sock") - assert.NoError(t, err) + require.NoError(t, err) // stop the gRPC server drv.cas.Stop() diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go index 828fafa12..015c1e564 100644 --- a/internal/rbd/encryption.go +++ b/internal/rbd/encryption.go @@ -312,7 +312,7 @@ func (ri *rbdImage) initKMS(ctx context.Context, volOptions, credentials map[str case util.EncryptionTypeFile: err = ri.configureFileEncryption(ctx, kmsID, credentials) case util.EncryptionTypeInvalid: - return fmt.Errorf("invalid encryption type") + return errors.New("invalid encryption type") case util.EncryptionTypeNone: return nil } diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index b1170b905..c203c82db 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -163,7 +163,7 @@ func (ns *NodeServer) populateRbdVol( isBlock := req.GetVolumeCapability().GetBlock() != nil disableInUseChecks := false // MULTI_NODE_MULTI_WRITER is supported by default for Block access type volumes - if req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER { + if req.GetVolumeCapability().GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER { if !isBlock { log.WarningLog( ctx, @@ -400,7 +400,7 @@ func (ns *NodeServer) stageTransaction( var err error // Allow image to be mounted on multiple nodes if it is ROX - if req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY { + if req.GetVolumeCapability().GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY { log.ExtendedLog(ctx, "setting disableInUseChecks on rbd volume to: %v", req.GetVolumeId) volOptions.DisableInUseChecks = true volOptions.readOnly = true @@ -777,8 +777,9 @@ func (ns *NodeServer) mountVolumeToStagePath( isBlock := req.GetVolumeCapability().GetBlock() != nil rOnly := "ro" - if req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || - req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { + mode := req.GetVolumeCapability().GetAccessMode().GetMode() + if mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || + mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { if !csicommon.MountOptionContains(opt, rOnly) { opt = append(opt, rOnly) } diff --git a/internal/rbd/nodeserver_test.go b/internal/rbd/nodeserver_test.go index 097769e74..91670eb78 100644 --- a/internal/rbd/nodeserver_test.go +++ b/internal/rbd/nodeserver_test.go @@ -27,7 +27,7 @@ import ( "github.com/ceph/ceph-csi/internal/util" "github.com/container-storage-interface/spec/lib/go/csi" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetStagingPath(t *testing.T) { @@ -196,7 +196,7 @@ func TestNodeServer_appendReadAffinityMapOptions(t *testing.T) { Mounter: currentTT.args.mounter, } rv.appendReadAffinityMapOptions(currentTT.args.readAffinityMapOptions) - assert.Equal(t, currentTT.want, rv.MapOptions) + require.Equal(t, currentTT.want, rv.MapOptions) }) } } @@ -310,10 +310,10 @@ func TestReadAffinity_GetReadAffinityMapOptions(t *testing.T) { tmpConfPath, tc.clusterID, ns.CLIReadAffinityOptions, nodeLabels, ) if err != nil { - assert.Fail(t, err.Error()) + require.Fail(t, err.Error()) } - assert.Equal(t, tc.want, readAffinityMapOptions) + require.Equal(t, tc.want, readAffinityMapOptions) }) } } diff --git a/internal/rbd/rbd_attach.go b/internal/rbd/rbd_attach.go index fbac9a3f7..36ba2d77b 100644 --- a/internal/rbd/rbd_attach.go +++ b/internal/rbd/rbd_attach.go @@ -229,7 +229,7 @@ func waitForPath(ctx context.Context, pool, namespace, image string, maxRetries func SetRbdNbdToolFeatures() { var stderr string // check if the module is loaded or compiled in - _, err := os.Stat(fmt.Sprintf("/sys/module/%s", moduleNbd)) + _, err := os.Stat("/sys/module/" + moduleNbd) if os.IsNotExist(err) { // try to load the module _, stderr, err = util.ExecCommand(context.TODO(), "modprobe", moduleNbd) @@ -377,7 +377,7 @@ func appendNbdDeviceTypeAndOptions(cmdArgs []string, userOptions, cookie string) } if hasNBDCookieSupport { - cmdArgs = append(cmdArgs, fmt.Sprintf("--cookie=%s", cookie)) + cmdArgs = append(cmdArgs, "--cookie="+cookie) } } @@ -409,7 +409,7 @@ func appendKRbdDeviceTypeAndOptions(cmdArgs []string, userOptions string) []stri // provided for rbd integrated cli to rbd-nbd cli format specific. func appendRbdNbdCliOptions(cmdArgs []string, userOptions, cookie string) []string { if !strings.Contains(userOptions, useNbdNetlink) { - cmdArgs = append(cmdArgs, fmt.Sprintf("--%s", useNbdNetlink)) + cmdArgs = append(cmdArgs, "--"+useNbdNetlink) } if !strings.Contains(userOptions, setNbdReattach) { cmdArgs = append(cmdArgs, fmt.Sprintf("--%s=%d", setNbdReattach, defaultNbdReAttachTimeout)) @@ -418,12 +418,12 @@ func appendRbdNbdCliOptions(cmdArgs []string, userOptions, cookie string) []stri cmdArgs = append(cmdArgs, fmt.Sprintf("--%s=%d", setNbdIOTimeout, defaultNbdIOTimeout)) } if hasNBDCookieSupport { - cmdArgs = append(cmdArgs, fmt.Sprintf("--cookie=%s", cookie)) + cmdArgs = append(cmdArgs, "--cookie="+cookie) } if userOptions != "" { options := strings.Split(userOptions, ",") for _, opt := range options { - cmdArgs = append(cmdArgs, fmt.Sprintf("--%s", opt)) + cmdArgs = append(cmdArgs, "--"+opt) } } @@ -566,7 +566,7 @@ func detachRBDImageOrDeviceSpec( return err } - if len(mapper) > 0 { + if mapper != "" { // mapper found, so it is open Luks device err = util.CloseEncryptedVolume(ctx, mapperFile) if err != nil { diff --git a/internal/rbd/rbd_healer.go b/internal/rbd/rbd_healer.go index 3014f7ffc..2bacb9aa7 100644 --- a/internal/rbd/rbd_healer.go +++ b/internal/rbd/rbd_healer.go @@ -19,6 +19,7 @@ package rbd import ( "context" "crypto/sha256" + "encoding/hex" "fmt" "path/filepath" "sync" @@ -79,7 +80,7 @@ func getSecret(c *k8s.Clientset, ns, name string) (map[string]string, error) { func formatStagingTargetPath(c *k8s.Clientset, pv *v1.PersistentVolume, stagingPath string) (string, error) { // Kubernetes 1.24+ uses a hash of the volume-id in the path name unique := sha256.Sum256([]byte(pv.Spec.CSI.VolumeHandle)) - targetPath := filepath.Join(stagingPath, pv.Spec.CSI.Driver, fmt.Sprintf("%x", unique), "globalmount") + targetPath := filepath.Join(stagingPath, pv.Spec.CSI.Driver, hex.EncodeToString(unique[:]), "globalmount") major, minor, err := kubeclient.GetServerVersion(c) if err != nil { diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 04ad9138e..69611fd08 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -294,7 +294,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er // NOTE: Return volsize should be on-disk volsize, not request vol size, so // save it for size checks before fetching image data - requestSize := rv.VolSize //nolint:ifshort // FIXME: rename and split function into helpers + requestSize := rv.VolSize // Fetch on-disk image attributes and compare against request err = rv.getImageInfo() if err != nil { diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index ae0ea389f..e38528932 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1163,8 +1163,6 @@ func generateVolumeFromVolumeID( // GenVolFromVolID generates a rbdVolume structure from the provided identifier, updating // the structure with elements from on-disk image metadata as well. -// -//nolint:golint // TODO: returning unexported rbdVolume type, use an interface instead. func GenVolFromVolID( ctx context.Context, volumeID string, @@ -1231,7 +1229,7 @@ func generateVolumeFromMapping( vi.ClusterID) // Add mapping clusterID to Identifier nvi.ClusterID = mappedClusterID - poolID := fmt.Sprintf("%d", (vi.LocationID)) + poolID := strconv.FormatInt(vi.LocationID, 10) for _, pools := range cm.RBDpoolIDMappingInfo { for key, val := range pools { mappedPoolID := util.GetMappedID(key, val, poolID) @@ -1525,7 +1523,7 @@ func (rv *rbdVolume) setImageOptions(ctx context.Context, options *librbd.ImageO logMsg := fmt.Sprintf("setting image options on %s", rv) if rv.DataPool != "" { - logMsg += fmt.Sprintf(", data pool %s", rv.DataPool) + logMsg += ", data pool %s" + rv.DataPool err = options.SetString(librbd.RbdImageOptionDataPool, rv.DataPool) if err != nil { return fmt.Errorf("failed to set data pool: %w", err) diff --git a/internal/rbd/rbd_util_test.go b/internal/rbd/rbd_util_test.go index b707af713..25a47d45b 100644 --- a/internal/rbd/rbd_util_test.go +++ b/internal/rbd/rbd_util_test.go @@ -24,7 +24,7 @@ import ( "testing" librbd "github.com/ceph/go-ceph/rbd" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestHasSnapshotFeature(t *testing.T) { @@ -165,11 +165,11 @@ func TestValidateImageFeatures(t *testing.T) { for _, test := range tests { err := test.rbdVol.validateImageFeatures(test.imageFeatures) if test.isErr { - assert.EqualError(t, err, test.errMsg) + require.EqualError(t, err, test.errMsg) continue } - assert.Nil(t, err) + require.NoError(t, err) } }