Skip to content

Commit

Permalink
Merge pull request #1843 from jsafrane/remove-block-device-mapping
Browse files Browse the repository at this point in the history
Remove block-device-mapping from attach limit
  • Loading branch information
k8s-ci-robot authored Nov 29, 2023
2 parents 97688f8 + 885c935 commit 61c721b
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 121 deletions.
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

0 comments on commit 61c721b

Please sign in to comment.