diff --git a/.github/workflows/pr-check.yaml b/.github/workflows/pr-check.yaml index fd3ac8b..2e73cf6 100644 --- a/.github/workflows/pr-check.yaml +++ b/.github/workflows/pr-check.yaml @@ -12,7 +12,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.56.2 + version: v1.58.2 args: --timeout=5m build: diff --git a/.golangci.yml b/.golangci.yml index b1b5248..b815473 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,31 +4,34 @@ issues: max-same-issues: 0 # disable linters-settings: - - staticcheck: - go: "1.15" - checks: [ "all" ] - - stylecheck: - go: "1.15" - checks: [ "all" ] - + gci: + sections: + - standard + - default + - prefix(github.com/leaseweb/cloudstack-csi-driver) goimports: - local-prefixes: github.com/apalia/cloudstack-csi-driver + local-prefixes: github.com/leaseweb/cloudstack-csi-driver misspell: locale: US linters: - disable-all: true - enable: - - errcheck - - gosimple - - govet - - ineffassign - - staticcheck - - stylecheck - - goimports - - typecheck - - unused - - misspell + enable-all: true + disable: + - cyclop + - depguard + - err113 + - exhaustruct + - funlen + - gochecknoglobals + - gomnd + - inamedparam + - ireturn + - lll + - mnd + - paralleltest + - tagliatelle + - testpackage + - varnamelen + - wrapcheck + - wsl diff --git a/cmd/cloudstack-csi-driver/main.go b/cmd/cloudstack-csi-driver/main.go index 61fdf1c..c74e5fb 100644 --- a/cmd/cloudstack-csi-driver/main.go +++ b/cmd/cloudstack-csi-driver/main.go @@ -25,7 +25,7 @@ var ( debug = flag.Bool("debug", false, "Enable debug logging") showVersion = flag.Bool("version", false, "Show version") - // Version is set by the build process + // Version is set by the build process. version = "" isDevEnv = false ) @@ -35,7 +35,8 @@ func main() { if *showVersion { baseName := path.Base(os.Args[0]) - fmt.Println(baseName, version) + fmt.Println(baseName, version) //nolint:forbidigo + return } @@ -48,7 +49,7 @@ func main() { } func run() { - // Setup logging + // Setup logging. var logConfig zap.Config if isDevEnv { logConfig = zap.NewDevelopmentConfig() @@ -63,11 +64,11 @@ func run() { undo := zap.ReplaceGlobals(logger) defer undo() - // Setup cloud connector + // Setup cloud connector. config, err := cloud.ReadConfig(*cloudstackconfig) if err != nil { logger.Sugar().Errorw("Cannot read CloudStack configuration", "error", err) - os.Exit(1) + os.Exit(1) //nolint:gocritic } logger.Sugar().Debugf("Successfully read CloudStack configuration %v", *cloudstackconfig) csConnector := cloud.New(config) diff --git a/cmd/cloudstack-csi-sc-syncer/main.go b/cmd/cloudstack-csi-sc-syncer/main.go index bfd85ca..4cf3884 100644 --- a/cmd/cloudstack-csi-sc-syncer/main.go +++ b/cmd/cloudstack-csi-sc-syncer/main.go @@ -20,11 +20,11 @@ var ( kubeconfig = flag.String("kubeconfig", path.Join(os.Getenv("HOME"), ".kube/config"), "Kubernetes configuration file. Use \"-\" to use in-cluster configuration.") label = flag.String("label", "app.kubernetes.io/managed-by="+agent, "") namePrefix = flag.String("namePrefix", "cloudstack-", "") - delete = flag.Bool("delete", false, "Delete") + deleteUnused = flag.Bool("delete", false, "Delete") volumeExpansion = flag.Bool("volumeExpansion", false, "VolumeExpansion") showVersion = flag.Bool("version", false, "Show version") - // Version is set by the build process + // Version is set by the build process. version = "" ) @@ -33,7 +33,8 @@ func main() { if *showVersion { baseName := path.Base(os.Args[0]) - fmt.Println(baseName, version) + fmt.Println(baseName, version) //nolint:forbidigo + return } @@ -43,7 +44,7 @@ func main() { KubeConfig: *kubeconfig, Label: *label, NamePrefix: *namePrefix, - Delete: *delete, + Delete: *deleteUnused, VolumeExpansion: *volumeExpansion, }) if err != nil { diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index df51335..a8b5417 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -46,7 +46,7 @@ type VM struct { ZoneID string } -// Specific errors +// Specific errors. var ( ErrNotFound = errors.New("not found") ErrTooManyResults = errors.New("too many results") @@ -60,5 +60,6 @@ type client struct { // New creates a new cloud connector, given its configuration. func New(config *Config) Interface { csClient := cloudstack.NewAsyncClient(config.APIURL, config.APIKey, config.SecretKey, config.VerifySSL) + return &client{csClient} } diff --git a/pkg/cloud/config.go b/pkg/cloud/config.go index 19b753a..86e8c57 100644 --- a/pkg/cloud/config.go +++ b/pkg/cloud/config.go @@ -17,7 +17,7 @@ type Config struct { // csConfig wraps the config for the CloudStack cloud provider. // It is taken from https://github.com/apache/cloudstack-kubernetes-provider // in order to have the same config in cloudstack-kubernetes-provider -// and in this cloudstack-csi-driver +// and in this cloudstack-csi-driver. type csConfig struct { Global struct { APIURL string `gcfg:"api-url"` diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index dc6cc51..a9db626 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -35,6 +35,7 @@ func New() cloud.Interface { ID: "0d7107a3-94d2-44e7-89b8-8930881309a5", ZoneID: zoneID, } + return &fakeConnector{ node: node, volumesByID: map[string]cloud.Volume{volume.ID: volume}, @@ -42,38 +43,41 @@ func New() cloud.Interface { } } -func (f *fakeConnector) GetVMByID(ctx context.Context, vmID string) (*cloud.VM, error) { +func (f *fakeConnector) GetVMByID(_ context.Context, vmID string) (*cloud.VM, error) { if vmID == f.node.ID { return f.node, nil } + return nil, cloud.ErrNotFound } -func (f *fakeConnector) GetNodeInfo(ctx context.Context, vmName string) (*cloud.VM, error) { +func (f *fakeConnector) GetNodeInfo(_ context.Context, _ string) (*cloud.VM, error) { return f.node, nil } -func (f *fakeConnector) ListZonesID(ctx context.Context) ([]string, error) { +func (f *fakeConnector) ListZonesID(_ context.Context) ([]string, error) { return []string{zoneID}, nil } -func (f *fakeConnector) GetVolumeByID(ctx context.Context, volumeID string) (*cloud.Volume, error) { +func (f *fakeConnector) GetVolumeByID(_ context.Context, volumeID string) (*cloud.Volume, error) { vol, ok := f.volumesByID[volumeID] if ok { return &vol, nil } + return nil, cloud.ErrNotFound } -func (f *fakeConnector) GetVolumeByName(ctx context.Context, name string) (*cloud.Volume, error) { +func (f *fakeConnector) GetVolumeByName(_ context.Context, name string) (*cloud.Volume, error) { vol, ok := f.volumesByName[name] if ok { return &vol, nil } + return nil, cloud.ErrNotFound } -func (f *fakeConnector) CreateVolume(ctx context.Context, diskOfferingID, zoneID, name string, sizeInGB int64) (string, error) { +func (f *fakeConnector) CreateVolume(_ context.Context, diskOfferingID, zoneID, name string, sizeInGB int64) (string, error) { id, _ := uuid.GenerateUUID() vol := cloud.Volume{ ID: id, @@ -84,37 +88,39 @@ func (f *fakeConnector) CreateVolume(ctx context.Context, diskOfferingID, zoneID } f.volumesByID[vol.ID] = vol f.volumesByName[vol.Name] = vol + return vol.ID, nil } -func (f *fakeConnector) DeleteVolume(ctx context.Context, id string) error { +func (f *fakeConnector) DeleteVolume(_ context.Context, id string) error { if vol, ok := f.volumesByID[id]; ok { name := vol.Name delete(f.volumesByName, name) } delete(f.volumesByID, id) + return nil } -func (f *fakeConnector) AttachVolume(ctx context.Context, volumeID, vmID string) (string, error) { +func (f *fakeConnector) AttachVolume(_ context.Context, _, _ string) (string, error) { return "1", nil } -func (f *fakeConnector) DetachVolume(ctx context.Context, volumeID string) error { +func (f *fakeConnector) DetachVolume(_ context.Context, _ string) error { return nil } -func (f *fakeConnector) ExpandVolume(ctx context.Context, volumeID string, newSizeInGB int64) error { + +func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSizeInGB int64) error { if vol, ok := f.volumesByID[volumeID]; ok { newSizeInBytes := newSizeInGB * 1024 * 1024 * 1024 if newSizeInBytes > vol.Size { vol.Size = newSizeInBytes f.volumesByID[volumeID] = vol f.volumesByName[vol.Name] = vol - return nil - } else { - return nil } - } else { + return nil } + + return cloud.ErrNotFound } diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata.go index b2b5638..a4fe792 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" "os" "strings" @@ -22,6 +21,7 @@ func (c *client) metadataInstanceID(ctx context.Context) string { // Try a NODE_ID environment variable if envNodeID := os.Getenv("NODE_ID"); envNodeID != "" { slog.Debugf("Found CloudStack VM ID from environment variable NODE_ID: %s", envNodeID) + return envNodeID } @@ -32,11 +32,10 @@ func (c *client) metadataInstanceID(ctx context.Context) string { ciData, err := c.readCloudInit(ctx, cloudInitInstanceFilePath) if err != nil { slog.Errorf("Cannot read cloud-init instance data: %v", err) - } else { - if ciData.V1.InstanceID != "" { - slog.Debugf("Found CloudStack VM ID from cloud-init: %s", ciData.V1.InstanceID) - return ciData.V1.InstanceID - } + } else if ciData.V1.InstanceID != "" { + slog.Debugf("Found CloudStack VM ID from cloud-init: %s", ciData.V1.InstanceID) + + return ciData.V1.InstanceID } slog.Error("cloud-init instance ID is not provided") } else if os.IsNotExist(err) { @@ -46,6 +45,7 @@ func (c *client) metadataInstanceID(ctx context.Context) string { } slog.Debug("CloudStack VM ID not found in meta-data.") + return "" } @@ -62,20 +62,23 @@ type cloudInitV1 struct { func (c *client) readCloudInit(ctx context.Context, instanceFilePath string) (*cloudInitInstanceData, error) { slog := ctxzap.Extract(ctx).Sugar() - b, err := ioutil.ReadFile(instanceFilePath) + b, err := os.ReadFile(instanceFilePath) if err != nil { slog.Errorf("Cannot read %s", instanceFilePath) + return nil, err } var data cloudInitInstanceData if err := json.Unmarshal(b, &data); err != nil { slog.Errorf("Cannot parse JSON file %s", instanceFilePath) + return nil, err } if strings.ToLower(data.V1.CloudName) != cloudStackCloudName { slog.Errorf("Cloud-Init cloud name is %s, only %s is supported", data.V1.CloudName, cloudStackCloudName) + return nil, fmt.Errorf("Cloud-Init cloud name is %s, only %s is supported", data.V1.CloudName, cloudStackCloudName) } diff --git a/pkg/cloud/vms.go b/pkg/cloud/vms.go index 29124c3..5b41957 100644 --- a/pkg/cloud/vms.go +++ b/pkg/cloud/vms.go @@ -23,6 +23,7 @@ func (c *client) GetVMByID(ctx context.Context, vmID string) (*VM, error) { return nil, ErrTooManyResults } vm := l.VirtualMachines[0] + return &VM{ ID: vm.Id, ZoneID: vm.Zoneid, @@ -46,6 +47,7 @@ func (c *client) getVMByName(ctx context.Context, name string) (*VM, error) { return nil, ErrTooManyResults } vm := l.VirtualMachines[0] + return &VM{ ID: vm.Id, ZoneID: vm.Zoneid, diff --git a/pkg/cloud/volumes.go b/pkg/cloud/volumes.go index 4dd3766..e4c4d72 100644 --- a/pkg/cloud/volumes.go +++ b/pkg/cloud/volumes.go @@ -6,16 +6,13 @@ import ( "strconv" "strings" + "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "github.com/leaseweb/cloudstack-csi-driver/pkg/util" ) -func (c *client) GetVolumeByID(ctx context.Context, volumeID string) (*Volume, error) { - p := c.Volume.NewListVolumesParams() - p.SetId(volumeID) - ctxzap.Extract(ctx).Sugar().Infow("CloudStack API call", "command", "ListVolumes", "params", map[string]string{ - "id": volumeID, - }) +func (c *client) listVolumes(p *cloudstack.ListVolumesParams) (*Volume, error) { l, err := c.Volume.ListVolumes(p) if err != nil { return nil, err @@ -36,36 +33,28 @@ func (c *client) GetVolumeByID(ctx context.Context, volumeID string) (*Volume, e VirtualMachineID: vol.Virtualmachineid, DeviceID: strconv.FormatInt(vol.Deviceid, 10), } + return &v, nil } +func (c *client) GetVolumeByID(ctx context.Context, volumeID string) (*Volume, error) { + p := c.Volume.NewListVolumesParams() + p.SetId(volumeID) + ctxzap.Extract(ctx).Sugar().Infow("CloudStack API call", "command", "ListVolumes", "params", map[string]string{ + "id": volumeID, + }) + + return c.listVolumes(p) +} + func (c *client) GetVolumeByName(ctx context.Context, name string) (*Volume, error) { p := c.Volume.NewListVolumesParams() p.SetName(name) ctxzap.Extract(ctx).Sugar().Infow("CloudStack API call", "command", "ListVolumes", "params", map[string]string{ "name": name, }) - l, err := c.Volume.ListVolumes(p) - if err != nil { - return nil, err - } - if l.Count == 0 { - return nil, ErrNotFound - } - if l.Count > 1 { - return nil, ErrTooManyResults - } - vol := l.Volumes[0] - v := Volume{ - ID: vol.Id, - Name: vol.Name, - Size: vol.Size, - DiskOfferingID: vol.Diskofferingid, - ZoneID: vol.Zoneid, - VirtualMachineID: vol.Virtualmachineid, - DeviceID: strconv.FormatInt(vol.Deviceid, 10), - } - return &v, nil + + return c.listVolumes(p) } func (c *client) CreateVolume(ctx context.Context, diskOfferingID, zoneID, name string, sizeInGB int64) (string, error) { @@ -84,6 +73,7 @@ func (c *client) CreateVolume(ctx context.Context, diskOfferingID, zoneID, name if err != nil { return "", err } + return vol.Id, nil } @@ -97,6 +87,7 @@ func (c *client) DeleteVolume(ctx context.Context, id string) error { // CloudStack error InvalidParameterValueException return ErrNotFound } + return err } @@ -110,6 +101,7 @@ func (c *client) AttachVolume(ctx context.Context, volumeID, vmID string) (strin if err != nil { return "", err } + return strconv.FormatInt(r.Deviceid, 10), nil } @@ -120,14 +112,15 @@ func (c *client) DetachVolume(ctx context.Context, volumeID string) error { "id": volumeID, }) _, err := c.Volume.DetachVolume(p) + return err } -// ExpandVolume expands the volume to new size +// ExpandVolume expands the volume to new size. func (c *client) ExpandVolume(ctx context.Context, volumeID string, newSizeInGB int64) error { volume, _, err := c.Volume.GetVolumeByID(volumeID) if err != nil { - return fmt.Errorf("failed to retrieve volume '%s': %v", volumeID, err) + return fmt.Errorf("failed to retrieve volume '%s': %w", volumeID, err) } if volume.State != "Allocated" && volume.State != "Ready" { return fmt.Errorf("volume '%s' is not in 'Allocated' or 'Ready' state to get resized", volumeID) @@ -144,12 +137,12 @@ func (c *client) ExpandVolume(ctx context.Context, volumeID string, newSizeInGB "current_size": strconv.FormatInt(currentSizeInGB, 10), "requested_size": strconv.FormatInt(newSizeInGB, 10), }) - // Execute the API call to resize the volume - expandedVol, err := c.Volume.ResizeVolume(p) + // Execute the API call to resize the volume. + _, err = c.Volume.ResizeVolume(p) if err != nil { // Handle the error accordingly - return fmt.Errorf("failed to expand volume '%s': %v", volumeID, err) + return fmt.Errorf("failed to expand volume '%s': %w", volumeID, err) } - fmt.Printf("Volume %s resied to %d successfully", expandedVol.Id, expandedVol.Size) + return nil } diff --git a/pkg/cloud/zones.go b/pkg/cloud/zones.go index 7eee620..edb5f49 100644 --- a/pkg/cloud/zones.go +++ b/pkg/cloud/zones.go @@ -7,7 +7,7 @@ import ( ) func (c *client) ListZonesID(ctx context.Context) ([]string, error) { - result := []string{} + result := make([]string, 0) p := c.Zone.NewListZonesParams() p.SetAvailable(true) ctxzap.Extract(ctx).Sugar().Infow("CloudStack API call", "command", "ListZones", "params", map[string]string{ @@ -20,5 +20,6 @@ func (c *client) ListZonesID(ctx context.Context) ([]string, error) { for _, zone := range r.Zones { result = append(result, zone.Id) } + return result, nil } diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go index 2f548e5..f07dc5d 100644 --- a/pkg/driver/constants.go +++ b/pkg/driver/constants.go @@ -1,15 +1,15 @@ package driver -// DriverName is the name of the CSI plugin +// DriverName is the name of the CSI plugin. const DriverName = "csi.cloudstack.apache.org" -// Topology keys +// Topology keys. const ( ZoneKey = "topology." + DriverName + "/zone" HostKey = "topology." + DriverName + "/host" ) -// Volume parameters keys +// Volume parameters keys. const ( DiskOfferingKey = DriverName + "/disk-offering-id" ) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 56df0b2..a4d1412 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -2,6 +2,7 @@ package driver import ( "context" + "errors" "fmt" "math/rand" @@ -38,8 +39,7 @@ func NewControllerServer(connector cloud.Interface) csi.ControllerServer { } func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { - - // Check arguments + // Check arguments. if req.GetName() == "" { return nil, status.Error(codes.InvalidArgument, "Volume name missing in request") @@ -64,28 +64,30 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if acquired := cs.volumeLocks.TryAcquire(name); !acquired { ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, name) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, name) } defer cs.volumeLocks.Release(name) - // Check if a volume with that name already exists - if vol, err := cs.connector.GetVolumeByName(ctx, name); err == cloud.ErrNotFound { - // The volume does not exist - } else if err != nil { - // Error with CloudStack - return nil, status.Errorf(codes.Internal, "CloudStack error: %v", err) + // Check if a volume with that name already exists. + vol, err := cs.connector.GetVolumeByName(ctx, name) + if err != nil { + if !errors.Is(err, cloud.ErrNotFound) { + // Error with CloudStack + return nil, status.Errorf(codes.Internal, "CloudStack error: %v", err) + } } else { // The volume exists. Check if it suits the request. if ok, message := checkVolumeSuitable(vol, diskOfferingID, req.GetCapacityRange(), req.GetAccessibilityRequirements()); !ok { return nil, status.Errorf(codes.AlreadyExists, "Volume %v already exists but does not satisfy request: %s", name, message) } - // Existing volume is ok + // Existing volume is ok. return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ VolumeId: vol.ID, CapacityBytes: vol.Size, VolumeContext: req.GetParameters(), - // ContentSource: req.GetVolumeContentSource(), TODO: snapshot support + // ContentSource: req.GetVolumeContentSource(), TODO: snapshot support. AccessibleTopology: []*csi.Topology{ Topology{ZoneID: vol.ZoneID}.ToCSI(), }, @@ -93,19 +95,19 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, nil } - // We have to create the volume + // We have to create the volume. - // Determine volume size using requested capacity range + // Determine volume size using requested capacity range. sizeInGB, err := determineSize(req) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } - // Determine zone using topology constraints + // Determine zone using topology constraints. var zoneID string topologyRequirement := req.GetAccessibilityRequirements() - if topologyRequirement == nil || topologyRequirement.GetRequisite() == nil { - // No topology requirement. Use random zone + if topologyRequirement == nil || topologyRequirement.GetRequisite() == nil { //nolint:nestif + // No topology requirement. Use random zone. zones, err := cs.connector.ListZonesID(ctx) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) @@ -114,7 +116,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if n == 0 { return nil, status.Error(codes.Internal, "No zone available") } - zoneID = zones[rand.Intn(n)] + zoneID = zones[rand.Intn(n)] //nolint:gosec } else { reqTopology := topologyRequirement.GetRequisite() if len(reqTopology) > 1 { @@ -144,7 +146,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol VolumeId: volID, CapacityBytes: util.GigaBytesToBytes(sizeInGB), VolumeContext: req.GetParameters(), - // ContentSource: req.GetVolumeContentSource(), TODO: snapshot support + // ContentSource: req.GetVolumeContentSource(), TODO: snapshot support. AccessibleTopology: []*csi.Topology{ Topology{ZoneID: zoneID}.ToCSI(), }, @@ -153,8 +155,8 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol } func checkVolumeSuitable(vol *cloud.Volume, - diskOfferingID string, capRange *csi.CapacityRange, topologyRequirement *csi.TopologyRequirement) (bool, string) { - + diskOfferingID string, capRange *csi.CapacityRange, topologyRequirement *csi.TopologyRequirement, +) (bool, string) { if vol.DiskOfferingID != diskOfferingID { return false, fmt.Sprintf("Disk offering %s; requested disk offering %s", vol.DiskOfferingID, diskOfferingID) } @@ -207,6 +209,7 @@ func determineSize(req *csi.CreateVolumeRequest) (int64, error) { if sizeInGB == 0 { sizeInGB = 1 } + return sizeInGB, nil } @@ -219,6 +222,7 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol if acquired := cs.volumeLocks.TryAcquire(volumeID); !acquired { ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID) } defer cs.volumeLocks.Release(volumeID) @@ -228,14 +232,15 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol ) err := cs.connector.DeleteVolume(ctx, volumeID) - if err != nil && err != cloud.ErrNotFound { + if err != nil && !errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.Internal, "Cannot delete volume %s: %s", volumeID, err.Error()) } + return &csi.DeleteVolumeResponse{}, nil } func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) { - // Check arguments + // Check arguments. if req.GetVolumeId() == "" { return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") @@ -254,7 +259,7 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs if req.GetVolumeCapability() == nil { return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") } - if req.GetVolumeCapability().AccessMode.Mode != onlyVolumeCapAccessMode.GetMode() { + if req.GetVolumeCapability().GetAccessMode().GetMode() != onlyVolumeCapAccessMode.GetMode() { return nil, status.Error(codes.InvalidArgument, "Access mode not accepted") } @@ -263,9 +268,9 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs "nodeID", nodeID, ) - // Check volume + // Check volume. vol, err := cs.connector.GetVolumeByID(ctx, volumeID) - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } else if err != nil { // Error with CloudStack @@ -278,10 +283,11 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs "nodeID", nodeID, "attached nodeID", vol.VirtualMachineID, ) + return nil, status.Error(codes.AlreadyExists, "Volume already assigned to another node") } - if _, err := cs.connector.GetVMByID(ctx, nodeID); err == cloud.ErrNotFound { + if _, err := cs.connector.GetVMByID(ctx, nodeID); errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "VM %v not found", nodeID) } else if err != nil { // Error with CloudStack @@ -289,7 +295,7 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs } if vol.VirtualMachineID == nodeID { - // volume already attached + // volume already attached. ctxzap.Extract(ctx).Sugar().Infow("Volume already attached to node", "volumeID", volumeID, "nodeID", nodeID, @@ -298,6 +304,7 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs publishContext := map[string]string{ deviceIDContextKey: vol.DeviceID, } + return &csi.ControllerPublishVolumeResponse{PublishContext: publishContext}, nil } @@ -319,11 +326,12 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs publishContext := map[string]string{ deviceIDContextKey: deviceID, } + return &csi.ControllerPublishVolumeResponse{PublishContext: publishContext}, nil } func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) { - // Check arguments + // Check arguments. if req.GetVolumeId() == "" { return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") @@ -331,10 +339,10 @@ func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req * volumeID := req.GetVolumeId() nodeID := req.GetNodeId() - // Check volume - if vol, err := cs.connector.GetVolumeByID(ctx, volumeID); err == cloud.ErrNotFound { + // Check volume. + if vol, err := cs.connector.GetVolumeByID(ctx, volumeID); errors.Is(err, cloud.ErrNotFound) { // Volume does not exist in CloudStack. We can safely assume this volume is no longer attached - // The spec requires us to return OK here + // The spec requires us to return OK here. return &csi.ControllerUnpublishVolumeResponse{}, nil } else if err != nil { // Error with CloudStack @@ -344,13 +352,14 @@ func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req * return &csi.ControllerUnpublishVolumeResponse{}, nil } - // Check VM existence - if _, err := cs.connector.GetVMByID(ctx, nodeID); err == cloud.ErrNotFound { - // volumes cannot be attached to deleted VMs + // Check VM existence. + if _, err := cs.connector.GetVMByID(ctx, nodeID); errors.Is(err, cloud.ErrNotFound) { + // volumes cannot be attached to deleted VMs. ctxzap.Extract(ctx).Sugar().Warnw("VM not found, marking ControllerUnpublishVolume successful", "volumeID", volumeID, "nodeID", nodeID, ) + return &csi.ControllerUnpublishVolumeResponse{}, nil } else if err != nil { // Error with CloudStack @@ -386,7 +395,7 @@ func (cs *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req return nil, status.Error(codes.InvalidArgument, "Volume capabilities not provided") } - if _, err := cs.connector.GetVolumeByID(ctx, volumeID); err == cloud.ErrNotFound { + if _, err := cs.connector.GetVolumeByID(ctx, volumeID); errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } else if err != nil { // Error with CloudStack @@ -402,7 +411,8 @@ func (cs *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req VolumeContext: req.GetVolumeContext(), VolumeCapabilities: volCaps, Parameters: req.GetParameters(), - }}, nil + }, + }, nil } func isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) bool { @@ -411,6 +421,7 @@ func isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) bool { return false } } + return true } @@ -425,20 +436,20 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi } err := expandVolumeLock.GetExpandLock(volumeID) if err != nil { - logger.Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID) } defer expandVolumeLock.ReleaseExpandLock(volumeID) - cap := req.GetCapacityRange() - if cap == nil { + capRange := req.GetCapacityRange() + if capRange == nil { return nil, status.Error(codes.InvalidArgument, "Capacity range not provided") } - volSizeBytes := cap.GetRequiredBytes() + volSizeBytes := capRange.GetRequiredBytes() volSizeGB := util.RoundUpBytesToGB(volSizeBytes) - maxVolSize := cap.GetLimitBytes() + maxVolSize := capRange.GetLimitBytes() if maxVolSize > 0 && maxVolSize < util.GigaBytesToBytes(volSizeGB) { return nil, status.Error(codes.OutOfRange, "Volume size exceeds the limit specified") @@ -446,21 +457,21 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi volume, err := cs.connector.GetVolumeByID(ctx, volumeID) if err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } + return nil, status.Error(codes.Internal, fmt.Sprintf("GetVolume failed with error %v", err)) } if volume.Size >= util.GigaBytesToBytes(volSizeGB) { - // A volume was already resized + // A volume was already resized. logger.Infof("Volume %q has been already expanded to %d. requested %d", volumeID, volume.Size, volSizeGB) return &csi.ControllerExpandVolumeResponse{ CapacityBytes: volume.Size, NodeExpansionRequired: true, }, nil - } err = cs.connector.ExpandVolume(ctx, volumeID, volSizeGB) if err != nil { @@ -478,7 +489,7 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi }, nil } -func (cs *controllerServer) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { +func (cs *controllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { return &csi.ControllerGetCapabilitiesResponse{ Capabilities: []*csi.ControllerServiceCapability{ { diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 85b49a0..4f4945c 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -26,7 +26,7 @@ type cloudstackDriver struct { logger *zap.Logger } -// New instantiates a new CloudStack CSI driver +// New instantiates a new CloudStack CSI driver. func New(endpoint string, csConnector cloud.Interface, mounter mount.Interface, nodeName string, version string, logger *zap.Logger) (Interface, error) { return &cloudstackDriver{ endpoint: endpoint, diff --git a/pkg/driver/identity.go b/pkg/driver/identity.go index 52186f1..52c2d7f 100644 --- a/pkg/driver/identity.go +++ b/pkg/driver/identity.go @@ -20,7 +20,7 @@ func NewIdentityServer(version string) csi.IdentityServer { } } -func (ids *identityServer) GetPluginInfo(ctx context.Context, req *csi.GetPluginInfoRequest) (*csi.GetPluginInfoResponse, error) { +func (ids *identityServer) GetPluginInfo(_ context.Context, _ *csi.GetPluginInfoRequest) (*csi.GetPluginInfoResponse, error) { if ids.version == "" { return nil, status.Error(codes.Unavailable, "Driver is missing version") } @@ -31,11 +31,11 @@ func (ids *identityServer) GetPluginInfo(ctx context.Context, req *csi.GetPlugin }, nil } -func (ids *identityServer) Probe(ctx context.Context, req *csi.ProbeRequest) (*csi.ProbeResponse, error) { +func (ids *identityServer) Probe(_ context.Context, _ *csi.ProbeRequest) (*csi.ProbeResponse, error) { return &csi.ProbeResponse{}, nil } -func (ids *identityServer) GetPluginCapabilities(ctx context.Context, req *csi.GetPluginCapabilitiesRequest) (*csi.GetPluginCapabilitiesResponse, error) { +func (ids *identityServer) GetPluginCapabilities(_ context.Context, _ *csi.GetPluginCapabilitiesRequest) (*csi.GetPluginCapabilitiesResponse, error) { return &csi.GetPluginCapabilitiesResponse{ Capabilities: []*csi.PluginCapability{ { diff --git a/pkg/driver/node.go b/pkg/driver/node.go index a73ac02..2110cd3 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -2,6 +2,7 @@ package driver import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -17,7 +18,7 @@ import ( ) const ( - // default file system type to be used when it is not provided + // default file system type to be used when it is not provided. defaultFsType = "ext4" ) @@ -34,6 +35,7 @@ func NewNodeServer(connector cloud.Interface, mounter mount.Interface, nodeName if mounter == nil { mounter = mount.New() } + return &nodeServer{ connector: connector, mounter: mounter, @@ -43,7 +45,6 @@ func NewNodeServer(connector cloud.Interface, mounter mount.Interface, nodeName } func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) { - // Check parameters volumeID := req.GetVolumeId() @@ -66,13 +67,15 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if acquired := ns.volumeLocks.TryAcquire(volumeID); !acquired { ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID) } defer ns.volumeLocks.Release(volumeID) // Now, find the device path - deviceID := req.PublishContext[deviceIDContextKey] + pubCtx := req.GetPublishContext() + deviceID := pubCtx[deviceIDContextKey] devicePath, err := ns.mounter.GetDevicePath(ctx, volumeID) if err != nil { @@ -128,6 +131,7 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return nil, status.Error(codes.Internal, err.Error()) } } + return &csi.NodeStageVolumeResponse{}, nil } @@ -140,6 +144,7 @@ func hasMountOption(options []string, opt string) bool { return true } } + return false } @@ -158,16 +163,17 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag if acquired := ns.volumeLocks.TryAcquire(volumeID); !acquired { ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID) } defer ns.volumeLocks.Release(volumeID) notMnt, err := ns.mounter.IsLikelyNotMountPoint(target) - if err != nil { if os.IsNotExist(err) { return nil, status.Error(codes.NotFound, "Target path not found") } + return nil, status.Error(codes.Internal, err.Error()) } if notMnt { @@ -183,14 +189,14 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag return nil, status.Errorf(codes.Internal, "failed to unmount target %q: %v", target, err) } - ctxzap.Extract(ctx).Sugar().Infow("NodeUnstageVolume: unmount succesfull", + ctxzap.Extract(ctx).Sugar().Infow("NodeUnstageVolume: unmount successful", "target", target, ) return &csi.NodeUnstageVolumeResponse{}, nil } -func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { +func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { //nolint:gocognit // Check arguments if req.GetVolumeCapability() == nil { return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") @@ -227,7 +233,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis // Considering kubelet ensures the stage and publish operations // are serialized, we don't need any extra locking in NodePublishVolume. - if req.GetVolumeCapability().GetMount() != nil { + if req.GetVolumeCapability().GetMount() != nil { //nolint:nestif source := req.GetStagingTargetPath() notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) @@ -245,6 +251,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis "source", source, "targetPath", targetPath, ) + return &csi.NodePublishVolumeResponse{}, nil } @@ -267,7 +274,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } } - if req.GetVolumeCapability().GetBlock() != nil { + if req.GetVolumeCapability().GetBlock() != nil { //nolint:nestif volumeID := req.GetVolumeId() devicePath, err := ns.mounter.GetDevicePath(ctx, volumeID) @@ -291,6 +298,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis if removeErr := os.Remove(targetPath); removeErr != nil { return nil, status.Errorf(codes.Internal, "Could not remove mount target %q: %v", targetPath, removeErr) } + return nil, status.Errorf(codes.Internal, "Could not create file %q: %v", targetPath, err) } @@ -323,7 +331,7 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu // Considering that kubelet ensures the stage and publish operations // are serialized, we don't need any extra locking in NodeUnpublishVolume. - if _, err := ns.connector.GetVolumeByID(ctx, volumeID); err == cloud.ErrNotFound { + if _, err := ns.connector.GetVolumeByID(ctx, volumeID); errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } else if err != nil { // Error with CloudStack @@ -348,7 +356,7 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu return &csi.NodeUnpublishVolumeResponse{}, nil } -func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) { +func (ns *nodeServer) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) { if ns.nodeName == "" { return nil, status.Error(codes.Internal, "Missing node name") } @@ -365,6 +373,7 @@ func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoReque } topology := Topology{ZoneID: vm.ZoneID} + return &csi.NodeGetInfoResponse{ NodeId: vm.ID, AccessibleTopology: topology.ToCSI(), @@ -372,7 +381,6 @@ func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoReque } func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) { - volumeID := req.GetVolumeId() if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume ID not provided") @@ -389,18 +397,20 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV ) volCap := req.GetVolumeCapability() if volCap != nil { - switch volCap.GetAccessType().(type) { + switch volCap.GetAccessType().(type) { //nolint:gocritic case *csi.VolumeCapability_Block: ctxzap.Extract(ctx).Sugar().Info("filesystem expansion is skipped for block volumes") + return &csi.NodeExpandVolumeResponse{}, nil } } _, err := ns.connector.GetVolumeByID(ctx, volumeID) if err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Error(codes.NotFound, fmt.Sprintf("Volume with ID %s not found", volumeID)) } + return nil, status.Error(codes.Internal, fmt.Sprintf("NodeExpandVolume failed with error %v", err)) } @@ -422,10 +432,11 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV if _, err := r.Resize(devicePath, volumePath); err != nil { return nil, status.Errorf(codes.Internal, "Could not resize volume %q: %v", volumeID, err) } + return &csi.NodeExpandVolumeResponse{}, nil } -func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) { +func (ns *nodeServer) NodeGetCapabilities(_ context.Context, _ *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) { return &csi.NodeGetCapabilitiesResponse{ Capabilities: []*csi.NodeServiceCapability{ { diff --git a/pkg/driver/server.go b/pkg/driver/server.go index adb74cd..1082b4f 100644 --- a/pkg/driver/server.go +++ b/pkg/driver/server.go @@ -55,6 +55,7 @@ func (cs *cloudstackDriver) serve(ids csi.IdentityServer, ctrls csi.ControllerSe } cs.logger.Sugar().Infow("Listening for connections", "address", listener.Addr()) + return server.Serve(listener) } @@ -65,5 +66,6 @@ func parseEndpoint(ep string) (string, string, error) { return s[0], s[1], nil } } + return "", "", fmt.Errorf("invalid endpoint: %v", ep) } diff --git a/pkg/driver/topology.go b/pkg/driver/topology.go index 27246f6..0cedfb2 100644 --- a/pkg/driver/topology.go +++ b/pkg/driver/topology.go @@ -24,6 +24,7 @@ func NewTopology(t *csi.Topology) (Topology, error) { return Topology{}, errors.New("no zone in topology") } hostID := segments[HostKey] + return Topology{zoneID, hostID}, nil } @@ -34,6 +35,7 @@ func (t Topology) ToCSI() *csi.Topology { if t.HostID != "" { segments[ZoneKey] = t.ZoneID } + return &csi.Topology{ Segments: segments, } diff --git a/pkg/mount/fake.go b/pkg/mount/fake.go index 3ffff46..0dd9c78 100644 --- a/pkg/mount/fake.go +++ b/pkg/mount/fake.go @@ -14,7 +14,7 @@ type fakeMounter struct { utilsexec.Interface } -// NewFake creates an fake implementation of the +// NewFake creates a fake implementation of the // mount.Interface, to be used in tests. func NewFake() Interface { return &fakeMounter{ @@ -30,7 +30,7 @@ func (m *fakeMounter) CleanupMountPoint(path string, extensiveCheck bool) error return mount.CleanupMountPoint(path, m, extensiveCheck) } -func (m *fakeMounter) GetDevicePath(ctx context.Context, volumeID string) (string, error) { +func (m *fakeMounter) GetDevicePath(_ context.Context, _ string) (string, error) { return "/dev/sdb", nil } @@ -38,24 +38,25 @@ func (m *fakeMounter) GetDeviceName(mountPath string) (string, int, error) { return mount.GetDeviceNameFromMount(m, mountPath) } -func (*fakeMounter) ExistsPath(filename string) (bool, error) { +func (*fakeMounter) ExistsPath(_ string) (bool, error) { return true, nil } func (*fakeMounter) MakeDir(pathname string) error { - err := os.MkdirAll(pathname, os.FileMode(0755)) + err := os.MkdirAll(pathname, os.FileMode(0o755)) if err != nil { if !os.IsExist(err) { return err } } + return nil } -func (*fakeMounter) MakeFile(pathname string) error { +func (*fakeMounter) MakeFile(_ string) error { return nil } -func (*fakeMounter) NewResizeFs(exec utilsexec.Interface) *mount.ResizeFs { +func (*fakeMounter) NewResizeFs(_ utilsexec.Interface) *mount.ResizeFs { return mount.NewResizeFs(New()) } diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go index a8c50f6..e7efa27 100644 --- a/pkg/mount/mount.go +++ b/pkg/mount/mount.go @@ -5,7 +5,6 @@ package mount import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "strings" @@ -73,9 +72,11 @@ func (m *mounter) GetDevicePath(ctx context.Context, volumeID string) (string, e } if path != "" { devicePath = path + return true, nil } m.probeVolume(ctx) + return false, nil }) @@ -84,6 +85,7 @@ func (m *mounter) GetDevicePath(ctx context.Context, volumeID string) (string, e } else if devicePath == "" { return "", fmt.Errorf("device path was empty for volumeID: %q", volumeID) } + return devicePath, nil } @@ -100,19 +102,20 @@ func (m *mounter) getDevicePathBySerialID(volumeID string) (string, error) { return "", err } } + return "", nil } func (m *mounter) probeVolume(ctx context.Context) { log := ctxzap.Extract(ctx).Sugar() - log.Debug("Scaning SCSI host...") + log.Debug("Scanning SCSI host...") scsiPath := "/sys/class/scsi_host/" - if dirs, err := ioutil.ReadDir(scsiPath); err == nil { + if dirs, err := os.ReadDir(scsiPath); err == nil { for _, f := range dirs { name := scsiPath + f.Name() + "/scan" data := []byte("- - -") - if err = ioutil.WriteFile(name, data, 0666); err != nil { + if err = os.WriteFile(name, data, 0o666); err != nil { //nolint:gosec log.Warnf("Failed to rescan scsi host %s", name) } } @@ -142,6 +145,7 @@ func diskUUIDToSerial(uuid string) string { if len(uuidWithoutHyphen) < 20 { return uuidWithoutHyphen } + return uuidWithoutHyphen[:20] } @@ -151,21 +155,23 @@ func (*mounter) ExistsPath(filename string) (bool, error) { } else if err != nil { return false, err } + return true, nil } func (*mounter) MakeDir(pathname string) error { - err := os.MkdirAll(pathname, os.FileMode(0755)) + err := os.MkdirAll(pathname, os.FileMode(0o755)) if err != nil { if !os.IsExist(err) { return err } } + return nil } func (*mounter) MakeFile(pathname string) error { - f, err := os.OpenFile(pathname, os.O_CREATE, os.FileMode(0644)) + f, err := os.OpenFile(pathname, os.O_CREATE, os.FileMode(0o644)) if err != nil { if !os.IsExist(err) { return err @@ -174,9 +180,10 @@ func (*mounter) MakeFile(pathname string) error { if err = f.Close(); err != nil { return err } + return nil } -func (*mounter) NewResizeFs(exec exec.Interface) *mount.ResizeFs { +func (*mounter) NewResizeFs(_ exec.Interface) *mount.ResizeFs { return mount.NewResizeFs(New()) } diff --git a/pkg/syncer/error.go b/pkg/syncer/error.go index 1c23c37..4328fcc 100644 --- a/pkg/syncer/error.go +++ b/pkg/syncer/error.go @@ -2,12 +2,13 @@ package syncer import "fmt" -type combinedError []error +type combinedErrors []error -func (errs combinedError) Error() string { +func (errs combinedErrors) Error() string { err := "Collected errors:\n" for i, e := range errs { err += fmt.Sprintf("\tError %d: %s\n", i, e.Error()) } + return err } diff --git a/pkg/syncer/name.go b/pkg/syncer/name.go index b82dcd0..e6b001d 100644 --- a/pkg/syncer/name.go +++ b/pkg/syncer/name.go @@ -20,7 +20,7 @@ func createStorageClassName(origName string) (string, error) { return "", err } - // Replace non alphanumeric characters (except .) by a space + // Replace non-alphanumeric characters (except .) by a space nonAlpha := regexp.MustCompile("[^a-zA-Z0-9.]+") name = nonAlpha.ReplaceAllString(name, " ") diff --git a/pkg/syncer/name_test.go b/pkg/syncer/name_test.go index 89e623b..82480d0 100644 --- a/pkg/syncer/name_test.go +++ b/pkg/syncer/name_test.go @@ -32,7 +32,7 @@ func TestCreateStorageClassName(t *testing.T) { for _, c := range cases { t.Run(c.OrigName, func(t *testing.T) { name, err := createStorageClassName(c.OrigName) - if err != nil && !c.ShouldErr { + if err != nil && !c.ShouldErr { //nolint:gocritic t.Error(err) } else if err == nil && c.ShouldErr { t.Error("Expected a non-nil error; error was nil") diff --git a/pkg/syncer/run.go b/pkg/syncer/run.go index 43c7751..99d5497 100644 --- a/pkg/syncer/run.go +++ b/pkg/syncer/run.go @@ -55,7 +55,7 @@ func (s syncer) Run(ctx context.Context) error { for _, offering := range diskOfferings.DiskOfferings { name, err := s.syncOffering(ctx, offering) if err != nil { - err = fmt.Errorf("Error with offering %s: %w", offering.Name, err) + err = fmt.Errorf("error with offering %s: %w", offering.Name, err) log.Println(err.Error()) errs = append(errs, err) } @@ -92,7 +92,8 @@ func (s syncer) Run(ctx context.Context) error { if len(errs) == 0 { return nil } - return combinedError(errs) + + return combinedErrors(errs) } func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffering) (string, error) { @@ -100,6 +101,7 @@ func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffer custom := offering.Iscustomized if !custom { log.Printf("Disk offering \"%s\" has a fixed size: ignoring\n", offeringName) + return "", nil } @@ -114,9 +116,7 @@ func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffer sc, err := s.k8sClient.StorageV1().StorageClasses().Get(ctx, name, metav1.GetOptions{}) if err != nil { if k8serrors.IsNotFound(err) { - // Storage class does not exist; creating it - log.Printf("Creating storage class %s", name) newSc := &storagev1.StorageClass{ @@ -133,8 +133,10 @@ func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffer }, } _, err = s.k8sClient.StorageV1().StorageClasses().Create(ctx, newSc, metav1.CreateOptions{}) + return name, err } + return "", err } @@ -155,6 +157,7 @@ func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffer if err != nil { // Updates to provisioner, reclaimpolicy, volumeBindingMode and parameters are forbidden log.Printf("Storage class %s exists but it not compatible.", name) + return name, err } @@ -166,6 +169,7 @@ func (s syncer) syncOffering(ctx context.Context, offering *cloudstack.DiskOffer sc.Labels = labels.Merge(existingLabels, s.labelsSet) _, err = s.k8sClient.StorageV1().StorageClasses().Update(ctx, sc, metav1.UpdateOptions{}) + return name, err } @@ -194,24 +198,27 @@ func checkStorageClass(sc *storagev1.StorageClass, expectedOfferingID string, ex } if len(errs) > 0 { - return combinedError(errs) + return combinedErrors(errs) } + return nil } func toDelete(oldSc, newSc []string) []string { del := make([]string, 0) - for _, old := range oldSc { + for _, oldVal := range oldSc { var found bool - for _, new := range newSc { - if new == old { + for _, newVal := range newSc { + if newVal == oldVal { found = true + break } } if !found { - del = append(del, old) + del = append(del, oldVal) } } + return del } diff --git a/pkg/syncer/syncer.go b/pkg/syncer/syncer.go index cf8fa6d..9a4fea2 100644 --- a/pkg/syncer/syncer.go +++ b/pkg/syncer/syncer.go @@ -60,6 +60,7 @@ func createK8sClient(kubeconfig, agent string) (*kubernetes.Clientset, error) { } } config.UserAgent = agent + return kubernetes.NewForConfig(config) } @@ -69,6 +70,7 @@ func createCloudStackClient(cloudstackconfig string) (*cloudstack.CloudStackClie return nil, err } client := cloudstack.NewAsyncClient(config.APIURL, config.APIKey, config.SecretKey, config.VerifySSL) + return client, nil } @@ -83,7 +85,8 @@ func createLabelsSet(label string) labels.Set { } m[key] = value } - return labels.Set(m) + + return m } // New creates a new Syncer instance. diff --git a/pkg/util/gb.go b/pkg/util/gb.go index 3454857..1523872 100644 --- a/pkg/util/gb.go +++ b/pkg/util/gb.go @@ -2,12 +2,12 @@ package util // RoundUpBytesToGB converts a size given in bytes to GB with // an upper rounding (it gives the smallest amount in GB -// which is greater than the original amount) +// which is greater than the original amount). func RoundUpBytesToGB(n int64) int64 { return (((n+1023)/1024+1023)/1024 + 1023) / 1024 } -// GigaBytesToBytes gives an exact conversion from GigaBytes to Bytes +// GigaBytesToBytes gives an exact conversion from GigaBytes to Bytes. func GigaBytesToBytes(gb int64) int64 { return gb * 1024 * 1024 * 1024 } diff --git a/pkg/util/idlocker.go b/pkg/util/idlocker.go index 802b50b..675352f 100644 --- a/pkg/util/idlocker.go +++ b/pkg/util/idlocker.go @@ -82,7 +82,7 @@ type OperationLock struct { // // example map[restore][xxx-xxx-xxx-xxx]1 // map[restore][xxx-xxx-xxx-xxx]2 - // the counter value will be increased for allowed parallel operations and + // the counter value will be increased for allowed parallel operations, and // it will be decreased when the operation is completed, when the counter // value goes to zero the `xxx-xxx-xxx` key will be removed from the // operation map. @@ -90,7 +90,7 @@ type OperationLock struct { // lock to avoid concurrent operation on map mux sync.Mutex // context for logging - ctx context.Context + ctx context.Context //nolint:containedctx } // NewOperationLock returns new OperationLock. @@ -117,7 +117,7 @@ func (ol *OperationLock) tryAcquire(op operation, volumeID string) error { case createOp: // snapshot controller make sure the pvc which is the source for the // snapshot request won't get deleted while snapshot is getting created, - // so we dont need to check for any ongoing delete operation here on the + // so we don't need to check for any ongoing delete operation here on the // volume. // increment the counter for snapshot create operation val := ol.locks[createOp][volumeID]