Skip to content

Commit

Permalink
Merge pull request #1314 from jigisha620/snow
Browse files Browse the repository at this point in the history
Enable EBS CSI driver for snow device
  • Loading branch information
k8s-ci-robot authored Aug 12, 2022
2 parents 4b64c4e + ed527f4 commit 6022149
Show file tree
Hide file tree
Showing 16 changed files with 221 additions and 38 deletions.
40 changes: 32 additions & 8 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ const (
VolumeTypeSC1 = "sc1"
// VolumeTypeST1 represents a throughput-optimized HDD type of volume.
VolumeTypeST1 = "st1"
// VolumeTypeSBG1 represents a capacity-optimized HDD type of volume. Only for SBE devices.
VolumeTypeSBG1 = "sbg1"
// VolumeTypeSBP1 represents a performance-optimized SSD type of volume. Only for SBE devices.
VolumeTypeSBP1 = "sbp1"
// VolumeTypeStandard represents a previous type of volume.
VolumeTypeStandard = "standard"
)
Expand Down Expand Up @@ -280,7 +284,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
capacityGiB := util.BytesToGiB(diskOptions.CapacityBytes)

switch diskOptions.VolumeType {
case VolumeTypeGP2, VolumeTypeSC1, VolumeTypeST1, VolumeTypeStandard:
case VolumeTypeGP2, VolumeTypeSC1, VolumeTypeST1, VolumeTypeSBG1, VolumeTypeSBP1, VolumeTypeStandard:
createType = diskOptions.VolumeType
case VolumeTypeIO1:
createType = diskOptions.VolumeType
Expand Down Expand Up @@ -331,12 +335,15 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
clientToken := sha256.Sum256([]byte(volumeName))

requestInput := &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(zone),
ClientToken: aws.String(hex.EncodeToString(clientToken[:])),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Encrypted: aws.Bool(diskOptions.Encrypted),
AvailabilityZone: aws.String(zone),
ClientToken: aws.String(hex.EncodeToString(clientToken[:])),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
Encrypted: aws.Bool(diskOptions.Encrypted),
}

if !util.IsSBE(zone) {
requestInput.TagSpecifications = []*ec2.TagSpecification{&tagSpec}
}

// EBS doesn't handle empty outpost arn, so we have to include it only when it's non-empty
Expand Down Expand Up @@ -392,7 +399,24 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
}

outpostArn := aws.StringValue(response.OutpostArn)

var resources []*string
if util.IsSBE(zone) {
requestTagsInput := &ec2.CreateTagsInput{
Resources: append(resources, &volumeID),
Tags: tags,
}
_, err := c.ec2.CreateTagsWithContext(ctx, requestTagsInput)
if err != nil {
// To avoid leaking volume, we should delete the volume just created
// TODO: Need to figure out how to handle DeleteDisk failed scenario instead of just log the error
if _, error := c.DeleteDisk(ctx, volumeID); error != nil {
klog.Errorf("%v failed to be deleted, this may cause volume leak", volumeID)
} 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 &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID, OutpostArn: outpostArn}, nil
}

Expand Down
41 changes: 41 additions & 0 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
const (
defaultZone = "test-az"
expZone = "us-west-2b"
snowZone = "snow"
)

