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.

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

Upgrade golangci-lint; Fix linter errors #1435

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ bin/mockgen: | bin

bin/golangci-lint: | bin
echo "Installing golangci-lint..."
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.21.0
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.50.1

.PHONY: kubeval
kubeval: bin/kubeval
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ require (
github.com/aws/aws-sdk-go v1.44.122
github.com/container-storage-interface/spec v1.6.0
github.com/golang/mock v1.6.0
github.com/golang/protobuf v1.5.2
github.com/google/go-cmp v0.5.9
github.com/kubernetes-csi/csi-proxy/client v1.1.2
github.com/kubernetes-csi/csi-test v2.2.0+incompatible
Expand All @@ -14,6 +13,7 @@ require (
github.com/stretchr/testify v1.8.1
golang.org/x/sys v0.1.0
google.golang.org/grpc v1.50.1
google.golang.org/protobuf v1.28.1
k8s.io/api v0.25.3
k8s.io/apimachinery v0.25.3
k8s.io/client-go v1.22.11
Expand Down Expand Up @@ -43,6 +43,7 @@ require (
github.com/go-openapi/swag v0.22.3 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/gnostic v0.6.9 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/uuid v1.3.0 // indirect
Expand Down Expand Up @@ -91,7 +92,6 @@ require (
golang.org/x/tools v0.2.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20221025140454-527a21cfbd71 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
Expand Down
60 changes: 29 additions & 31 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,10 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *

zone := diskOptions.AvailabilityZone
if zone == "" {
var err error
zone, err = c.randomAvailabilityZone(ctx)
klog.V(5).Infof("[Debug] AZ is not provided. Using node AZ [%s]", zone)
if err != nil {
return nil, fmt.Errorf("failed to get availability zone %s", err)
return nil, fmt.Errorf("failed to get availability zone %w", err)
}
}

Expand Down Expand Up @@ -410,7 +409,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
if isAWSErrorIdempotentParameterMismatch(err) {
return nil, ErrIdempotentParameterMismatch
}
return nil, fmt.Errorf("could not create volume in EC2: %v", err)
return nil, fmt.Errorf("could not create volume in EC2: %w", err)
}

volumeID := aws.StringValue(response.VolumeId)
Expand All @@ -431,7 +430,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
} else {
klog.V(5).Infof("[Debug] %v is deleted because it is not in desired state within retry limit", volumeID)
}
return nil, fmt.Errorf("failed to get an available volume in EC2: %v", err)
return nil, fmt.Errorf("failed to get an available volume in EC2: %w", err)
}

outpostArn := aws.StringValue(response.OutpostArn)
Expand All @@ -450,7 +449,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
} else {
klog.V(5).Infof("[Debug] %v is deleted because there was an error while attaching the tags", volumeID)
}
return nil, fmt.Errorf("could not attach tags to volume: %v. %v", volumeID, err)
return nil, fmt.Errorf("could not attach tags to volume: %v. %w", volumeID, err)
}
}
return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID, OutpostArn: outpostArn}, nil
Expand All @@ -462,7 +461,7 @@ func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) {
if isAWSErrorVolumeNotFound(err) {
return false, ErrNotFound
}
return false, fmt.Errorf("DeleteDisk could not delete volume: %v", err)
return false, fmt.Errorf("DeleteDisk could not delete volume: %w", err)
}
return true, nil
}
Expand All @@ -486,17 +485,17 @@ func (c *cloud) AttachDisk(ctx context.Context, volumeID, nodeID string) (string
VolumeId: aws.String(volumeID),
}

resp, err := c.ec2.AttachVolumeWithContext(ctx, request)
if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
resp, attachErr := c.ec2.AttachVolumeWithContext(ctx, request)
if attachErr != nil {
var awsErr awserr.Error
if errors.As(attachErr, &awsErr) {
if awsErr.Code() == "VolumeInUse" {
return "", ErrVolumeInUse
}
}
return "", fmt.Errorf("could not attach volume %q to node %q: %v", volumeID, nodeID, err)
return "", fmt.Errorf("could not attach volume %q to node %q: %w", volumeID, nodeID, attachErr)
}
klog.V(5).Infof("[Debug] AttachVolume volume=%q instance=%q request returned %v", volumeID, nodeID, resp)

}

attachment, err := c.WaitForAttachmentState(ctx, volumeID, volumeAttachedState, *instance.InstanceId, device.Path, device.IsAlreadyAssigned)
Expand Down Expand Up @@ -557,7 +556,7 @@ func (c *cloud) DetachDisk(ctx context.Context, volumeID, nodeID string) error {
isAWSErrorVolumeNotFound(err) {
return ErrNotFound
}
return fmt.Errorf("could not detach volume %q from node %q: %v", volumeID, nodeID, err)
return fmt.Errorf("could not detach volume %q from node %q: %w", volumeID, nodeID, err)
}

