diff --git a/src/bosh_aws_cpi/lib/cloud/aws/network_configurator.rb b/src/bosh_aws_cpi/lib/cloud/aws/network_configurator.rb index b1a10b9b..aa9fe498 100644 --- a/src/bosh_aws_cpi/lib/cloud/aws/network_configurator.rb +++ b/src/bosh_aws_cpi/lib/cloud/aws/network_configurator.rb @@ -77,9 +77,20 @@ def configure_vip(ec2, instance) # if this IP is actually an allocated EC2 elastic IP, as # API call will fail in that case. + # AWS requires network_interface_id when the instance has multiple network interfaces. + network_interfaces = ec2.client.describe_instances(instance_ids: [instance.id]).reservations.first.instances.first.network_interfaces + primary_nic = network_interfaces.find { |nic| nic.attachment.device_index == 0 } + + if primary_nic.nil? + cloud_error("Could not find primary network interface for instance '#{instance.id}'") + end + errors = [Aws::EC2::Errors::IncorrectInstanceState, Aws::EC2::Errors::InvalidInstanceID] Bosh::Common.retryable(tries: 10, sleep: 1, on: errors) do - ec2.client.associate_address(instance_id: instance.id, allocation_id: allocation_id) + ec2.client.associate_address( + network_interface_id: primary_nic.network_interface_id, + allocation_id: allocation_id + ) true # need to return true to end the retries end end diff --git a/src/bosh_aws_cpi/spec/integration/manual_network_spec.rb b/src/bosh_aws_cpi/spec/integration/manual_network_spec.rb index 29bd70f7..f3fb55b5 100644 --- a/src/bosh_aws_cpi/spec/integration/manual_network_spec.rb +++ b/src/bosh_aws_cpi/spec/integration/manual_network_spec.rb @@ -1020,4 +1020,74 @@ def create_vm(*args) vm_id end end + + context 'with multiple network interfaces and VIP' do + let(:network_spec) do + { + 'default' => { + 'type' => 'manual', + 'ip' => @manual_ip, + 'cloud_properties' => { + 'subnet' => @manual_subnet_id, + 'nic_group' => 0 + } + }, + 'secondary' => { + 'type' => 'manual', + 'ip' => @secondary_manual_ip, + 'cloud_properties' => { + 'subnet' => @manual_subnet_id, + 'nic_group' => 1 + } + }, + 'elastic' => { + 'type' => 'vip', + 'ip' => eip + } + } + end + + before(:each) do + @ip_semaphore.synchronize do + secondary_cidr = @ec2.subnet(@manual_subnet_id).cidr_block + secondary_ips = IPAddr.new(secondary_cidr).to_range.to_a.map(&:to_s) + ip_addresses = secondary_ips.first(secondary_ips.size - 1).drop(9) + ip_addresses -= @already_used + @secondary_manual_ip = ip_addresses[rand(ip_addresses.size)] + @already_used << @secondary_manual_ip + end + end + + it 'creates VM with multiple NICs and associates Elastic IP to primary NIC' do + vm_lifecycle do |instance_id| + resp = @cpi.ec2_resource.client.describe_instances(filters: [{ name: 'instance-id', values: [instance_id] }]) + instance_data = resp.reservations[0].instances[0] + network_interfaces = instance_data.network_interfaces + + expect(network_interfaces.length).to eq(2), "Expected 2 network interfaces, got #{network_interfaces.length}" + + primary_nic = network_interfaces.find { |nic| nic.attachment.device_index == 0 } + secondary_nic = network_interfaces.find { |nic| nic.attachment.device_index == 1 } + + expect(primary_nic).not_to be_nil, "Primary NIC (device_index 0) not found" + expect(secondary_nic).not_to be_nil, "Secondary NIC (device_index 1) not found" + + addresses_resp = @cpi.ec2_resource.client.describe_addresses( + filters: [{ name: 'public-ip', values: [eip] }] + ) + + expect(addresses_resp.addresses.length).to eq(1), "Elastic IP #{eip} not found" + address = addresses_resp.addresses[0] + + # EIP must be associated via network_interface_id (not instance_id) + # when multiple NICs are present, and it must be the primary NIC + expect(address.network_interface_id).to eq(primary_nic.network_interface_id), + "Elastic IP should be associated with primary NIC #{primary_nic.network_interface_id}, " \ + "but is associated with #{address.network_interface_id}" + + expect(primary_nic.association).not_to be_nil, "No public IP association found on primary NIC" + expect(primary_nic.association.public_ip).to eq(eip), "Expected Elastic IP #{eip} on primary NIC" + end + end + end end diff --git a/src/bosh_aws_cpi/spec/spec_helper.rb b/src/bosh_aws_cpi/spec/spec_helper.rb index ff7f33d2..d851fa5f 100644 --- a/src/bosh_aws_cpi/spec/spec_helper.rb +++ b/src/bosh_aws_cpi/spec/spec_helper.rb @@ -133,6 +133,28 @@ def asset(filename) File.expand_path(File.join(File.dirname(__FILE__), 'assets', filename)) end +def create_nic_mock(device_index, eni_id) + nic = instance_double(Aws::EC2::Types::InstanceNetworkInterface) + attachment = instance_double(Aws::EC2::Types::InstanceNetworkInterfaceAttachment) + allow(attachment).to receive(:device_index).and_return(device_index) + allow(nic).to receive(:attachment).and_return(attachment) + allow(nic).to receive(:network_interface_id).and_return(eni_id) if device_index == 0 + nic +end + +def mock_describe_instances(ec2_client, instance_id, nics) + instance_data = instance_double(Aws::EC2::Types::Instance, network_interfaces: nics) + reservation = instance_double(Aws::EC2::Types::Reservation, instances: [instance_data]) + response = instance_double(Aws::EC2::Types::DescribeInstancesResult, reservations: [reservation]) + expect(ec2_client).to receive(:describe_instances).with(instance_ids: [instance_id]).and_return(response) +end + +def setup_vip_mocks(ec2_client, elastic_ip, describe_addresses_arguments, describe_addresses_response, allocation_id: 'allocation-id') + expect(ec2_client).to receive(:describe_addresses) + .with(describe_addresses_arguments).and_return(describe_addresses_response) + expect(elastic_ip).to receive(:allocation_id).and_return(allocation_id) +end + RSpec.configure do |config| config.before do logger = Bosh::Cpi::Logger.new('/dev/null') diff --git a/src/bosh_aws_cpi/spec/unit/network_configurator_spec.rb b/src/bosh_aws_cpi/spec/unit/network_configurator_spec.rb index fa3bee3e..6c7f0b02 100644 --- a/src/bosh_aws_cpi/spec/unit/network_configurator_spec.rb +++ b/src/bosh_aws_cpi/spec/unit/network_configurator_spec.rb @@ -138,11 +138,13 @@ def set_security_groups(spec, security_groups) let(:response_addresses) { [elastic_ip] } it 'should associate Elastic/Public IP to the instance' do - expect(ec2_client).to receive(:describe_addresses) - .with(describe_addresses_arguments).and_return(describe_addresses_response) - expect(elastic_ip).to receive(:allocation_id).and_return('allocation-id') + primary_nic = create_nic_mock(0, 'eni-12345678') + + setup_vip_mocks(ec2_client, elastic_ip, describe_addresses_arguments, describe_addresses_response) + mock_describe_instances(ec2_client, 'i-xxxxxxxx', [primary_nic]) + expect(ec2_client).to receive(:associate_address).with( - instance_id: 'i-xxxxxxxx', + network_interface_id: 'eni-12345678', allocation_id: 'allocation-id' ) @@ -162,6 +164,41 @@ def set_security_groups(spec, security_groups) }.to raise_error(/Elastic IP with VPC scope not found with address '#{vip_public_ip}'/) end end + + context 'with multiple network interfaces' do + let(:elastic_ip) { instance_double(Aws::EC2::Types::Address) } + let(:response_addresses) { [elastic_ip] } + + it 'should associate Elastic IP to primary NIC (device_index 0)' do + primary_nic = create_nic_mock(0, 'eni-primary') + secondary_nic = create_nic_mock(1, nil) + + setup_vip_mocks(ec2_client, elastic_ip, describe_addresses_arguments, describe_addresses_response) + mock_describe_instances(ec2_client, 'i-xxxxxxxx', [secondary_nic, primary_nic]) + + expect(ec2_client).to receive(:associate_address).with( + network_interface_id: 'eni-primary', + allocation_id: 'allocation-id' + ) + + Bosh::AwsCloud::NetworkConfigurator.new(network_cloud_props).configure(ec2_resource, instance) + end + + it 'should handle multiple NICs in any order' do + primary_nic = create_nic_mock(0, 'eni-primary') + secondary_nic = create_nic_mock(1, nil) + + setup_vip_mocks(ec2_client, elastic_ip, describe_addresses_arguments, describe_addresses_response) + mock_describe_instances(ec2_client, 'i-xxxxxxxx', [primary_nic, secondary_nic]) + + expect(ec2_client).to receive(:associate_address).with( + network_interface_id: 'eni-primary', + allocation_id: 'allocation-id' + ) + + Bosh::AwsCloud::NetworkConfigurator.new(network_cloud_props).configure(ec2_resource, instance) + end + end end end end