Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Golint removal #274

Merged
merged 11 commits into from
Aug 20, 2024
Merged

Golint removal #274

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
11473c8
update to use goalngci-lint
aqu-dell Aug 19, 2024
98c945b
Initial removal of format string errors
aqu-dell Aug 19, 2024
5bb59e1
Updating getmessagewithrunid function
aqu-dell Aug 19, 2024
d73ac62
removing unnecessary variable settings
aqu-dell Aug 19, 2024
7a926cc
fixing run id in unit test
aqu-dell Aug 19, 2024
3044725
merge main
aqu-dell Aug 19, 2024
33833bd
removing function creation
aqu-dell Aug 19, 2024
09062ff
Removing ''
aqu-dell Aug 19, 2024
33b7d2a
fixing unit-test
aqu-dell Aug 19, 2024
9ade209
removing extra error word
aqu-dell Aug 19, 2024
32ea465
Merge branch 'main' into golint-removal
donatwork Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions check.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ VET_RETURN_CODE=$?
echo === Finished

echo === Linting...
(command -v golint >/dev/null 2>&1 \
|| go install golang.org/x/lint/golint@latest) \
&& golint --set_exit_status ./service/... ./common/...
(command -v golangci-lint >/dev/null 2>&1 \
|| curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.60.1) \
&& golangci-lint run ./service/... ./common/...
LINT_RETURN_CODE=$?
echo === Finished

Expand Down
9 changes: 7 additions & 2 deletions common/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,12 @@ func GetVolumeNameFromExportPath(exportPath string) string {
}

// GetMessageWithRunID returns message with runID information
func GetMessageWithRunID(runid string, format string, args ...interface{}) string {
func GetMessageWithRunID(runid string, format string) string {
return fmt.Sprintf(" runid=%s %s", runid, format)
}

// GetMessageWithRunIDf formats according to a format specifier and returns message with runID information
func GetMessageWithRunIDf(runid string, format string, args ...interface{}) string {
str := fmt.Sprintf(format, args...)
return fmt.Sprintf(" runid=%s %s", runid, str)
return GetMessageWithRunID(runid, str)
}
79 changes: 43 additions & 36 deletions service/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,11 @@ func (s *service) CreateVolume(
var snapshotSrcClusterName string
sourceSnapshotID, snapshotSrcClusterName, _, err = utils.ParseNormalizedSnapshotID(ctx, normalizedSnapshotID)
if err != nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "failed to parse snapshot ID '%s', error : '%v'", normalizedSnapshotID, err))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "failed to parse snapshot ID '%s', error : '%v'", normalizedSnapshotID, err))
}

if snapshotSrcClusterName != "" && snapshotSrcClusterName != clusterName {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "source snapshot's cluster name '%s' and new volume's cluster name '%s' doesn't match", snapshotSrcClusterName, clusterName))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "source snapshot's cluster name '%s' and new volume's cluster name '%s' doesn't match", snapshotSrcClusterName, clusterName))
}

log.Infof("Creating volume from snapshot ID: '%s'", sourceSnapshotID)
Expand All @@ -387,17 +387,17 @@ func (s *service) CreateVolume(

vcs := req.GetVolumeCapabilities()
if len(vcs) == 0 {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "volume capabilty is required"))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "volume capabilty is required"))
}

for _, vc := range vcs {
if vc == nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "volume capabilty is required"))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "volume capabilty is required"))
}

am := vc.GetAccessMode()
if am == nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "access mode is required"))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "access mode is required"))
}

if am.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY {
Expand Down Expand Up @@ -541,7 +541,7 @@ func (s *service) CreateVolume(
var errMsg string
if err == nil {
if foundVol {
return nil, status.Error(codes.Internal, utils.GetMessageWithRunID(runID, "the export may not be ready yet and the path is %s", path))
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "the export may not be ready yet and the path is '%s'", path))
}
} else {
// internal error
Expand Down Expand Up @@ -678,7 +678,7 @@ func (s *service) CreateVolume(
return nil, err
}
}
return nil, status.Error(codes.Internal, utils.GetMessageWithRunID(runID, "the export id %d and the path %s may not be ready yet after retrying", exportID, path))
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "the export id '%d' and path '%s' may not be ready yet after retrying", exportID, path))
}

