Deprecate Snow Device Support

Author: AndrewSirenko

docs/parameters.md

@@ -56,10 +56,6 @@ 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"
 )
@@ -547,7 +543,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 	}
 
 	switch createType {
-	case VolumeTypeGP2, VolumeTypeSC1, VolumeTypeST1, VolumeTypeSBG1, VolumeTypeSBP1, VolumeTypeStandard:
+	case VolumeTypeGP2, VolumeTypeSC1, VolumeTypeST1, VolumeTypeStandard:
 	case VolumeTypeIO1:
 		maxIops = io1MaxTotalIOPS
 		minIops = io1MinTotalIOPS
@@ -624,10 +620,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 		VolumeType:         types.VolumeType(createType),
 		Encrypted:          aws.Bool(diskOptions.Encrypted),
 		MultiAttachEnabled: aws.Bool(diskOptions.MultiAttachEnabled),
-	}
-
-	if !util.IsSBE(zone) {
-		requestInput.TagSpecifications = []types.TagSpecification{tagSpec}
+		TagSpecifications:  []types.TagSpecification{tagSpec},
 	}
 
 	// EBS doesn't handle empty outpost arn, so we have to include it only when it's non-empty
@@ -685,26 +678,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 		return nil, fmt.Errorf("timed out waiting for volume to create: %w", err)
 	}
 
-	outpostArn := aws.ToString(response.OutpostArn)
-	var resources []string
-	if util.IsSBE(zone) {
-		requestTagsInput := &ec2.CreateTagsInput{
-			Resources: append(resources, volumeID),
-			Tags:      tags,
-		}
-		_, err := c.ec2.CreateTags(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 _, deleteDiskErr := c.DeleteDisk(ctx, volumeID); deleteDiskErr != nil {
-				klog.ErrorS(deleteDiskErr, "failed to be deleted, this may cause volume leak", "volumeID", volumeID)
-			} else {
-				klog.V(5).InfoS("volume is deleted because there was an error while attaching the tags", "volumeID", volumeID)
-			}
-			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
+	return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID, OutpostArn: aws.ToString(response.OutpostArn)}, nil
 }
 
 // execBatchDescribeVolumesModifications executes a batched DescribeVolumesModifications API call.

pkg/cloud/cloud.go

@@ -42,7 +42,6 @@ import (
 const (
 	defaultZone     = "test-az"
 	expZone         = "us-west-2b"
-	snowZone        = "snow"
 	defaultVolumeID = "vol-test-1234"
 	defaultNodeID   = "node-1234"
 	defaultPath     = "/dev/xvdaa"
@@ -1263,41 +1262,6 @@ 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:     errors.New("CreateTags generic error"),
-			expDisk: &Disk{
-				VolumeID:         "vol-test",
-				CapacityGiB:      1,
-				AvailabilityZone: snowZone,
-			},
-			expErr: errors.New("could not attach tags to volume: vol-test. CreateTags generic error"),
-		},
 		{
 			name:       "success: create default volume with throughput",
 			volumeName: "vol-test-name",
@@ -1439,10 +1403,6 @@ func TestCreateDisk(t *testing.T) {
 						},
 					},
 				}, tc.expDescVolumeErr).AnyTimes()
-				if tc.diskOptions.AvailabilityZone == "snow" {
-					mockEC2.EXPECT().CreateTags(gomock.Any(), gomock.Any()).Return(&ec2.CreateTagsOutput{}, tc.expCreateTagsErr)
-					mockEC2.EXPECT().DeleteVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil).AnyTimes()
-				}
 				if len(tc.diskOptions.SnapshotID) > 0 {
 					mockEC2.EXPECT().DescribeSnapshots(gomock.Any(), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []types.Snapshot{snapshot}}, nil).AnyTimes()
 				}

pkg/cloud/cloud_test.go

@@ -50,7 +50,7 @@ var DefaultEC2MetadataClient = func() (EC2Metadata, error) {
 	return svc, nil
 }
 