func TestCreateDisk(t *testing.T) {
Expand All @@ -49,6 +50,7 @@ func TestCreateDisk(t *testing.T) {
expErr error
expCreateVolumeErr error
expDescVolumeErr error
expCreateTagsErr error
expCreateVolumeInput *ec2.CreateVolumeInput
}{
{
Expand Down Expand Up @@ -438,6 +440,41 @@ func TestCreateDisk(t *testing.T) {
},
expErr: nil,
},
{
name: "success: create volume when zone is snow and add tags",
volumeName: "vol-test-name",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"},
AvailabilityZone: snowZone,
VolumeType: "sbp1",
},
expCreateVolumeInput: &ec2.CreateVolumeInput{},
expDisk: &Disk{
VolumeID: "vol-test",
CapacityGiB: 1,
AvailabilityZone: snowZone,
},
expErr: nil,
},
{
name: "fail: zone is snow and add tags throws error",
volumeName: "vol-test-name",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"},
AvailabilityZone: snowZone,
VolumeType: "sbg1",
},
expCreateVolumeInput: &ec2.CreateVolumeInput{},
expCreateTagsErr: fmt.Errorf("CreateTags generic error"),
expDisk: &Disk{
VolumeID: "vol-test",
CapacityGiB: 1,
AvailabilityZone: snowZone,
},
expErr: fmt.Errorf("could not attach tags to volume: vol-test. CreateTags generic error"),
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -469,6 +506,10 @@ func TestCreateDisk(t *testing.T) {
matcher := eqCreateVolume(tc.expCreateVolumeInput)
mockEC2.EXPECT().CreateVolumeWithContext(gomock.Eq(ctx), matcher).Return(vol, tc.expCreateVolumeErr)
mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []*ec2.Volume{vol}}, tc.expDescVolumeErr).AnyTimes()
if tc.diskOptions.AvailabilityZone == "snow" {
mockEC2.EXPECT().CreateTagsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.CreateTagsOutput{}, tc.expCreateTagsErr)
mockEC2.EXPECT().DeleteVolumeWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil).AnyTimes()
}
if len(tc.diskOptions.SnapshotID) > 0 {
mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []*ec2.Snapshot{snapshot}}, nil).AnyTimes()
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cloud/ec2_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ type EC2 interface {
ModifyVolumeWithContext(ctx aws.Context, input *ec2.ModifyVolumeInput, opts ...request.Option) (*ec2.ModifyVolumeOutput, error)
DescribeVolumesModificationsWithContext(ctx aws.Context, input *ec2.DescribeVolumesModificationsInput, opts ...request.Option) (*ec2.DescribeVolumesModificationsOutput, error)
DescribeAvailabilityZonesWithContext(ctx aws.Context, input *ec2.DescribeAvailabilityZonesInput, opts ...request.Option) (*ec2.DescribeAvailabilityZonesOutput, error)
CreateTagsWithContext(ctx aws.Context, input *ec2.CreateTagsInput, opts ...request.Option) (*ec2.CreateTagsOutput, error)
}
4 changes: 2 additions & 2 deletions pkg/cloud/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (m *Metadata) GetOutpostArn() arn.ARN {
return m.OutpostArn
}