func (s *service) createVolumeFromSnapshot(ctx context.Context, isiConfig *IsilonClusterConfig,
Expand Down Expand Up @@ -1100,12 +1100,12 @@ func (s *service) ControllerPublishVolume(
volID := req.GetVolumeId()
if volID == "" {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "volume ID is required"))
utils.GetMessageWithRunIDf(runID, "volume ID is required"))
}

volName, exportID, accessZone, clusterName, err := utils.ParseNormalizedVolumeID(ctx, volID)
if err != nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "failed to parse volume ID '%s', error : '%v'", volID, err))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "failed to parse volume ID '%s', error : '%v'", volID, err))
}

isiConfig, err := s.getIsilonConfig(ctx, &clusterName)
Expand Down Expand Up @@ -1134,43 +1134,45 @@ func (s *service) ControllerPublishVolume(
if isROVolumeFromSnapshot {
log.Info("Volume source is snapshot")
if export, err := isiConfig.isiSvc.GetExportWithPathAndZone(ctx, exportPath, accessZone); err != nil || export == nil {
return nil, status.Errorf(codes.Internal, " runid=%s error retrieving export for %s", runID, exportPath)
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "error retrieving export for '%s'", exportPath))
}
} else {
isiPath = utils.GetIsiPathFromExportPath(exportPath)
vol, err := isiConfig.isiSvc.GetVolume(ctx, isiPath, "", volName)
if err != nil || vol.Name == "" {
return nil, status.Errorf(codes.Internal, " runid=%s failure checking volume status before controller publish: %v", runID, err)
return nil, status.Error(codes.Internal,
utils.GetMessageWithRunIDf(runID, "failure checking volume status before controller publish: '%s'",
err.Error()))
}
}

nodeID := req.GetNodeId()
if nodeID == "" {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "node ID is required"))
utils.GetMessageWithRunIDf(runID, "node ID is required"))
}

vc := req.GetVolumeCapability()
if vc == nil {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "volume capability is required"))
utils.GetMessageWithRunIDf(runID, "volume capability is required"))
}

am := vc.GetAccessMode()
if am == nil {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "access mode is required"))
utils.GetMessageWithRunIDf(runID, "access mode is required"))
}

if am.Mode == csi.VolumeCapability_AccessMode_UNKNOWN {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, errUnknownAccessMode))
utils.GetMessageWithRunIDf(runID, errUnknownAccessMode))
}

vcs := []*csi.VolumeCapability{req.GetVolumeCapability()}
if !checkValidAccessTypes(vcs) {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, errUnknownAccessType))
utils.GetMessageWithRunIDf(runID, errUnknownAccessType))
}

rootClientEnabled := false
Expand Down Expand Up @@ -1213,7 +1215,9 @@ func (s *service) ControllerPublishVolume(
break
}
if isiConfig.isiSvc.OtherClientsAlreadyAdded(ctx, exportID, accessZone, nodeID) {
return nil, status.Errorf(codes.FailedPrecondition, " runid=%s export %d in access zone %s already has other clients added to it, and the access mode is %s thus the request fails", runID, exportID, accessZone, am.Mode)
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunIDf(runID,
"export '%d' in access zone '%s' already has other clients added to it, and the access mode is "+
"%s, thus the request fails", exportID, accessZone, am.Mode))
}

if !isiConfig.isiSvc.IsHostAlreadyAdded(ctx, exportID, accessZone, utils.DummyHostNodeID) {
Expand All @@ -1224,11 +1228,12 @@ func (s *service) ControllerPublishVolume(
err = isiConfig.isiSvc.AddExportClientNetworkIdentifierByIDWithZone(ctx, clusterName, exportID, accessZone, nodeID, *isiConfig.IgnoreUnresolvableHosts, isiConfig.isiSvc.AddExportClientByIDWithZone)
}
default:
return nil, status.Errorf(codes.InvalidArgument, " runid=%s unsupported access mode: %s", runID, am.String())
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "unsupported access mode: '%s'", am.String()))
}