attachment, err := c.WaitForAttachmentState(ctx, volumeID, volumeDetachedState, *instance.InstanceId, "", false)
Expand Down Expand Up @@ -756,7 +755,7 @@ func (c *cloud) CreateSnapshot(ctx context.Context, volumeID string, snapshotOpt

res, err := c.ec2.CreateSnapshotWithContext(ctx, request)
if err != nil {
return nil, fmt.Errorf("error creating snapshot of volume %s: %v", volumeID, err)
return nil, fmt.Errorf("error creating snapshot of volume %s: %w", volumeID, err)
}
if res == nil {
return nil, fmt.Errorf("nil CreateSnapshotResponse")
Expand All @@ -773,7 +772,7 @@ func (c *cloud) DeleteSnapshot(ctx context.Context, snapshotID string) (success
if isAWSErrorSnapshotNotFound(err) {
return false, ErrNotFound
}
return false, fmt.Errorf("DeleteSnapshot could not delete volume: %v", err)
return false, fmt.Errorf("DeleteSnapshot could not delete volume: %w", err)
}
return true, nil
}
Expand Down Expand Up @@ -913,7 +912,7 @@ func (c *cloud) getInstance(ctx context.Context, nodeID string) (*ec2.Instance,
if isAWSErrorInstanceNotFound(err) {
return nil, ErrNotFound
}
return nil, fmt.Errorf("error listing AWS instances: %q", err)
return nil, fmt.Errorf("error listing AWS instances: %w", err)
}

for _, reservation := range response.Reservations {
Expand Down Expand Up @@ -1017,8 +1016,9 @@ func (c *cloud) waitForVolume(ctx context.Context, volumeID string) error {
// and has the given code. More information on AWS error codes at:
// https://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html
func isAWSError(err error, code string) bool {
if awsError, ok := err.(awserr.Error); ok {
if awsError.Code() == code {
var awsErr awserr.Error
if errors.As(err, &awsErr) {
if awsErr.Code() == code {
return true
}
}
Expand Down Expand Up @@ -1095,7 +1095,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in
if latestMod != nil && modFetchError == nil {
state := aws.StringValue(latestMod.ModificationState)
if state == ec2.VolumeModificationStateModifying {
_, err = c.waitForVolumeSize(ctx, volumeID)
err = c.waitForVolumeSize(ctx, volumeID)
if err != nil {
return oldSizeGiB, err
}
Expand All @@ -1105,16 +1105,16 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in

// if there was an error fetching volume modifications and it was anything other than VolumeNotBeingModified error
// that means we have an API problem.
if modFetchError != nil && modFetchError != VolumeNotBeingModified {
return oldSizeGiB, fmt.Errorf("error fetching volume modifications for %q: %v", volumeID, modFetchError)
if modFetchError != nil && !errors.Is(modFetchError, VolumeNotBeingModified) {
return oldSizeGiB, fmt.Errorf("error fetching volume modifications for %q: %w", volumeID, modFetchError)
}

// Even if existing volume size is greater than user requested size, we should ensure that there are no pending
// volume modifications objects or volume has completed previously issued modification request.
if oldSizeGiB >= newSizeGiB {
klog.V(5).Infof("[Debug] Volume %q current size (%d GiB) is greater or equal to the new size (%d GiB)", volumeID, oldSizeGiB, newSizeGiB)
_, err = c.waitForVolumeSize(ctx, volumeID)
if err != nil && err != VolumeNotBeingModified {
err = c.waitForVolumeSize(ctx, volumeID)
if err != nil && !errors.Is(err, VolumeNotBeingModified) {
return oldSizeGiB, err
}
return oldSizeGiB, nil
Expand All @@ -1128,7 +1128,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in
klog.V(4).Infof("expanding volume %q to size %d", volumeID, newSizeGiB)
response, err := c.ec2.ModifyVolumeWithContext(ctx, req)
if err != nil {
return 0, fmt.Errorf("could not modify AWS volume %q: %v", volumeID, err)
return 0, fmt.Errorf("could not modify AWS volume %q: %w", volumeID, err)
}

mod := response.VolumeModification
Expand All @@ -1138,7 +1138,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in
return c.checkDesiredSize(ctx, volumeID, newSizeGiB)
}

_, err = c.waitForVolumeSize(ctx, volumeID)
err = c.waitForVolumeSize(ctx, volumeID)
if err != nil {
return oldSizeGiB, err
}
Expand Down Expand Up @@ -1167,15 +1167,14 @@ func (c *cloud) checkDesiredSize(ctx context.Context, volumeID string, newSizeGi
return oldSizeGiB, fmt.Errorf("volume %q is still being expanded to %d size", volumeID, newSizeGiB)
}

// waitForVolumeSize waits for a volume modification to finish and return its size.
func (c *cloud) waitForVolumeSize(ctx context.Context, volumeID string) (int64, error) {
// waitForVolumeSize waits for a volume modification to finish.
func (c *cloud) waitForVolumeSize(ctx context.Context, volumeID string) error {
backoff := wait.Backoff{
Duration: volumeModificationDuration,
Factor: volumeModificationWaitFactor,
Steps: volumeModificationWaitSteps,
}

var modVolSizeGiB int64
waitErr := wait.ExponentialBackoff(backoff, func() (bool, error) {
m, err := c.getLatestVolumeModification(ctx, volumeID)
if err != nil {
Expand All @@ -1184,18 +1183,17 @@ func (c *cloud) waitForVolumeSize(ctx context.Context, volumeID string) (int64,

state := aws.StringValue(m.ModificationState)
if volumeModificationDone(state) {
modVolSizeGiB = aws.Int64Value(m.TargetSize)
return true, nil
}

return false, nil
})

if waitErr != nil {
return 0, waitErr
return waitErr
}

return modVolSizeGiB, nil
return nil
}

// getLatestVolumeModification returns the last modification of the volume.
Expand All @@ -1210,7 +1208,7 @@ func (c *cloud) getLatestVolumeModification(ctx context.Context, volumeID string
if isAWSErrorModificationNotFound(err) {
return nil, VolumeNotBeingModified
}
return nil, fmt.Errorf("error describing modifications in volume %q: %v", volumeID, err)
return nil, fmt.Errorf("error describing modifications in volume %q: %w", volumeID, err)
}

volumeMods := mod.VolumesModifications
Expand Down
5 changes: 3 additions & 2 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1582,8 +1582,9 @@ func TestListSnapshots(t *testing.T) {

mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{}, nil)

if _, err := c.ListSnapshots(ctx, "", 0, ""); err != nil {
if err != ErrNotFound {
_, err := c.ListSnapshots(ctx, "", 0, "")
if err != nil {
if !errors.Is(err, ErrNotFound) {
t.Fatalf("Expected error %v, got %v", ErrNotFound, err)
}
} else {
Expand Down
12 changes: 6 additions & 6 deletions pkg/cloud/metadata_ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada
doc, err := svc.GetInstanceIdentityDocument()
klog.Infof("regionFromSession %v", regionFromSession)
if err != nil {
return nil, fmt.Errorf("could not get EC2 instance identity metadata: %v", err)
return nil, fmt.Errorf("could not get EC2 instance identity metadata: %w", err)
}

if len(doc.InstanceID) == 0 {
Expand Down Expand Up @@ -53,7 +53,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada

enis, err := svc.GetMetadata(enisEndpoint)
if err != nil {
return nil, fmt.Errorf("could not get number of attached ENIs: %v", err)
return nil, fmt.Errorf("could not get number of attached ENIs: %w", err)
}
// the ENIs should not be empty; if (somehow) it is empty, return an error
if enis == "" {
Expand All @@ -66,11 +66,11 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada
blockDevMappings := 1

if !util.IsSBE(doc.Region) {
mappings, err := svc.GetMetadata(blockDevicesEndpoint)
mappings, mapErr := svc.GetMetadata(blockDevicesEndpoint)
// The output contains 1 volume for the AMI. Any other block device contributes to the attachment limit
blockDevMappings = strings.Count(mappings, "\n")
if err != nil {
return nil, fmt.Errorf("could not get number of block device mappings: %v", err)
if mapErr != nil {
return nil, fmt.Errorf("could not get number of block device mappings: %w", err)
}
}

Expand All @@ -88,7 +88,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada
// it's guaranteed to be in the form `arn:<partition>:outposts:<region>:<account>:outpost/<outpost-id>`
// There's a case to be made here to ignore the error so a failure here wouldn't affect non-outpost calls.
if err != nil && !strings.Contains(err.Error(), "404") {
return nil, fmt.Errorf("something went wrong while getting EC2 outpost arn: %s", err.Error())
return nil, fmt.Errorf("something went wrong while getting EC2 outpost arn: %w", err)
} else if err == nil {
klog.Infof("Running in an outpost environment with arn: %s", outpostArn)
outpostArn = strings.ReplaceAll(outpostArn, "outpost/", "")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/metadata_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func KubernetesAPIInstanceInfo(clientset kubernetes.Interface) (*Metadata, error
// get node with k8s API
node, err := clientset.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("error getting Node %v: %v", nodeName, err)
return nil, fmt.Errorf("error getting Node %v: %w", nodeName, err)
}

providerID := node.Spec.ProviderID
Expand Down
Loading