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

Remove block-device-mapping from attach limit #1843

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
17 changes: 6 additions & 11 deletions pkg/cloud/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import (

// Metadata is info about the ec2 instance on which the driver is running
type Metadata struct {
InstanceID string
InstanceType string
Region string
AvailabilityZone string
NumAttachedENIs int
NumBlockDeviceMappings int
OutpostArn arn.ARN
InstanceID string
InstanceType string
Region string
AvailabilityZone string
NumAttachedENIs int
OutpostArn arn.ARN
}

const (
Expand Down Expand Up @@ -72,10 +71,6 @@ func (m *Metadata) GetNumAttachedENIs() int {
return m.NumAttachedENIs
}

func (m *Metadata) GetNumBlockDeviceMappings() int {
return m.NumBlockDeviceMappings
}

// GetOutpostArn returns outpost arn if instance is running on an outpost. empty otherwise.
func (m *Metadata) GetOutpostArn() arn.ARN {
return m.OutpostArn
Expand Down
20 changes: 5 additions & 15 deletions pkg/cloud/metadata_ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,13 @@ func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metada
}

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

if !util.IsSBE(doc.Region) {
mappings, mapErr := svc.GetMetadata(blockDevicesEndpoint)
if mapErr != nil {
return nil, fmt.Errorf("could not get number of block device mappings: %w", err)
}
blockDevMappings = strings.Count(mappings, "ebs")
}

instanceInfo := Metadata{
InstanceID: doc.InstanceID,
InstanceType: doc.InstanceType,
Region: doc.Region,
AvailabilityZone: doc.AvailabilityZone,
NumAttachedENIs: attachedENIs,
NumBlockDeviceMappings: blockDevMappings,
InstanceID: doc.InstanceID,
InstanceType: doc.InstanceType,
Region: doc.Region,
AvailabilityZone: doc.AvailabilityZone,
NumAttachedENIs: attachedENIs,
}

outpostArn, err := svc.GetMetadata(outpostArnEndpoint)
Expand Down
1 change: 0 additions & 1 deletion pkg/cloud/metadata_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ type MetadataService interface {
GetRegion() string
GetAvailabilityZone() string
GetNumAttachedENIs() int
GetNumBlockDeviceMappings() int
GetOutpostArn() arn.ARN
}

Expand Down
11 changes: 5 additions & 6 deletions pkg/cloud/metadata_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,11 @@ func KubernetesAPIInstanceInfo(clientset kubernetes.Interface) (*Metadata, error
}

instanceInfo := Metadata{
InstanceID: instanceID,
InstanceType: instanceType,
Region: region,
AvailabilityZone: availabilityZone,
NumAttachedENIs: 1, // All nodes have at least 1 attached ENI, so we'll use that
NumBlockDeviceMappings: 0,
InstanceID: instanceID,
InstanceType: instanceType,
Region: region,
AvailabilityZone: availabilityZone,
NumAttachedENIs: 1, // All nodes have at least 1 attached ENI, so we'll use that
}

return &instanceInfo, nil
Expand Down
25 changes: 3 additions & 22 deletions pkg/cloud/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ func TestNewMetadataService(t *testing.T) {
imdsBlockDeviceOutput string
imdsENIOutput string
expectedENIs int
expectedBlockDevices int
expectedOutpostArn arn.ARN
expectedErr error
node v1.Node
Expand Down Expand Up @@ -318,20 +317,6 @@ func TestNewMetadataService(t *testing.T) {
imdsENIOutput: "00:00:00:00:00:00\n00:00:00:00:00:01",
expectedENIs: 2,
},
{
name: "success: GetMetadata() returns correct number of block device mappings",
ec2metadataAvailable: true,
getInstanceIdentityDocumentValue: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
InstanceType: stdInstanceType,
Region: stdRegion,
AvailabilityZone: stdAvailabilityZone,
},
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
imdsBlockDeviceOutput: "ami\nroot\nebs1\nebs2",
expectedBlockDevices: 2,
},
{
name: "success: region from session is snow",
ec2metadataAvailable: true,
Expand All @@ -341,10 +326,9 @@ func TestNewMetadataService(t *testing.T) {
Region: "",
AvailabilityZone: "",
},
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
regionFromSession: snowRegion,
expectedBlockDevices: 0,
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
regionFromSession: snowRegion,
},
}

Expand Down Expand Up @@ -425,9 +409,6 @@ func TestNewMetadataService(t *testing.T) {
if m.GetNumAttachedENIs() != tc.expectedENIs {
t.Errorf("GetMetadata() failed for %s: got %v, expected %v", enisEndpoint, m.GetNumAttachedENIs(), tc.expectedENIs)
}
if m.GetNumBlockDeviceMappings() != tc.expectedBlockDevices {
t.Errorf("GetMetadata() failed for %s: got %v, expected %v", blockDevicesEndpoint, m.GetNumBlockDeviceMappings(), tc.expectedBlockDevices)
}
}
mockCtrl.Finish()
})
Expand Down
14 changes: 0 additions & 14 deletions pkg/cloud/mock_metadata.go

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

