From a362ef6bd420b5ee32070fe1548140a0ad7c6c34 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 4 Apr 2024 10:50:20 +0200 Subject: [PATCH] cephfs: address golangci-lint issues address golangci-lint issues in cephfs related code. Signed-off-by: Madhu Rajanna --- internal/cephfs/controllerserver.go | 22 +++++++++++----------- internal/cephfs/core/clone_test.go | 5 ++--- internal/cephfs/core/filesystem.go | 6 +++--- internal/cephfs/driver_test.go | 3 +-- internal/cephfs/groupcontrollerserver.go | 8 ++++---- internal/cephfs/mounter/kernel.go | 2 +- internal/cephfs/mounter/kernel_test.go | 6 +++--- internal/cephfs/nodeserver.go | 10 ++++------ internal/cephfs/store/backingsnapshot.go | 3 +-- internal/cephfs/store/volumeoptions.go | 10 +++++----- internal/cephfs/validator.go | 10 +++++----- 11 files changed, 40 insertions(+), 45 deletions(-) diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go index 28e3837e2..7673c2785 100644 --- a/internal/cephfs/controllerserver.go +++ b/internal/cephfs/controllerserver.go @@ -183,13 +183,13 @@ func (cs *ControllerServer) checkContentSource( req *csi.CreateVolumeRequest, cr *util.Credentials, ) (*store.VolumeOptions, *store.VolumeIdentifier, *store.SnapshotIdentifier, error) { - if req.VolumeContentSource == nil { + if req.GetVolumeContentSource() == nil { return nil, nil, nil, nil } - volumeSource := req.VolumeContentSource - switch volumeSource.Type.(type) { + volumeSource := req.GetVolumeContentSource() + switch volumeSource.GetType().(type) { case *csi.VolumeContentSource_Snapshot: - snapshotID := req.VolumeContentSource.GetSnapshot().GetSnapshotId() + snapshotID := req.GetVolumeContentSource().GetSnapshot().GetSnapshotId() volOpt, _, sid, err := store.NewSnapshotOptionsFromID(ctx, snapshotID, cr, req.GetSecrets(), cs.ClusterName, cs.SetMetadata) if err != nil { @@ -203,9 +203,9 @@ func (cs *ControllerServer) checkContentSource( return volOpt, nil, sid, nil case *csi.VolumeContentSource_Volume: // Find the volume using the provided VolumeID - volID := req.VolumeContentSource.GetVolume().GetVolumeId() + volID := req.GetVolumeContentSource().GetVolume().GetVolumeId() parentVol, pvID, err := store.NewVolumeOptionsFromVolID(ctx, - volID, nil, req.Secrets, cs.ClusterName, cs.SetMetadata) + volID, nil, req.GetSecrets(), cs.ClusterName, cs.SetMetadata) if err != nil { if !errors.Is(err, cerrors.ErrVolumeNotFound) { return nil, nil, nil, status.Error(codes.NotFound, err.Error()) @@ -342,7 +342,7 @@ func (cs *ControllerServer) CreateVolume( // As we are trying to create RWX volume from backing snapshot, we need to // retrieve the snapshot details from the backing snapshot and create a // subvolume clone from the snapshot. - if parentVol != nil && parentVol.BackingSnapshot && !store.IsVolumeCreateRO(req.VolumeCapabilities) { + if parentVol != nil && parentVol.BackingSnapshot && !store.IsVolumeCreateRO(req.GetVolumeCapabilities()) { // unset pvID as we dont have real subvolume for the parent volumeID as its a backing snapshot pvID = nil parentVol, _, sID, err = store.NewSnapshotOptionsFromID(ctx, parentVol.BackingSnapshotID, cr, @@ -674,7 +674,7 @@ func (cs *ControllerServer) ValidateVolumeCapabilities( req *csi.ValidateVolumeCapabilitiesRequest, ) (*csi.ValidateVolumeCapabilitiesResponse, error) { // Cephfs doesn't support Block volume - for _, capability := range req.VolumeCapabilities { + for _, capability := range req.GetVolumeCapabilities() { if capability.GetBlock() != nil { return &csi.ValidateVolumeCapabilitiesResponse{Message: ""}, nil } @@ -682,7 +682,7 @@ func (cs *ControllerServer) ValidateVolumeCapabilities( return &csi.ValidateVolumeCapabilitiesResponse{ Confirmed: &csi.ValidateVolumeCapabilitiesResponse_Confirmed{ - VolumeCapabilities: req.VolumeCapabilities, + VolumeCapabilities: req.GetVolumeCapabilities(), }, }, nil } @@ -970,10 +970,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.NotFound, "snapshot Name cannot be empty") } - if req.SourceVolumeId == "" { + if req.GetSourceVolumeId() == "" { return status.Error(codes.NotFound, "source Volume ID cannot be empty") } diff --git a/internal/cephfs/core/clone_test.go b/internal/cephfs/core/clone_test.go index 4a74a4c3b..1cf9cd6c2 100644 --- a/internal/cephfs/core/clone_test.go +++ b/internal/cephfs/core/clone_test.go @@ -17,13 +17,12 @@ limitations under the License. package core import ( - "errors" "testing" cerrors "github.com/ceph/ceph-csi/internal/cephfs/errors" fsa "github.com/ceph/go-ceph/cephfs/admin" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCloneStateToError(t *testing.T) { @@ -36,6 +35,6 @@ func TestCloneStateToError(t *testing.T) { errorState[cephFSCloneState{fsa.CloneFailed, "", ""}] = cerrors.ErrCloneFailed for state, err := range errorState { - assert.True(t, errors.Is(state.ToError(), err)) + require.ErrorIs(t, state.ToError(), err) } } diff --git a/internal/cephfs/core/filesystem.go b/internal/cephfs/core/filesystem.go index 3e2ae4a7a..b63bdad50 100644 --- a/internal/cephfs/core/filesystem.go +++ b/internal/cephfs/core/filesystem.go @@ -29,11 +29,11 @@ import ( // that interacts with CephFS filesystem API's. type FileSystem interface { // GetFscID returns the ID of the filesystem with the given name. - GetFscID(context.Context, string) (int64, error) + GetFscID(ctx context.Context, fsName string) (int64, error) // GetMetadataPool returns the metadata pool name of the filesystem with the given name. - GetMetadataPool(context.Context, string) (string, error) + GetMetadataPool(ctx context.Context, fsName string) (string, error) // GetFsName returns the name of the filesystem with the given ID. - GetFsName(context.Context, int64) (string, error) + GetFsName(ctx context.Context, fsID int64) (string, error) } // fileSystem is the implementation of FileSystem interface. diff --git a/internal/cephfs/driver_test.go b/internal/cephfs/driver_test.go index 28dc628b6..0377cbcea 100644 --- a/internal/cephfs/driver_test.go +++ b/internal/cephfs/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/cephfs/groupcontrollerserver.go b/internal/cephfs/groupcontrollerserver.go index 2e2a5b7d6..321ec5752 100644 --- a/internal/cephfs/groupcontrollerserver.go +++ b/internal/cephfs/groupcontrollerserver.go @@ -180,7 +180,7 @@ func (cs *ControllerServer) CreateVolumeGroupSnapshot( for _, r := range *resp { r.Snapshot.GroupSnapshotId = vgs.VolumeGroupSnapshotID - response.GroupSnapshot.Snapshots = append(response.GroupSnapshot.Snapshots, r.Snapshot) + response.GroupSnapshot.Snapshots = append(response.GroupSnapshot.Snapshots, r.GetSnapshot()) } return response, nil @@ -293,7 +293,7 @@ func (cs *ControllerServer) releaseQuiesceAndGetVolumeGroupSnapshotResponse( for _, r := range snapshotResponses { r.Snapshot.GroupSnapshotId = vgs.VolumeGroupSnapshotID - response.GroupSnapshot.Snapshots = append(response.GroupSnapshot.Snapshots, r.Snapshot) + response.GroupSnapshot.Snapshots = append(response.GroupSnapshot.Snapshots, r.GetSnapshot()) } return response, nil @@ -703,7 +703,7 @@ func (cs *ControllerServer) DeleteVolumeGroupSnapshot(ctx context.Context, return nil, err } - groupSnapshotID := req.GroupSnapshotId + groupSnapshotID := req.GetGroupSnapshotId() // Existence and conflict checks if acquired := cs.VolumeGroupLocks.TryAcquire(groupSnapshotID); !acquired { log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, groupSnapshotID) @@ -718,7 +718,7 @@ func (cs *ControllerServer) DeleteVolumeGroupSnapshot(ctx context.Context, } defer cr.DeleteCredentials() - vgo, vgsi, err := store.NewVolumeGroupOptionsFromID(ctx, req.GroupSnapshotId, cr) + vgo, vgsi, err := store.NewVolumeGroupOptionsFromID(ctx, req.GetGroupSnapshotId(), cr) if err != nil { log.ErrorLog(ctx, "failed to get volume group options: %v", err) err = extractDeleteVolumeGroupError(err) diff --git a/internal/cephfs/mounter/kernel.go b/internal/cephfs/mounter/kernel.go index 924b9565a..09bc707f5 100644 --- a/internal/cephfs/mounter/kernel.go +++ b/internal/cephfs/mounter/kernel.go @@ -81,7 +81,7 @@ func (m *kernelMounter) mountKernel( optionsStr := fmt.Sprintf("name=%s,secretfile=%s", cr.ID, cr.KeyFile) mdsNamespace := "" if volOptions.FsName != "" { - mdsNamespace = fmt.Sprintf("mds_namespace=%s", volOptions.FsName) + mdsNamespace = "mds_namespace=" + volOptions.FsName } optionsStr = util.MountOptionsAdd(optionsStr, mdsNamespace, volOptions.KernelMountOptions, netDev) diff --git a/internal/cephfs/mounter/kernel_test.go b/internal/cephfs/mounter/kernel_test.go index f370d4768..95983b8a5 100644 --- a/internal/cephfs/mounter/kernel_test.go +++ b/internal/cephfs/mounter/kernel_test.go @@ -19,7 +19,7 @@ package mounter import ( "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFilesystemSupported(t *testing.T) { @@ -31,8 +31,8 @@ func TestFilesystemSupported(t *testing.T) { // "proc" is always a supported filesystem, we detect supported // filesystems by reading from it - assert.True(t, filesystemSupported("proc")) + require.True(t, filesystemSupported("proc")) // "nonefs" is a made-up name, and does not exist - assert.False(t, filesystemSupported("nonefs")) + require.False(t, filesystemSupported("nonefs")) } diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index 2af514c36..1f963d71c 100644 --- a/internal/cephfs/nodeserver.go +++ b/internal/cephfs/nodeserver.go @@ -110,8 +110,7 @@ func (ns *NodeServer) getVolumeOptions( func validateSnapshotBackedVolCapability(volCap *csi.VolumeCapability) error { // Snapshot-backed volumes may be used with read-only volume access modes only. - mode := volCap.AccessMode.Mode - + mode := volCap.GetAccessMode().GetMode() if mode != csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY && mode != csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { return status.Error(codes.InvalidArgument, @@ -352,7 +351,6 @@ func (ns *NodeServer) mount( true, []string{"bind", "_netdev"}, ) - if err != nil { log.ErrorLog(ctx, "failed to bind mount snapshot root %s: %v", absoluteSnapshotRoot, err) @@ -813,9 +811,9 @@ func (ns *NodeServer) setMountOptions( } const readOnly = "ro" - - if volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || - volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { + mode := volCap.GetAccessMode().GetMode() + if mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || + mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { switch mnt.(type) { case *mounter.FuseMounter: if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) { diff --git a/internal/cephfs/store/backingsnapshot.go b/internal/cephfs/store/backingsnapshot.go index 11d3cb1f0..4bd876dcb 100644 --- a/internal/cephfs/store/backingsnapshot.go +++ b/internal/cephfs/store/backingsnapshot.go @@ -18,7 +18,6 @@ package store import ( "context" - "fmt" fsutil "github.com/ceph/ceph-csi/internal/cephfs/util" "github.com/ceph/ceph-csi/internal/util/log" @@ -28,7 +27,7 @@ import ( ) func fmtBackingSnapshotReftrackerName(backingSnapID string) string { - return fmt.Sprintf("rt-backingsnapshot-%s", backingSnapID) + return "rt-backingsnapshot-" + backingSnapID } func AddSnapshotBackedVolumeRef( diff --git a/internal/cephfs/store/volumeoptions.go b/internal/cephfs/store/volumeoptions.go index 5ed493e86..2165854df 100644 --- a/internal/cephfs/store/volumeoptions.go +++ b/internal/cephfs/store/volumeoptions.go @@ -168,7 +168,7 @@ func extractMounter(dest *string, options map[string]string) error { func GetClusterInformation(options map[string]string) (*cephcsi.ClusterInfo, error) { clusterID, ok := options["clusterID"] if !ok { - err := fmt.Errorf("clusterID must be set") + err := errors.New("clusterID must be set") return nil, err } @@ -344,15 +344,15 @@ func NewVolumeOptions( // IsShallowVolumeSupported returns true only for ReadOnly volume requests // with datasource as snapshot. func IsShallowVolumeSupported(req *csi.CreateVolumeRequest) bool { - isRO := IsVolumeCreateRO(req.VolumeCapabilities) + isRO := IsVolumeCreateRO(req.GetVolumeCapabilities()) return isRO && (req.GetVolumeContentSource() != nil && req.GetVolumeContentSource().GetSnapshot() != nil) } func IsVolumeCreateRO(caps []*csi.VolumeCapability) bool { for _, cap := range caps { - if cap.AccessMode != nil { - switch cap.AccessMode.Mode { //nolint:exhaustive // only check what we want + if cap.GetAccessMode() != nil { + switch cap.GetAccessMode().GetMode() { //nolint:exhaustive // only check what we want case csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY: return true @@ -612,7 +612,7 @@ func NewVolumeOptionsFromMonitorList( // check if there are mon values in secret and if so override option retrieved monitors from // monitors in the secret mon, err := util.GetMonValFromSecret(secrets) - if err == nil && len(mon) > 0 { + if err == nil && mon != "" { opts.Monitors = mon } diff --git a/internal/cephfs/validator.go b/internal/cephfs/validator.go index 07e38688c..74099fc9d 100644 --- a/internal/cephfs/validator.go +++ b/internal/cephfs/validator.go @@ -54,11 +54,11 @@ func (cs *ControllerServer) validateCreateVolumeRequest(req *csi.CreateVolumeReq return err } - if req.VolumeContentSource != nil { - volumeSource := req.VolumeContentSource - switch volumeSource.Type.(type) { + if req.GetVolumeContentSource() != nil { + volumeSource := req.GetVolumeContentSource() + switch volumeSource.GetType().(type) { case *csi.VolumeContentSource_Snapshot: - snapshot := req.VolumeContentSource.GetSnapshot() + snapshot := req.GetVolumeContentSource().GetSnapshot() // CSI spec requires returning NOT_FOUND when the volumeSource is missing/incorrect. if snapshot == nil { return status.Error(codes.NotFound, "volume Snapshot cannot be empty") @@ -68,7 +68,7 @@ func (cs *ControllerServer) validateCreateVolumeRequest(req *csi.CreateVolumeReq } case *csi.VolumeContentSource_Volume: // CSI spec requires returning NOT_FOUND when the volumeSource is missing/incorrect. - vol := req.VolumeContentSource.GetVolume() + vol := req.GetVolumeContentSource().GetVolume() if vol == nil { return status.Error(codes.NotFound, "volume cannot be empty") }