From f5e41814eea14b76536326da4619e3f1bce9b516 Mon Sep 17 00:00:00 2001 From: Eddie Torres Date: Mon, 11 Dec 2023 18:10:58 +0000 Subject: [PATCH] Revert #1843 - Remove block-device-mapping from attach limit Signed-off-by: Eddie Torres --- pkg/cloud/metadata.go | 17 +++++++++++------ pkg/cloud/metadata_ec2.go | 20 +++++++++++++++----- pkg/cloud/metadata_interface.go | 1 + pkg/cloud/metadata_k8s.go | 11 ++++++----- pkg/cloud/metadata_test.go | 25 ++++++++++++++++++++++--- pkg/cloud/mock_metadata.go | 14 ++++++++++++++ pkg/driver/node.go | 3 ++- 7 files changed, 71 insertions(+), 20 deletions(-) diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata.go index b3d9110193..f289ade496 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata.go @@ -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 ( @@ -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 diff --git a/pkg/cloud/metadata_ec2.go b/pkg/cloud/metadata_ec2.go index 509e65e394..d3841997d8 100644 --- a/pkg/cloud/metadata_ec2.go +++ b/pkg/cloud/metadata_ec2.go @@ -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) diff --git a/pkg/cloud/metadata_interface.go b/pkg/cloud/metadata_interface.go index 8e0d1d7b79..f98c22a832 100644 --- a/pkg/cloud/metadata_interface.go +++ b/pkg/cloud/metadata_interface.go @@ -12,6 +12,7 @@ type MetadataService interface { GetRegion() string GetAvailabilityZone() string GetNumAttachedENIs() int + GetNumBlockDeviceMappings() int GetOutpostArn() arn.ARN } diff --git a/pkg/cloud/metadata_k8s.go b/pkg/cloud/metadata_k8s.go index 4c610265ee..1ba84a0990 100644 --- a/pkg/cloud/metadata_k8s.go +++ b/pkg/cloud/metadata_k8s.go @@ -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 diff --git a/pkg/cloud/metadata_test.go b/pkg/cloud/metadata_test.go index 770395fcfa..16e4dccf37 100644 --- a/pkg/cloud/metadata_test.go +++ b/pkg/cloud/metadata_test.go @@ -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 @@ -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, @@ -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, }, } @@ -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() }) diff --git a/pkg/cloud/mock_metadata.go b/pkg/cloud/mock_metadata.go index fc61fbc6e5..7eab9d0998 100644 --- a/pkg/cloud/mock_metadata.go +++ b/pkg/cloud/mock_metadata.go @@ -91,6 +91,20 @@ func (mr *MockMetadataServiceMockRecorder) GetNumAttachedENIs() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNumAttachedENIs", reflect.TypeOf((*MockMetadataService)(nil).GetNumAttachedENIs)) } +// GetNumBlockDeviceMappings mocks base method. +func (m *MockMetadataService) GetNumBlockDeviceMappings() int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNumBlockDeviceMappings") + ret0, _ := ret[0].(int) + return ret0 +} + +// GetNumBlockDeviceMappings indicates an expected call of GetNumBlockDeviceMappings. +func (mr *MockMetadataServiceMockRecorder) GetNumBlockDeviceMappings() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNumBlockDeviceMappings", reflect.TypeOf((*MockMetadataService)(nil).GetNumBlockDeviceMappings)) +} + // GetOutpostArn mocks base method. func (m *MockMetadataService) GetOutpostArn() arn.ARN { m.ctrl.T.Helper() diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 12904f4fc3..68d9b84523 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -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 @@ -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 }