-func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metadata, error) {
+func EC2MetadataInstanceInfo(svc EC2Metadata) (*Metadata, error) {
 	docOutput, err := svc.GetInstanceIdentityDocument(context.Background(), &imds.GetInstanceIdentityDocumentInput{})
 	if err != nil {
 		return nil, fmt.Errorf("could not get EC2 instance identity metadata: %w", err)
@@ -66,38 +66,27 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada
 	}
 
 	if len(doc.Region) == 0 {
-		if len(regionFromSession) != 0 && util.IsSBE(regionFromSession) {
-			doc.Region = regionFromSession
-		} else {
-			return nil, errors.New("could not get valid EC2 region")
-		}
+		return nil, errors.New("could not get valid EC2 region")
 	}
 
 	if len(doc.AvailabilityZone) == 0 {
-		if len(regionFromSession) != 0 && util.IsSBE(regionFromSession) {
-			doc.AvailabilityZone = regionFromSession
-		} else {
-			return nil, errors.New("could not get valid EC2 availability zone")
-		}
+		return nil, errors.New("could not get valid EC2 availability zone")
 	}
 
 	attachedENIs, err := getAttachedENIs(svc)
 	if err != nil {
 		return nil, err
 	}
 
-	blockDevMappings := 0
-	if !util.IsSBE(doc.Region) {
-		mappingsOutput, mappingsOutputErr := svc.GetMetadata(context.Background(), &imds.GetMetadataInput{Path: BlockDevicesEndpoint})
-		if mappingsOutputErr != nil {
-			return nil, fmt.Errorf("could not get metadata for block device mappings: %w", mappingsOutputErr)
-		}
-		mappings, mappingsErr := io.ReadAll(mappingsOutput.Content)
-		if mappingsErr != nil {
-			return nil, fmt.Errorf("could not read block device mappings metadata content: %w", mappingsErr)
-		}
-		blockDevMappings = strings.Count(string(mappings), "ebs")
+	mappingsOutput, mappingsOutputErr := svc.GetMetadata(context.Background(), &imds.GetMetadataInput{Path: BlockDevicesEndpoint})
+	if mappingsOutputErr != nil {
+		return nil, fmt.Errorf("could not get metadata for block device mappings: %w", mappingsOutputErr)
+	}
+	mappings, mappingsErr := io.ReadAll(mappingsOutput.Content)
+	if mappingsErr != nil {
+		return nil, fmt.Errorf("could not read block device mappings metadata content: %w", mappingsErr)
 	}
+	blockDevMappings := strings.Count(string(mappings), "ebs")
 
 	instanceInfo := Metadata{
 		InstanceID:             doc.InstanceID,

pkg/cloud/metadata/ec2.go

@@ -52,7 +52,7 @@ func NewMetadataService(cfg MetadataServiceConfig, region string) (MetadataServi
 		klog.V(2).InfoS("Environment variable AWS_EC2_METADATA_DISABLED set to 'true'. Will not rely on IMDS for instance metadata")
 	} else {
 		klog.V(2).InfoS("Attempting to retrieve instance metadata from IMDS")
-		metadata, err := retrieveEC2Metadata(cfg.EC2MetadataClient, region)
+		metadata, err := retrieveEC2Metadata(cfg.EC2MetadataClient)
 		if err == nil {
 			klog.V(2).InfoS("Retrieved metadata from IMDS")
 			return metadata.overrideRegion(region), nil
@@ -88,13 +88,13 @@ func (m *Metadata) UpdateMetadata() error {
 	return nil
 }
 
-func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient, region string) (*Metadata, error) {
+func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient) (*Metadata, error) {
 	svc, err := ec2MetadataClient()
 	if err != nil {
 		klog.ErrorS(err, "failed to initialize EC2 Metadata client")
 		return nil, err
 	}
-	return EC2MetadataInstanceInfo(svc, region)
+	return EC2MetadataInstanceInfo(svc)
 }
 
 func retrieveK8sMetadata(k8sAPIClient KubernetesAPIClient) (*Metadata, error) {

pkg/cloud/metadata/metadata.go

@@ -169,11 +169,10 @@ func TestEC2MetadataInstanceInfo(t *testing.T) {
 	defer ctrl.Finish()
 
 	testCases := []struct {
-		name              string
-		regionFromSession string
-		mockEC2Metadata   func(m *MockEC2Metadata)
-		expectedMetadata  *Metadata
-		expectedError     error
+		name             string
+		mockEC2Metadata  func(m *MockEC2Metadata)
+		expectedMetadata *Metadata
+		expectedError    error
 	}{
 		{
 			name: "TestEC2MetadataInstanceInfo: Error getting instance identity document",
@@ -368,33 +367,6 @@ func TestEC2MetadataInstanceInfo(t *testing.T) {
 				OutpostArn:             arn.ARN{},
 			},
 		},
-		{
-			name:              "TestEC2MetadataInstanceInfo: Valid metadata retrieving snow region/AZ from session",
-			regionFromSession: "snow",
-			mockEC2Metadata: func(m *MockEC2Metadata) {
-				m.EXPECT().GetInstanceIdentityDocument(gomock.Any(), &imds.GetInstanceIdentityDocumentInput{}).Return(&imds.GetInstanceIdentityDocumentOutput{
-					InstanceIdentityDocument: imds.InstanceIdentityDocument{
-						InstanceID:       "i-1234567890abcdef0",
-						InstanceType:     "c5.xlarge",
-						Region:           "",
-						AvailabilityZone: "",
-					},
-				}, nil)
-				m.EXPECT().GetMetadata(gomock.Any(), &imds.GetMetadataInput{Path: EnisEndpoint}).Return(&imds.GetMetadataOutput{
-					Content: io.NopCloser(strings.NewReader("01:23:45:67:89:ab\n02:23:45:67:89:ab")),
-				}, nil)
-				m.EXPECT().GetMetadata(gomock.Any(), &imds.GetMetadataInput{Path: OutpostArnEndpoint}).Return(nil, errors.New("404 - Not Found"))
-			},
-			expectedMetadata: &Metadata{
-				InstanceID:             "i-1234567890abcdef0",
-				InstanceType:           "c5.xlarge",
-				Region:                 "snow",
-				AvailabilityZone:       "snow",
-				NumAttachedENIs:        2,
-				NumBlockDeviceMappings: 0,
-				OutpostArn:             arn.ARN{},
-			},
-		},
 	}
 
 	for _, tc := range testCases {
@@ -405,7 +377,7 @@ func TestEC2MetadataInstanceInfo(t *testing.T) {
 			mockEC2Metadata := NewMockEC2Metadata(mockCtrl)
 			tc.mockEC2Metadata(mockEC2Metadata)
 
-			metadata, err := EC2MetadataInstanceInfo(mockEC2Metadata, tc.regionFromSession)
+			metadata, err := EC2MetadataInstanceInfo(mockEC2Metadata)
 
 			if tc.expectedError != nil {
 				require.EqualError(t, err, tc.expectedError.Error())

pkg/cloud/metadata/metadata_test.go

@@ -49,9 +49,6 @@ const (
 
 	// VolumeOperationAlreadyExists is message fmt returned to CO when there is another in-flight call on the given volumeID.
 	VolumeOperationAlreadyExists = "An operation with the given volume=%q is already in progress"
-
-	// sbeDeviceVolumeAttachmentLimit refers to the maximum number of volumes that can be attached to an instance on snow.
-	sbeDeviceVolumeAttachmentLimit = 10
 )
 
 var (
@@ -773,9 +770,6 @@ func (d *NodeService) getVolumesLimit() int64 {
 		klog.V(4).InfoS("getVolumesLimit: VolumeAttachLimit manually set to", d.options.VolumeAttachLimit, "overriding the default value")
 		return d.options.VolumeAttachLimit
 	}
-	if util.IsSBE(d.metadata.GetRegion()) {
-		return sbeDeviceVolumeAttachmentLimit
-	}
 
 	instanceType := d.metadata.GetInstanceType()
 	klog.V(4).InfoS("getVolumesLimit:", "instanceType", instanceType)