if err != nil {
return nil, status.Errorf(codes.Internal, " runid=%s internal error occurred when attempting to add client ip %s to export %d, error : %v", runID, nodeID, exportID, err)
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID,
"internal error occurred when attempting to add client ip '%s' to export '%d', error : '%v'", nodeID, exportID, err))
}
return &csi.ControllerPublishVolumeResponse{}, nil
}
Expand Down Expand Up @@ -1275,7 +1280,9 @@ func (s *service) ValidateVolumeCapabilities(

vol, err := s.getVolByName(ctx, isiPath, volName, isiConfig)
if err != nil {
return nil, status.Errorf(codes.Internal, " runid=%s failure checking volume status for capabilities: %s", runID, err.Error())
return nil, status.Error(codes.Internal,
utils.GetMessageWithRunIDf(runID, "failure checking volume status for capabilities: '%s'",
err.Error()))
}

vcs := req.GetVolumeCapabilities()
Expand Down Expand Up @@ -1383,12 +1390,12 @@ func (s *service) ControllerUnpublishVolume(
noProbeOnStart = false

if req.VolumeId == "" {
return nil, status.Errorf(codes.InvalidArgument, " runid=%s ControllerUnpublishVolumeRequest.VolumeId is empty", runID)
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "ControllerUnpublishVolumeRequest.VolumeId is empty"))
}

_, exportID, accessZone, clusterName, err := utils.ParseNormalizedVolumeID(ctx, req.VolumeId)
if err != nil {
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunID(runID, "failed to parse volume ID %s error : %s", req.VolumeId, err.Error()))
return nil, status.Error(codes.InvalidArgument, utils.GetMessageWithRunIDf(runID, "failed to parse volume ID '%s', error : '%s'", req.VolumeId, err.Error()))
}

isiConfig, err := s.getIsilonConfig(ctx, &clusterName)
Expand All @@ -1402,13 +1409,13 @@ func (s *service) ControllerUnpublishVolume(

// auto probe
if err := s.autoProbe(ctx, isiConfig); err != nil {
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunID(runID, "error %s", err.Error()))
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunIDf(runID, "error %s", err.Error()))
}

nodeID := req.GetNodeId()
if nodeID == "" {
return nil, status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "node ID is required"))
utils.GetMessageWithRunIDf(runID, "node ID is required"))
}

log.Debugf("ignoreUnresolvableHosts value is '%t', for clusterName '%s'", *isiConfig.IgnoreUnresolvableHosts, clusterName)
Expand All @@ -1420,7 +1427,7 @@ func (s *service) ControllerUnpublishVolume(
return nil, delErr
}
} else {
return nil, status.Errorf(codes.Internal, " runid=%s error encountered when trying to remove client '%s' from export '%d' with access zone '%s' on cluster '%s', error %s", runID, nodeID, exportID, accessZone, clusterName, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "error encountered when trying to remove client '%s' from export '%d' with access zone '%s' on cluster '%s', error %s", nodeID, exportID, accessZone, clusterName, err.Error()))
}
}

Expand Down Expand Up @@ -1465,10 +1472,10 @@ func (s *service) GetCapacity(

stat, err := isiConfig.isiSvc.GetStatistics(ctx, keyArray)
if err != nil || len(stat.StatsList) < 1 {
return nil, status.Errorf(codes.Internal, " runid=%s Could not retrieve capacity. Error '%s'", runID, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "Could not retrieve capacity. Error '%s'", err.Error()))
}
if stat.StatsList[0].Error != "" {
return nil, status.Errorf(codes.Internal, " runid=%s Could not retrieve capacity. Data returned error '%s'", runID, stat.StatsList[0].Error)
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "Could not retrieve capacity. Data returned error '%s'", stat.StatsList[0].Error))
}
remainingCapInBytes := stat.StatsList[0].Value

Expand Down Expand Up @@ -1671,7 +1678,7 @@ func (s *service) CreateSnapshot(
}
// return already exists error
return nil, status.Error(codes.AlreadyExists,
utils.GetMessageWithRunID(runID, "a snapshot with name '%s' already exists but is "+
utils.GetMessageWithRunIDf(runID, "a snapshot with name '%s' already exists but is "+
"incompatible with the specified source volume id '%s'", snapshotName, req.GetSourceVolumeId()))
}

