Skip to content

Commit

Permalink
Revert kubernetes-sigs#1843 - Remove block-device-mapping from attach…
Browse files Browse the repository at this point in the history
… limit

Signed-off-by: Eddie Torres <[email protected]>
  • Loading branch information
torredil committed Dec 11, 2023
1 parent b894ce5 commit 80fbe37
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 20 deletions.
17 changes: 11 additions & 6 deletions pkg/cloud/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ 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
OutpostArn arn.ARN
InstanceID string
InstanceType string
Region string
AvailabilityZone string
NumAttachedENIs int
NumBlockDeviceMappings int
OutpostArn arn.ARN
}

const (
Expand Down Expand Up @@ -71,6 +72,10 @@ 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: 15 additions & 5 deletions pkg/cloud/metadata_ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,23 @@ 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,
InstanceID: doc.InstanceID,
InstanceType: doc.InstanceType,
Region: doc.Region,
AvailabilityZone: doc.AvailabilityZone,
NumAttachedENIs: attachedENIs,
NumBlockDeviceMappings: blockDevMappings,
}

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

Expand Down
11 changes: 6 additions & 5 deletions pkg/cloud/metadata_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,12 @@ 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
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,
}

return &instanceInfo, nil
Expand Down
25 changes: 22 additions & 3 deletions pkg/cloud/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ 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 @@ -317,6 +318,20 @@ 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 @@ -326,9 +341,10 @@ func TestNewMetadataService(t *testing.T) {
Region: "",
AvailabilityZone: "",
},
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
regionFromSession: snowRegion,
imdsENIOutput: "00:00:00:00:00:00",
expectedENIs: 1,
regionFromSession: snowRegion,
expectedBlockDevices: 0,
},
}

Expand Down Expand Up @@ -409,6 +425,9 @@ 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: 14 additions & 0 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: 2 additions & 1 deletion pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,7 @@ 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 @@ -788,7 +789,7 @@ func (d *nodeService) getVolumesLimit() int64 {
nvmeInstanceStoreVolumes := cloud.GetNVMeInstanceStoreVolumesForInstanceType(instanceType)
availableAttachments = availableAttachments - enis - nvmeInstanceStoreVolumes
}
availableAttachments = availableAttachments - 1 // -1 for root device
availableAttachments = availableAttachments - blockVolumes - 1 // -1 for root device
if availableAttachments < 0 {
availableAttachments = 0
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2025,6 +2025,7 @@ func TestNodeGetInfo(t *testing.T) {
availabilityZone string
region string
attachedENIs int
blockDevices int
volumeAttachLimit int64
expMaxVolumes int64
outpostArn arn.ARN
Expand Down Expand Up @@ -2136,6 +2137,54 @@ 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 @@ -2168,6 +2217,7 @@ 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 80fbe37

Please sign in to comment.