From 37f127b5e894005ade2f03f0aa8af71d6650c8bd Mon Sep 17 00:00:00 2001 From: jigisha620 Date: Tue, 2 Aug 2022 17:05:22 -0700 Subject: [PATCH] Changes to resolve review comments --- pkg/cloud/cloud.go | 4 ++-- pkg/cloud/metadata_ec2.go | 7 ++++--- pkg/cloud/metadata_test.go | 15 ++++++++------- pkg/driver/node.go | 3 ++- pkg/driver/node_linux.go | 3 ++- pkg/driver/node_test.go | 22 +++++++++++----------- pkg/util/util.go | 4 ++++ 7 files changed, 33 insertions(+), 25 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index ee481652fb..f227ee6712 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -341,7 +341,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * Encrypted: aws.Bool(diskOptions.Encrypted), } - if zone != "snow" { + if !util.IsSBE(zone) { requestInput.TagSpecifications = []*ec2.TagSpecification{&tagSpec} } @@ -399,7 +399,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * outpostArn := aws.StringValue(response.OutpostArn) var resources []*string - if zone == "snow" { + if util.IsSBE(zone) { requestTagsInput := &ec2.CreateTagsInput{ Resources: append(resources, &volumeID), Tags: tags, diff --git a/pkg/cloud/metadata_ec2.go b/pkg/cloud/metadata_ec2.go index 2e2830dd8e..0d8667c023 100644 --- a/pkg/cloud/metadata_ec2.go +++ b/pkg/cloud/metadata_ec2.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/ec2metadata" "github.com/aws/aws-sdk-go/aws/session" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" "k8s.io/klog" ) @@ -35,7 +36,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada } if len(doc.Region) == 0 { - if len(regionFromSession) != 0 && regionFromSession == "snow" { + if len(regionFromSession) != 0 && util.IsSBE(regionFromSession) { doc.Region = regionFromSession } else { return nil, fmt.Errorf("could not get valid EC2 region") @@ -43,7 +44,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada } if len(doc.AvailabilityZone) == 0 { - if len(regionFromSession) != 0 && regionFromSession == "snow" { + if len(regionFromSession) != 0 && util.IsSBE(regionFromSession) { doc.AvailabilityZone = regionFromSession } else { return nil, fmt.Errorf("could not get valid EC2 availability zone") @@ -63,7 +64,7 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada blockDevMappings := 1 - if doc.Region != "snow" { + if !util.IsSBE(doc.Region) { mappings, err := svc.GetMetadata(blockDevicesEndpoint) blockDevMappings = strings.Count(mappings, "\n") if err != nil { diff --git a/pkg/cloud/metadata_test.go b/pkg/cloud/metadata_test.go index 681c309db9..81a949a7d6 100644 --- a/pkg/cloud/metadata_test.go +++ b/pkg/cloud/metadata_test.go @@ -34,12 +34,13 @@ import ( ) const ( - nodeName = "ip-123-45-67-890.us-west-2.compute.internal" - stdInstanceID = "i-abcdefgh123456789" - stdInstanceType = "t2.medium" - stdRegion = "us-west-2" - stdAvailabilityZone = "us-west-2b" - snowRegion = "snow" + nodeName = "ip-123-45-67-890.us-west-2.compute.internal" + stdInstanceID = "i-abcdefgh123456789" + stdInstanceType = "t2.medium" + stdRegion = "us-west-2" + stdAvailabilityZone = "us-west-2b" + snowRegion = "snow" + snowAvailabilityZone = "snow" ) func TestNewMetadataService(t *testing.T) { @@ -400,7 +401,7 @@ func TestNewMetadataService(t *testing.T) { if m.GetRegion() != stdRegion && m.GetRegion() != snowRegion { t.Errorf("NewMetadataService() failed: got wrong region %v, expected %v", m.GetRegion(), stdRegion) } - if m.GetAvailabilityZone() != stdAvailabilityZone && m.GetAvailabilityZone() != snowRegion { + if m.GetAvailabilityZone() != stdAvailabilityZone && m.GetAvailabilityZone() != snowAvailabilityZone { t.Errorf("NewMetadataService() failed: got wrong AZ %v, expected %v", m.GetAvailabilityZone(), stdAvailabilityZone) } if m.GetOutpostArn() != tc.expectedOutpostArn { diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 23d75b2100..76b28a2f0d 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -25,6 +25,7 @@ import ( csi "github.com/container-storage-interface/spec/lib/go/csi" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/klog" @@ -698,7 +699,7 @@ func (d *nodeService) getVolumesLimit() int64 { return d.driverOptions.volumeAttachLimit } - if d.metadata.GetRegion() == "snow" { + if util.IsSBE(d.metadata.GetRegion()) { return 10 } diff --git a/pkg/driver/node_linux.go b/pkg/driver/node_linux.go index 4fdc16801d..51244e800b 100644 --- a/pkg/driver/node_linux.go +++ b/pkg/driver/node_linux.go @@ -26,6 +26,7 @@ import ( "strconv" "strings" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" "golang.org/x/sys/unix" "k8s.io/klog" ) @@ -98,7 +99,7 @@ func (d *nodeService) findDevicePath(devicePath, volumeID, partition string) (st klog.V(5).Infof("[Debug] error searching for nvme path %q: %v", nvmeName, err) } - if d.metadata.GetRegion() == "snow" { + if util.IsSBE(d.metadata.GetRegion()) { klog.V(5).Infof("[Debug] Falling back to snow volume lookup for: %q", devicePath) canonicalDevicePath = "/dev/vd" + strings.TrimPrefix(devicePath, "/dev/xvdb") } diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 4c76dcf2c7..764a4f3737 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -1925,7 +1925,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "t2.medium", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, expMaxVolumes: 39, attachedENIs: 1, @@ -1936,7 +1936,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "t2.medium", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: 42, expMaxVolumes: 42, outpostArn: emptyOutpostArn, @@ -1946,7 +1946,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "t3.xlarge", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 2, expMaxVolumes: 26, // 28 (max) - 2 (enis) @@ -1957,7 +1957,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "m5d.large", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 2, expMaxVolumes: 25, @@ -1968,7 +1968,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "m5d.large", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: 30, expMaxVolumes: 30, outpostArn: emptyOutpostArn, @@ -1978,7 +1978,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "m5d.large", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: 30, expMaxVolumes: 30, outpostArn: validOutpostArn, @@ -1988,7 +1988,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "c6i.metal", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 1, expMaxVolumes: 31, @@ -1999,7 +1999,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "u-12tb1.metal", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 1, expMaxVolumes: 19, @@ -2010,7 +2010,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "mac1.metal", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 1, expMaxVolumes: 16, @@ -2021,7 +2021,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "inf1.24xlarge", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 1, expMaxVolumes: 11, @@ -2032,7 +2032,7 @@ func TestNodeGetInfo(t *testing.T) { instanceID: "i-123456789abcdef01", instanceType: "t3.xlarge", availabilityZone: "us-west-2b", - region: "us-west-2b", + region: "us-west-2", volumeAttachLimit: -1, attachedENIs: 1, blockDevices: 2, diff --git a/pkg/util/util.go b/pkg/util/util.go index 6884dea4bb..4f68a5cc3f 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -89,3 +89,7 @@ func GetAccessModes(caps []*csi.VolumeCapability) *[]string { } return &modes } + +func IsSBE(region string) bool { + return region == "snow" +}