Skip to content

Commit

Permalink
Changes to resolve review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@jigisha620
jigisha620 committed Aug 3, 2022
1 parent 3a0d469 commit 37f127b
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 25 deletions.
4 changes: 2 additions & 2 deletions pkg/cloud/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

Expand Down Expand Up @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions pkg/cloud/metadata_ec2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -35,15 +36,15 @@ 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")
}
}

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")
Expand All @@ -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 {
Expand Down
15 changes: 8 additions & 7 deletions pkg/cloud/metadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/driver/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/driver/node_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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")
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/driver/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions pkg/util/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,7 @@ func GetAccessModes(caps []*csi.VolumeCapability) *[]string {
}
return &modes
}

func IsSBE(region string) bool {
return region == "snow"
}

0 comments on commit 37f127b

Please sign in to comment.