rbd: address golangci-lint issues

addressing golangci-lint issues in rbd
related code.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
This commit is contained in:
Madhu Rajanna 2024-04-04 10:52:46 +02:00 committed by mergify[bot]
parent b66b311770
commit 8c4a38eec6
10 changed files with 41 additions and 42 deletions

View File

@ -68,10 +68,10 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea
return err return err
} }
// Check sanity of request Name, Volume Capabilities // Check sanity of request Name, Volume Capabilities
if req.Name == "" { if req.GetName() == "" {
return status.Error(codes.InvalidArgument, "volume Name cannot be empty") 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") return status.Error(codes.InvalidArgument, "volume Capabilities cannot be empty")
} }
options := req.GetParameters() options := req.GetParameters()
@ -105,7 +105,7 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea
return err return err
} }
err = validateStriping(req.Parameters) err = validateStriping(req.GetParameters())
if err != nil { if err != nil {
return status.Error(codes.InvalidArgument, err.Error()) 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` // below capability check indicates that we support both {SINGLE_NODE or MULTI_NODE} WRITERs and the `isMultiWriter`
// flag has been set accordingly. // 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. // 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. // 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 // We want to fail early if the user is trying to create a RWX on a non-block type device
if !isRWOFile && !isBlock && !isROOnly { if !isRWOFile && !isBlock && !isROOnly {
return nil, status.Error( return nil, status.Error(
@ -782,13 +782,13 @@ func checkContentSource(
req *csi.CreateVolumeRequest, req *csi.CreateVolumeRequest,
cr *util.Credentials, cr *util.Credentials,
) (*rbdVolume, *rbdSnapshot, error) { ) (*rbdVolume, *rbdSnapshot, error) {
if req.VolumeContentSource == nil { if req.GetVolumeContentSource() == nil {
return nil, nil, nil return nil, nil, nil
} }
volumeSource := req.VolumeContentSource volumeSource := req.GetVolumeContentSource()
switch volumeSource.Type.(type) { switch volumeSource.GetType().(type) {
case *csi.VolumeContentSource_Snapshot: case *csi.VolumeContentSource_Snapshot:
snapshot := req.VolumeContentSource.GetSnapshot() snapshot := req.GetVolumeContentSource().GetSnapshot()
if snapshot == nil { if snapshot == nil {
return nil, nil, status.Error(codes.NotFound, "volume Snapshot cannot be empty") return nil, nil, status.Error(codes.NotFound, "volume Snapshot cannot be empty")
} }
@ -808,7 +808,7 @@ func checkContentSource(
return nil, rbdSnap, nil return nil, rbdSnap, nil
case *csi.VolumeContentSource_Volume: case *csi.VolumeContentSource_Volume:
vol := req.VolumeContentSource.GetVolume() vol := req.GetVolumeContentSource().GetVolume()
if vol == nil { if vol == nil {
return nil, nil, status.Error(codes.NotFound, "volume cannot be empty") 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") 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") 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 { if capability.GetAccessMode().GetMode() != csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER {
return &csi.ValidateVolumeCapabilitiesResponse{Message: ""}, nil return &csi.ValidateVolumeCapabilitiesResponse{Message: ""}, nil
} }
@ -1078,7 +1078,7 @@ func (cs *ControllerServer) ValidateVolumeCapabilities(
return &csi.ValidateVolumeCapabilitiesResponse{ return &csi.ValidateVolumeCapabilitiesResponse{
Confirmed: &csi.ValidateVolumeCapabilitiesResponse_Confirmed{ Confirmed: &csi.ValidateVolumeCapabilitiesResponse_Confirmed{
VolumeCapabilities: req.VolumeCapabilities, VolumeCapabilities: req.GetVolumeCapabilities(),
}, },
}, nil }, nil
} }
@ -1297,10 +1297,10 @@ func (cs *ControllerServer) validateSnapshotReq(ctx context.Context, req *csi.Cr
} }
// Check sanity of request Snapshot Name, Source Volume Id // 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") 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") return status.Error(codes.InvalidArgument, "source Volume ID cannot be empty")
} }

View File

@ -20,7 +20,6 @@ import (
"os" "os"
"testing" "testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util"
@ -44,7 +43,7 @@ func TestSetupCSIAddonsServer(t *testing.T) {
// verify the socket file has been created // verify the socket file has been created
_, err = os.Stat(tmpDir + "/csi-addons.sock") _, err = os.Stat(tmpDir + "/csi-addons.sock")
assert.NoError(t, err) require.NoError(t, err)
// stop the gRPC server // stop the gRPC server
drv.cas.Stop() drv.cas.Stop()

View File

@ -312,7 +312,7 @@ func (ri *rbdImage) initKMS(ctx context.Context, volOptions, credentials map[str
case util.EncryptionTypeFile: case util.EncryptionTypeFile:
err = ri.configureFileEncryption(ctx, kmsID, credentials) err = ri.configureFileEncryption(ctx, kmsID, credentials)
case util.EncryptionTypeInvalid: case util.EncryptionTypeInvalid:
return fmt.Errorf("invalid encryption type") return errors.New("invalid encryption type")
case util.EncryptionTypeNone: case util.EncryptionTypeNone:
return nil return nil
} }

View File

@ -163,7 +163,7 @@ func (ns *NodeServer) populateRbdVol(
isBlock := req.GetVolumeCapability().GetBlock() != nil isBlock := req.GetVolumeCapability().GetBlock() != nil
disableInUseChecks := false disableInUseChecks := false
// MULTI_NODE_MULTI_WRITER is supported by default for Block access type volumes // 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 { if !isBlock {
log.WarningLog( log.WarningLog(
ctx, ctx,
@ -400,7 +400,7 @@ func (ns *NodeServer) stageTransaction(
var err error var err error
// Allow image to be mounted on multiple nodes if it is ROX // 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) log.ExtendedLog(ctx, "setting disableInUseChecks on rbd volume to: %v", req.GetVolumeId)
volOptions.DisableInUseChecks = true volOptions.DisableInUseChecks = true
volOptions.readOnly = true volOptions.readOnly = true
@ -777,8 +777,9 @@ func (ns *NodeServer) mountVolumeToStagePath(
isBlock := req.GetVolumeCapability().GetBlock() != nil isBlock := req.GetVolumeCapability().GetBlock() != nil
rOnly := "ro" rOnly := "ro"
if req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || mode := req.GetVolumeCapability().GetAccessMode().GetMode()
req.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { if mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY ||
mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY {
if !csicommon.MountOptionContains(opt, rOnly) { if !csicommon.MountOptionContains(opt, rOnly) {
opt = append(opt, rOnly) opt = append(opt, rOnly)
} }

View File

@ -27,7 +27,7 @@ import (
"github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util"
"github.com/container-storage-interface/spec/lib/go/csi" "github.com/container-storage-interface/spec/lib/go/csi"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/require"
) )
func TestGetStagingPath(t *testing.T) { func TestGetStagingPath(t *testing.T) {
@ -196,7 +196,7 @@ func TestNodeServer_appendReadAffinityMapOptions(t *testing.T) {
Mounter: currentTT.args.mounter, Mounter: currentTT.args.mounter,
} }
rv.appendReadAffinityMapOptions(currentTT.args.readAffinityMapOptions) 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, tmpConfPath, tc.clusterID, ns.CLIReadAffinityOptions, nodeLabels,
) )
if err != nil { 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)
}) })
} }
} }

View File

@ -229,7 +229,7 @@ func waitForPath(ctx context.Context, pool, namespace, image string, maxRetries
func SetRbdNbdToolFeatures() { func SetRbdNbdToolFeatures() {
var stderr string var stderr string
// check if the module is loaded or compiled in // 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) { if os.IsNotExist(err) {
// try to load the module // try to load the module
_, stderr, err = util.ExecCommand(context.TODO(), "modprobe", moduleNbd) _, stderr, err = util.ExecCommand(context.TODO(), "modprobe", moduleNbd)
@ -377,7 +377,7 @@ func appendNbdDeviceTypeAndOptions(cmdArgs []string, userOptions, cookie string)
} }
if hasNBDCookieSupport { 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. // provided for rbd integrated cli to rbd-nbd cli format specific.
func appendRbdNbdCliOptions(cmdArgs []string, userOptions, cookie string) []string { func appendRbdNbdCliOptions(cmdArgs []string, userOptions, cookie string) []string {
if !strings.Contains(userOptions, useNbdNetlink) { if !strings.Contains(userOptions, useNbdNetlink) {
cmdArgs = append(cmdArgs, fmt.Sprintf("--%s", useNbdNetlink)) cmdArgs = append(cmdArgs, "--"+useNbdNetlink)
} }
if !strings.Contains(userOptions, setNbdReattach) { if !strings.Contains(userOptions, setNbdReattach) {
cmdArgs = append(cmdArgs, fmt.Sprintf("--%s=%d", setNbdReattach, defaultNbdReAttachTimeout)) 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)) cmdArgs = append(cmdArgs, fmt.Sprintf("--%s=%d", setNbdIOTimeout, defaultNbdIOTimeout))
} }
if hasNBDCookieSupport { if hasNBDCookieSupport {
cmdArgs = append(cmdArgs, fmt.Sprintf("--cookie=%s", cookie)) cmdArgs = append(cmdArgs, "--cookie="+cookie)
} }
if userOptions != "" { if userOptions != "" {
options := strings.Split(userOptions, ",") options := strings.Split(userOptions, ",")
for _, opt := range options { for _, opt := range options {
cmdArgs = append(cmdArgs, fmt.Sprintf("--%s", opt)) cmdArgs = append(cmdArgs, "--"+opt)
} }
} }
@ -566,7 +566,7 @@ func detachRBDImageOrDeviceSpec(
return err return err
} }
if len(mapper) > 0 { if mapper != "" {
// mapper found, so it is open Luks device // mapper found, so it is open Luks device
err = util.CloseEncryptedVolume(ctx, mapperFile) err = util.CloseEncryptedVolume(ctx, mapperFile)
if err != nil { if err != nil {

View File

@ -19,6 +19,7 @@ package rbd
import ( import (
"context" "context"
"crypto/sha256" "crypto/sha256"
"encoding/hex"
"fmt" "fmt"
"path/filepath" "path/filepath"
"sync" "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) { 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 // Kubernetes 1.24+ uses a hash of the volume-id in the path name
unique := sha256.Sum256([]byte(pv.Spec.CSI.VolumeHandle)) 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) major, minor, err := kubeclient.GetServerVersion(c)
if err != nil { if err != nil {

View File

@ -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 // NOTE: Return volsize should be on-disk volsize, not request vol size, so
// save it for size checks before fetching image data // 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 // Fetch on-disk image attributes and compare against request
err = rv.getImageInfo() err = rv.getImageInfo()
if err != nil { if err != nil {

View File

@ -1163,8 +1163,6 @@ func generateVolumeFromVolumeID(
// GenVolFromVolID generates a rbdVolume structure from the provided identifier, updating // GenVolFromVolID generates a rbdVolume structure from the provided identifier, updating
// the structure with elements from on-disk image metadata as well. // the structure with elements from on-disk image metadata as well.
//
//nolint:golint // TODO: returning unexported rbdVolume type, use an interface instead.
func GenVolFromVolID( func GenVolFromVolID(
ctx context.Context, ctx context.Context,
volumeID string, volumeID string,
@ -1231,7 +1229,7 @@ func generateVolumeFromMapping(
vi.ClusterID) vi.ClusterID)
// Add mapping clusterID to Identifier // Add mapping clusterID to Identifier
nvi.ClusterID = mappedClusterID nvi.ClusterID = mappedClusterID
poolID := fmt.Sprintf("%d", (vi.LocationID)) poolID := strconv.FormatInt(vi.LocationID, 10)
for _, pools := range cm.RBDpoolIDMappingInfo { for _, pools := range cm.RBDpoolIDMappingInfo {
for key, val := range pools { for key, val := range pools {
mappedPoolID := util.GetMappedID(key, val, poolID) 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) logMsg := fmt.Sprintf("setting image options on %s", rv)
if rv.DataPool != "" { if rv.DataPool != "" {
logMsg += fmt.Sprintf(", data pool %s", rv.DataPool) logMsg += ", data pool %s" + rv.DataPool
err = options.SetString(librbd.RbdImageOptionDataPool, rv.DataPool) err = options.SetString(librbd.RbdImageOptionDataPool, rv.DataPool)
if err != nil { if err != nil {
return fmt.Errorf("failed to set data pool: %w", err) return fmt.Errorf("failed to set data pool: %w", err)

View File

@ -24,7 +24,7 @@ import (
"testing" "testing"
librbd "github.com/ceph/go-ceph/rbd" librbd "github.com/ceph/go-ceph/rbd"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/require"
) )
func TestHasSnapshotFeature(t *testing.T) { func TestHasSnapshotFeature(t *testing.T) {
@ -165,11 +165,11 @@ func TestValidateImageFeatures(t *testing.T) {
for _, test := range tests { for _, test := range tests {
err := test.rbdVol.validateImageFeatures(test.imageFeatures) err := test.rbdVol.validateImageFeatures(test.imageFeatures)
if test.isErr { if test.isErr {
assert.EqualError(t, err, test.errMsg) require.EqualError(t, err, test.errMsg)
continue continue
} }
assert.Nil(t, err) require.NoError(t, err)
} }
} }