Expand Down Expand Up @@ -1705,13 +1712,13 @@ func (s *service) validateCreateSnapshotRequest(

if !isiConfig.isiSvc.IsVolumeExistent(ctx, isiPath, "", srcVolumeID) {
return "", "", status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "source volume id is invalid"))
utils.GetMessageWithRunIDf(runID, "source volume id is invalid"))
}

snapshotName := req.GetName()
if snapshotName == "" {
return "", "", status.Error(codes.InvalidArgument,
utils.GetMessageWithRunID(runID, "name cannot be empty"))
utils.GetMessageWithRunIDf(runID, "name cannot be empty"))
}

return srcVolumeID, snapshotName, nil
Expand Down Expand Up @@ -1749,7 +1756,7 @@ func (s *service) DeleteSnapshot(
ctx, log, runID := GetRunIDLog(ctx)
log.Infof("DeleteSnapshot started")
if req.GetSnapshotId() == "" {
return nil, status.Errorf(codes.FailedPrecondition, " runid=%s snapshot id to be deleted is required", runID)
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunIDf(runID, "snapshot id to be deleted is required"))
}
// parse the input snapshot id and fetch it's components
snapshotID, clusterName, accessZone, err := utils.ParseNormalizedSnapshotID(ctx, req.GetSnapshotId())
Expand All @@ -1772,7 +1779,7 @@ func (s *service) DeleteSnapshot(

id, err := strconv.ParseInt(snapshotID, 10, 64)
if err != nil {
return nil, status.Errorf(codes.Internal, " runid=%s cannot convert snapshot to integer: '%s'", runID, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "cannot convert snapshot to integer: '%s'", err.Error()))
}
snapshot, err := isiConfig.isiSvc.GetSnapshot(ctx, snapshotID)
// Idempotency check
Expand All @@ -1785,13 +1792,13 @@ func (s *service) DeleteSnapshot(
return &csi.DeleteSnapshotResponse{}, nil
}
// Internal server error if the error is not about "not found"
return nil, status.Errorf(codes.Internal, " runid=%s cannot check the existence of the snapshot: '%s'", runID, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "cannot check the existence of the snapshot: '%s'", err.Error()))
}

if jsonError.StatusCode == 404 {
return &csi.DeleteSnapshotResponse{}, nil
}
return nil, status.Errorf(codes.Internal, " runid=%s cannot check the existence of the snapshot: '%s'", runID, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "cannot check the existence of the snapshot: '%s'", err.Error()))
}

// Get snapshot path
Expand Down Expand Up @@ -1822,7 +1829,7 @@ func (s *service) DeleteSnapshot(
if deleteSnapshot {
err = isiConfig.isiSvc.DeleteSnapshot(ctx, id, "")
if err != nil {
return nil, status.Errorf(codes.Internal, " runid=%s error deleting snapshot: '%s'", runID, err.Error())
return nil, status.Error(codes.Internal, utils.GetMessageWithRunIDf(runID, "error deleting snapshot: '%s'", err.Error()))
}
}
log.Infof("Snapshot with id '%s' deleted", snapshotID)
Expand Down Expand Up @@ -1974,7 +1981,7 @@ func (s *service) ControllerGetVolume(ctx context.Context,

volID := req.GetVolumeId()
if volID == "" {
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunID(runID, "no VolumeID found in request"))
return nil, status.Error(codes.FailedPrecondition, utils.GetMessageWithRunIDf(runID, "no VolumeID found in request"))
}

volName, exportID, accessZone, clusterName, err := utils.ParseNormalizedVolumeID(ctx, volID)
Expand Down
2 changes: 1 addition & 1 deletion service/features/service.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Feature: Isilon CSI interface

Examples:
| induced | errormsg |
| "StatsError" | "Could not retrieve capacity. Data returned error" |
| "StatsError" | "runid=1 Could not retrieve capacity. Data returned error" |
| "InstancesError" | "runid=1 Could not retrieve capacity. Error 'Error retrieving Statistics'" |
| "none" | "none" |

Expand Down
Loading
Loading