func NewMetadataService(ec2MetadataClient EC2MetadataClient, k8sAPIClient KubernetesAPIClient) (MetadataService, error) {
func NewMetadataService(ec2MetadataClient EC2MetadataClient, k8sAPIClient KubernetesAPIClient, region string) (MetadataService, error) {
klog.Infof("retrieving instance data from ec2 metadata")
svc, err := ec2MetadataClient()
if !svc.Available() {
Expand All @@ -90,7 +90,7 @@ func NewMetadataService(ec2MetadataClient EC2MetadataClient, k8sAPIClient Kubern
klog.Warningf("error creating ec2 metadata client: %v", err)
} else {
klog.Infof("ec2 metadata is available")
return EC2MetadataInstanceInfo(svc)
return EC2MetadataInstanceInfo(svc, region)
}

klog.Infof("retrieving instance data from kubernetes api")
Expand Down
31 changes: 23 additions & 8 deletions pkg/cloud/metadata_ec2.go
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 All @@ -19,8 +20,9 @@ var DefaultEC2MetadataClient = func() (EC2Metadata, error) {
return svc, nil
}

func EC2MetadataInstanceInfo(svc EC2Metadata) (*Metadata, error) {
func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metadata, error) {
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)
}
Expand All @@ -34,11 +36,19 @@ func EC2MetadataInstanceInfo(svc EC2Metadata) (*Metadata, error) {
}

if len(doc.Region) == 0 {
return nil, fmt.Errorf("could not get valid EC2 region")
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 {
return nil, fmt.Errorf("could not get valid EC2 availability zone")
if len(regionFromSession) != 0 && util.IsSBE(regionFromSession) {
doc.AvailabilityZone = regionFromSession
} else {
return nil, fmt.Errorf("could not get valid EC2 availability zone")
}
}

enis, err := svc.GetMetadata(enisEndpoint)
Expand All @@ -52,12 +62,17 @@ func EC2MetadataInstanceInfo(svc EC2Metadata) (*Metadata, error) {

attachedENIs := strings.Count(enis, "\n") + 1

mappings, err := svc.GetMetadata(blockDevicesEndpoint)
if err != nil {
return nil, fmt.Errorf("could not get number of block device mappings: %v", err)
//As block device mapping contains 1 volume for the AMI.
blockDevMappings := 1

if !util.IsSBE(doc.Region) {
mappings, err := 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)
}
}
// The output contains 1 volume for the AMI. Any other block device contributes to the attachment limit
blockDevMappings := strings.Count(mappings, "\n")

instanceInfo := Metadata{
InstanceID: doc.InstanceID,
Expand Down
6 changes: 3 additions & 3 deletions pkg/cloud/metadata_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ func KubernetesAPIInstanceInfo(clientset kubernetes.Interface) (*Metadata, error
return nil, fmt.Errorf("node providerID empty, cannot parse")
}

awsRegionRegex := "([a-z]{2}(-gov)?)-(central|(north|south)?(east|west)?)-[0-9]"
awsAvailabilityZoneRegex := "([a-z]{2}(-gov)?)-(central|(north|south)?(east|west)?)-[0-9][a-z]"
awsInstanceIDRegex := "i-[a-z0-9]+$"
awsRegionRegex := "(snow)|(([a-z]{2}(-gov)?)-(central|(north|south)?(east|west)?)-[0-9])"
awsAvailabilityZoneRegex := "(snow)|(([a-z]{2}(-gov)?)-(central|(north|south)?(east|west)?)-[0-9][a-z])"
awsInstanceIDRegex := "s\\.i-[a-z0-9]+|i-[a-z0-9]+$"

re := regexp.MustCompile(awsRegionRegex)
region := re.FindString(providerID)
Expand Down
42 changes: 32 additions & 10 deletions pkg/cloud/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +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"
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 All @@ -63,6 +65,7 @@ func TestNewMetadataService(t *testing.T) {
expectedErr error
node v1.Node
nodeNameEnvVar string
regionFromSession string
}{
{
name: "success: normal",
Expand Down Expand Up @@ -314,6 +317,20 @@ func TestNewMetadataService(t *testing.T) {
imdsBlockDeviceOutput: "ami\nroot\nebs1\nebs2",
expectedBlockDevices: 3,
},
{
name: "success: region from session is snow",
ec2metadataAvailable: true,
getInstanceIdentityDocumentValue: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
InstanceType: stdInstanceType,
Region: "",
AvailabilityZone: "",
},
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
regionFromSession: snowRegion,
expectedBlockDevices: 1,
},
}

for _, tc := range testCases {
Expand All @@ -339,7 +356,7 @@ func TestNewMetadataService(t *testing.T) {
// output
if tc.getInstanceIdentityDocumentError == nil && !tc.invalidInstanceIdentityDocument {
mockEC2Metadata.EXPECT().GetMetadata(enisEndpoint).Return(tc.imdsENIOutput, nil)
mockEC2Metadata.EXPECT().GetMetadata(blockDevicesEndpoint).Return(tc.imdsBlockDeviceOutput, nil)
mockEC2Metadata.EXPECT().GetMetadata(blockDevicesEndpoint).Return(tc.imdsBlockDeviceOutput, nil).AnyTimes()

if tc.getMetadataValue != "" || tc.getMetadataError != nil {
mockEC2Metadata.EXPECT().GetMetadata(outpostArnEndpoint).Return(tc.getMetadataValue, tc.getMetadataError)
Expand All @@ -358,8 +375,13 @@ func TestNewMetadataService(t *testing.T) {
}

os.Setenv("CSI_NODE_NAME", tc.nodeNameEnvVar)

m, err := NewMetadataService(ec2MetadataClient, k8sAPIClient)
var m MetadataService
var err error
if tc.regionFromSession == snowRegion {
m, err = NewMetadataService(ec2MetadataClient, k8sAPIClient, snowRegion)
} else {
m, err = NewMetadataService(ec2MetadataClient, k8sAPIClient, stdRegion)
}
if err != nil {
if tc.expectedErr == nil {
t.Errorf("got error %q, expected no error", err)
Expand All @@ -376,10 +398,10 @@ func TestNewMetadataService(t *testing.T) {
if m.GetInstanceType() != stdInstanceType {
t.Errorf("GetInstanceType() failed: got wrong instance type %v, expected %v", m.GetInstanceType(), stdInstanceType)
}
if m.GetRegion() != stdRegion {
if m.GetRegion() != stdRegion && m.GetRegion() != snowRegion {
t.Errorf("NewMetadataService() failed: got wrong region %v, expected %v", m.GetRegion(), stdRegion)
}
if m.GetAvailabilityZone() != stdAvailabilityZone {
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
20 changes: 20 additions & 0 deletions pkg/cloud/mock_ec2.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func newControllerService(driverOptions *DriverOptions) controllerService {
region := os.Getenv("AWS_REGION")
if region == "" {
klog.V(5).Infof("[Debug] Retrieving region from metadata service")
metadata, err := NewMetadataFunc(cloud.DefaultEC2MetadataClient, cloud.DefaultKubernetesAPIClient)
metadata, err := NewMetadataFunc(cloud.DefaultEC2MetadataClient, cloud.DefaultKubernetesAPIClient, region)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestNewControllerService(t *testing.T) {

oldNewMetadataFunc := NewMetadataFunc
defer func() { NewMetadataFunc = oldNewMetadataFunc }()
NewMetadataFunc = func(cloud.EC2MetadataClient, cloud.KubernetesAPIClient) (cloud.MetadataService, error) {
NewMetadataFunc = func(cloud.EC2MetadataClient, cloud.KubernetesAPIClient, string) (cloud.MetadataService, error) {
if tc.newMetadataFuncErrors {
return nil, testErr
}
Expand Down
Loading

0 comments on commit 6022149

Please sign in to comment.