3 changes: 1 addition & 2 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,6 @@ func (d *nodeService) getVolumesLimit() int64 {

isNitro := cloud.IsNitroInstanceType(instanceType)
availableAttachments := cloud.GetMaxAttachments(isNitro)
blockVolumes := d.metadata.GetNumBlockDeviceMappings()
dedicatedLimit := cloud.GetDedicatedLimitForInstanceType(instanceType)

// For special dedicated limit instance types, the limit is only for EBS volumes
Expand All @@ -789,7 +788,7 @@ func (d *nodeService) getVolumesLimit() int64 {
nvmeInstanceStoreVolumes := cloud.GetNVMeInstanceStoreVolumesForInstanceType(instanceType)
availableAttachments = availableAttachments - enis - nvmeInstanceStoreVolumes
}
availableAttachments = availableAttachments - blockVolumes - 1 // -1 for root device
availableAttachments = availableAttachments - 1 // -1 for root device
if availableAttachments < 0 {
availableAttachments = 0
}
Expand Down
50 changes: 0 additions & 50 deletions pkg/driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2025,7 +2025,6 @@ func TestNodeGetInfo(t *testing.T) {
availabilityZone string
region string
attachedENIs int
blockDevices int
volumeAttachLimit int64
expMaxVolumes int64
outpostArn arn.ARN
Expand Down Expand Up @@ -2137,54 +2136,6 @@ func TestNodeGetInfo(t *testing.T) {
expMaxVolumes: 11,
outpostArn: emptyOutpostArn,
},
{
name: "nitro instances already attached EBS volumes",
instanceID: "i-123456789abcdef01",
instanceType: "t3.xlarge",
availabilityZone: "us-west-2b",
region: "us-west-2",
volumeAttachLimit: -1,
attachedENIs: 1,
blockDevices: 2,
expMaxVolumes: 24,
outpostArn: emptyOutpostArn,
},
{
name: "nitro instance already attached max EBS volumes",
instanceID: "i-123456789abcdef01",
instanceType: "t3.xlarge",
availabilityZone: "us-west-2b",
region: "us-west-2",
volumeAttachLimit: -1,
attachedENIs: 1,
blockDevices: 27,
expMaxVolumes: 0,
outpostArn: emptyOutpostArn,
},
{
name: "non-nitro instance already attached max EBS volumes",
instanceID: "i-123456789abcdef01",
instanceType: "m5.xlarge",
availabilityZone: "us-west-2b",
region: "us-west-2",
volumeAttachLimit: -1,
attachedENIs: 1,
blockDevices: 39,
expMaxVolumes: 0,
outpostArn: emptyOutpostArn,
},
{
name: "nitro instance already attached max ENIs",
instanceID: "i-123456789abcdef01",
instanceType: "t3.xlarge",
availabilityZone: "us-west-2b",
region: "us-west-2",
volumeAttachLimit: -1,
attachedENIs: 27,
blockDevices: 1,
expMaxVolumes: 0,
outpostArn: emptyOutpostArn,
},
{
name: "nitro instance with dedicated limit",
instanceID: "i-123456789abcdef01",
Expand Down Expand Up @@ -2217,7 +2168,6 @@ func TestNodeGetInfo(t *testing.T) {

if tc.volumeAttachLimit < 0 {
mockMetadata.EXPECT().GetInstanceType().Return(tc.instanceType)
mockMetadata.EXPECT().GetNumBlockDeviceMappings().Return(tc.blockDevices)
if cloud.IsNitroInstanceType(tc.instanceType) && cloud.GetDedicatedLimitForInstanceType(tc.instanceType) == 0 {
mockMetadata.EXPECT().GetNumAttachedENIs().Return(tc.attachedENIs)
}
Expand Down