Fix Elastic IP association for instances with multiple network interfaces

src/bosh_aws_cpi/lib/cloud/aws/cloud_props.rb

@@ -311,12 +311,13 @@ class DynamicNetwork < Network
     end
 
     class PublicNetwork < Network
-      attr_reader :ip
+      attr_reader :ip, :nic_group
 
       def initialize(name, settings)
         super(name, settings)
 
         @ip = settings['ip']
+        @nic_group = settings['nic_group']
       end
     end
   end

src/bosh_aws_cpi/lib/cloud/aws/network_configurator.rb

@@ -2,23 +2,17 @@
 
 module Bosh::AwsCloud
   ##
-  # Represents AWS instance network config. EC2 instance has single NIC
-  # with dynamic IP address and (optionally) a single elastic IP address
-  # which instance itself is not aware of (vip). Thus we should perform
-  # a number of sanity checks for the network spec provided by director
-  # to make sure we don't apply something EC2 doesn't understand how to
-  # deal with.
+  # Represents AWS instance network configuration.
+  # Handles Elastic IP (VIP) association to network interfaces.
   #
   class NetworkConfigurator
     include Helpers
 
-    ##
-    # Creates new network spec
-    #
-    # @param [Hash] spec raw network spec passed by director
+    # @param [NetworkCloudProps] network_cloud_props parsed network configuration
     def initialize(network_cloud_props)
       @logger = Bosh::Clouds::Config.logger
       @vip_network = nil
+      @network_cloud_props = network_cloud_props
 
       network_cloud_props.networks.each do |net|
         if net.instance_of?(Bosh::AwsCloud::NetworkCloudProps::PublicNetwork)
@@ -54,16 +48,23 @@ def configure_vip(ec2, instance)
         cloud_error("No IP provided for vip network '#{@vip_network.name}'")
       end
 
-      # AWS accounts that support both EC2-VPC and EC2-Classic platform access explicitly require allocation_id instead of public_ip
-      addresses = ec2.client.describe_addresses(
-        public_ips: [@vip_network.ip],
-        filters: [
-          name: 'domain',
-          values: [
-            'vpc'
+      describe_address_errors = [
+        Aws::EC2::Errors::ServiceError,
+        Aws::EC2::Errors::RequestLimitExceeded
+      ]
+
+      addresses = Bosh::Common.retryable(tries: 10, sleep: 1, on: describe_address_errors) do
+        ec2.client.describe_addresses(
+          public_ips: [@vip_network.ip],
+          filters: [
+            name: 'domain',
+            values: [
+              'vpc'
+            ]
           ]
-        ]
-      ).addresses
+        ).addresses
+      end
+
       found_address = addresses.first
       cloud_error("Elastic IP with VPC scope not found with address '#{@vip_network.ip}'") if found_address.nil?
 
@@ -72,16 +73,65 @@ def configure_vip(ec2, instance)
       @logger.info("Associating instance `#{instance.id}' " \
                    "with elastic IP `#{@vip_network.ip}' and allocation_id '#{allocation_id}'")
 
-      # New elastic IP reservation supposed to clear the old one,
-      # so no need to disassociate manually. Also, we don't check
-      # if this IP is actually an allocated EC2 elastic IP, as
-      # API call will fail in that case.
+      describe_errors = [
+        Aws::EC2::Errors::ServiceError,
+        Aws::EC2::Errors::RequestLimitExceeded,
+        Aws::EC2::Errors::InvalidInstanceID
+      ]
+
+      network_interfaces = Bosh::Common.retryable(tries: 10, sleep: 1, on: describe_errors) do
+        response = ec2.client.describe_instances(instance_ids: [instance.id])
+        if response.reservations.empty? || response.reservations.first.instances.empty?
+          raise Aws::EC2::Errors::InvalidInstanceID.new(nil, "Instance '#{instance.id}' not found in describe_instances response")
+        end
+        response.reservations.first.instances.first.network_interfaces
+      end
+
+      if network_interfaces.nil? || network_interfaces.empty?
+        cloud_error("No network interfaces found for instance '#{instance.id}'. " \
+                    "Instance may not be fully initialized.")
+      end
+
+      target_device_index = determine_nic_index
+
+      target_nic = network_interfaces.find { |nic| nic.attachment.device_index == target_device_index }
+
+      if target_nic.nil?
+        nic_indexes = network_interfaces.map { |nic| nic.attachment.device_index }.sort.join(', ')
+
+        nic_group_info = @vip_network.nic_group ? "nic_group '#{@vip_network.nic_group}'" : "default (nic_group not specified)"
+        cloud_error("Could not find network interface with device_index #{target_device_index} " \
+                    "(#{nic_group_info}) on instance '#{instance.id}'. " \
+                    "Found network interfaces with device indexes: #{nic_indexes}")
+      end
+
+      nic_group_info = @vip_network.nic_group ? "nic_group '#{@vip_network.nic_group}'" : "default (nic_group not specified)"
+      @logger.info("Associating elastic IP with network interface '#{target_nic.network_interface_id}' " \
+                   "(device_index #{target_device_index}, #{nic_group_info}) on instance '#{instance.id}'")
 
       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)
-        true # need to return true to end the retries
+        ec2.client.associate_address(
+          network_interface_id: target_nic.network_interface_id,
+          allocation_id: allocation_id
+        )
+        true
       end
     end
+
+    # Maps VIP network's nic_group to the corresponding device_index
+    # Returns 0 (primary NIC) if nic_group is not specified or not found
+    def determine_nic_index
+      return 0 unless @vip_network.nic_group
+
+      nic_groups = @network_cloud_props.networks
+        .select { |net| (net.type == 'manual' || net.type == 'dynamic') && net.respond_to?(:nic_group) && net.nic_group }
+        .map { |net| net.nic_group }
+        .uniq
+
+      device_index = nic_groups.index(@vip_network.nic_group)
+
+      device_index || 0
+    end
   end
-end
+end

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

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)
+  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])
+  allow(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')