From 5cbfb803c25b4058d8d0feb5b60fbfa139f116c8 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Mon, 30 Dec 2024 13:22:59 +0100 Subject: [PATCH 1/2] enable dual stack support on aws --- .../lib/cloud/aws/instance_manager.rb | 2 +- .../lib/cloud/aws/instance_param_mapper.rb | 75 +++++++++++++++---- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/src/bosh_aws_cpi/lib/cloud/aws/instance_manager.rb b/src/bosh_aws_cpi/lib/cloud/aws/instance_manager.rb index e8c58145..df82f7e8 100644 --- a/src/bosh_aws_cpi/lib/cloud/aws/instance_manager.rb +++ b/src/bosh_aws_cpi/lib/cloud/aws/instance_manager.rb @@ -12,7 +12,7 @@ def initialize(ec2, logger) @imds_v2_enable = {} security_group_mapper = SecurityGroupMapper.new(@ec2) - @param_mapper = InstanceParamMapper.new(security_group_mapper) + @param_mapper = InstanceParamMapper.new(security_group_mapper, logger) end def create(stemcell_id, vm_cloud_props, networks_cloud_props, disk_locality, default_security_groups, block_device_mappings, user_data, tags, metadata_options) diff --git a/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb b/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb index 0c25179c..0651d24f 100644 --- a/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb +++ b/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb @@ -4,8 +4,9 @@ module Bosh::AwsCloud class InstanceParamMapper attr_accessor :manifest_params - def initialize(security_group_mapper) + def initialize(security_group_mapper, logger) @manifest_params = {} + @logger = logger @security_group_mapper = security_group_mapper end @@ -90,14 +91,36 @@ def instance_params nic = {} nic[:groups] = sg unless sg.nil? || sg.empty? - nic[:subnet_id] = subnet_id if subnet_id - - # only supporting one ip address for now (either ipv4 or ipv6) - if private_ip_address - if ipv6_address?(private_ip_address) - nic[:ipv_6_addresses] = [{ipv_6_address: private_ip_address}] + nic[:subnet_id] = subnet_id + + if dual_stack_mode? + ipv6_count = ipv4_count = 0 + private_ip_addresses.each do |private_ip| + if ipv6_address?(private_ip) + nic[:ipv_6_addresses] = [{ipv_6_address: private_ip}] + ipv6_count += 1 + else + nic[:private_ip_address] = private_ip + ipv4_count += 1 + end + end + if ipv4_count == 1 && ipv6_count == 1 + @logger.info("Running in dual stack mode") else - nic[:private_ip_address] = private_ip_address + if ipv4_count == 0 + raise Bosh::Clouds::CloudError, "Dual Stack Mode: No ipv4 address assigned" + elsif ipv6_count == 0 + raise Bosh::Clouds::CloudError, "Dual Stack Mode: No ipv6 address assigned" + end + end + else + private_ip_address = private_ip_addresses.first + if private_ip_address + if ipv6_address?(private_ip_address) + nic[:ipv_6_addresses] = [{ipv_6_address: private_ip_address}] + else + nic[:private_ip_address] = private_ip_address + end end end @@ -147,19 +170,41 @@ def ipv6_address?(addr) addr.to_s.include?(':') end - def private_ip_address - first_manual_network = networks_cloud_props.filter('manual').first - first_manual_network.ip unless first_manual_network.nil? + def private_ip_addresses + ips = subnets.map { |subnet_network| subnet_network.ip } + + ips unless ips.nil? end # NOTE: do NOT lookup the subnet (from EC2 client) anymore. We just need to # pass along the subnet_id anyway, and we have that. - def subnet_id - subnet_network_spec = networks_cloud_props.filter('manual', 'dynamic').reject do |net| + def subnets + subnet_network_specs = networks_cloud_props.filter('manual', 'dynamic').reject do |net| net.subnet.nil? - end.first + end + + subnet_network_specs unless subnet_network_specs.nil? + end - subnet_network_spec.subnet unless subnet_network_spec.nil? + def subnet_id + subnet_ids.first + end + + def subnet_ids + subnets.map { |subnet_network| subnet_network.subnet } + end + + def dual_stack_mode? + dual_stack_mode = false + if subnets.length == 2 + # if two networks refer to the same subnet we assume dual stack mode + if subnet_ids[0] == subnet_ids[1] + dual_stack_mode = true + else + raise Bosh::Clouds::CloudError, "Dual Stack Mode: Subnet #{subnet_ids[0]} and subnet #{subnet_ids[1]} are different" + end + end + dual_stack_mode end def availability_zone From 463ff5e08924bc9667a7d911d4fdbfa0006ed688 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Wed, 8 Jan 2025 13:32:44 +0100 Subject: [PATCH 2/2] create tests --- .../lib/cloud/aws/instance_param_mapper.rb | 11 +- .../spec/unit/instance_param_mapper_spec.rb | 251 ++++++++++-------- 2 files changed, 148 insertions(+), 114 deletions(-) diff --git a/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb b/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb index 0651d24f..2929de0d 100644 --- a/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb +++ b/src/bosh_aws_cpi/lib/cloud/aws/instance_param_mapper.rb @@ -91,7 +91,7 @@ def instance_params nic = {} nic[:groups] = sg unless sg.nil? || sg.empty? - nic[:subnet_id] = subnet_id + nic[:subnet_id] = subnet_id if subnet_id if dual_stack_mode? ipv6_count = ipv4_count = 0 @@ -171,7 +171,8 @@ def ipv6_address?(addr) end def private_ip_addresses - ips = subnets.map { |subnet_network| subnet_network.ip } + manual_subnets = subnets.select {|subnet| subnet.type == 'manual' } + ips = manual_subnets.map { |subnet_network| subnet_network.ip } ips unless ips.nil? end @@ -196,12 +197,10 @@ def subnet_ids def dual_stack_mode? dual_stack_mode = false - if subnets.length == 2 - # if two networks refer to the same subnet we assume dual stack mode + if subnets.length == 2 && subnets.find {|subnet| subnet.type == 'dynamic'}.nil? + # if two manual networks refer to the same subnet we assume dual stack mode if subnet_ids[0] == subnet_ids[1] dual_stack_mode = true - else - raise Bosh::Clouds::CloudError, "Dual Stack Mode: Subnet #{subnet_ids[0]} and subnet #{subnet_ids[1]} are different" end end dual_stack_mode diff --git a/src/bosh_aws_cpi/spec/unit/instance_param_mapper_spec.rb b/src/bosh_aws_cpi/spec/unit/instance_param_mapper_spec.rb index 0221abd7..f00f707e 100644 --- a/src/bosh_aws_cpi/spec/unit/instance_param_mapper_spec.rb +++ b/src/bosh_aws_cpi/spec/unit/instance_param_mapper_spec.rb @@ -3,7 +3,8 @@ module Bosh::AwsCloud describe InstanceParamMapper do - let(:instance_param_mapper) { InstanceParamMapper.new(security_group_mapper) } + let(:logger) { Logger.new('/dev/null') } + let(:instance_param_mapper) { InstanceParamMapper.new(security_group_mapper, logger) } let(:user_data) { {} } let(:security_group_mapper) { SecurityGroupMapper.new(ec2_resource) } let(:ec2_resource) { instance_double(Aws::EC2::Resource) } @@ -266,13 +267,15 @@ module Bosh::AwsCloud 'cloud_properties' => { 'security_groups' => ['sg-11111111', 'sg-2-name'], 'subnet' => dynamic_subnet_id - } + }, + 'type' => 'dynamic' }, 'net2' => { 'cloud_properties' => { 'security_groups' => 'sg-33333333', 'subnet' => dynamic_subnet_id - } + }, + 'type' => 'dynamic' } } end @@ -306,13 +309,15 @@ module Bosh::AwsCloud 'cloud_properties' => { 'security_groups' => ['sg-33333333', 'sg-4-name'], 'subnet' => dynamic_subnet_id - } + }, + 'type' => 'dynamic' }, 'net2' => { 'cloud_properties' => { 'security_groups' => 'sg-55555555', 'subnet' => dynamic_subnet_id - } + }, + 'type' => 'dynamic' } } end @@ -378,7 +383,7 @@ module Bosh::AwsCloud end end - context 'when IPv6 network address is present' do + context 'when no meaningful network is given' do let(:user_data) { Base64.encode64("{'networks' => #{agent_network_spec(network_cloud_props)}}".to_json).strip } let(:networks_spec) do { @@ -400,180 +405,200 @@ module Bosh::AwsCloud } end - it 'maps to Base64 encoded user_data.networks' do + it 'does not attach a network interface' do expect(mapping(input)).to eq(output) end end describe 'IP address options' do - context 'when an IP address is provided for explicitly specified manual networks in networks_spec' do - let(:networks_spec) do - { - 'net1' => { - 'type' => 'dynamic' - }, - 'net2' => { - 'type' => 'manual', - 'ip' => '1.1.1.1' - }, - 'net3' => { - 'type' => 'manual', - 'ip' => '2.2.2.2' - } - } - end + context 'when associate_public_ip_address is true' do + let(:vm_type) { { 'auto_assign_public_ip' => true } } let(:input) do { vm_type: vm_cloud_props, - networks_spec: network_cloud_props + networks_spec: network_cloud_props, } end let(:output) do { network_interfaces: [ { - private_ip_address: '1.1.1.1', + associate_public_ip_address: true, device_index: 0 } ] } end - it 'maps the first (explicit) manual network IP address to private_ip_address' do + it 'adds the option to the output' do + expect(mapping(input)).to eq(output) + end + end + + context 'when associate_public_ip_address is false' do + let(:vm_type) { { 'auto_assign_public_ip' => false } } + let(:input) do + { + vm_type: vm_cloud_props, + networks_spec: network_cloud_props, + } + end + let(:output) do + { + network_interfaces: [ + { + associate_public_ip_address: false, + device_index: 0 + } + ] + } + end + + it 'adds the option to the output' do expect(mapping(input)).to eq(output) end end + end - context 'when an IP address is provided for implicitly specified manual networks in networks_spec' do + describe 'Subnet options' do + context 'when subnet is provided by manual (explicit or implicit)' do let(:networks_spec) do { 'net1' => { - 'type' => 'dynamic' + 'type' => 'vip', + 'cloud_properties' => { 'subnet' => 'vip-subnet' } }, 'net2' => { - 'ip' => '1.1.1.1' - }, - 'net3' => { - 'type' => 'manual', - 'ip' => '2.2.2.2' + 'cloud_properties' => { 'subnet' => manual_subnet_id } } } end let(:input) do { vm_type: vm_cloud_props, - networks_spec: network_cloud_props + networks_spec: network_cloud_props, + subnet_az_mapping: { + manual_subnet_id => 'region-1b' + } } end let(:output) do { network_interfaces: [ { - private_ip_address: '1.1.1.1', + subnet_id: manual_subnet_id, device_index: 0 } - ] + ], + placement: { availability_zone: 'region-1b' } } end - it 'maps the first (implicit) manual network IP address to private_ip_address' do + it 'maps subnet from the first matching network to subnet_id' do expect(mapping(input)).to eq(output) end end - context 'when associate_public_ip_address is true' do - let(:vm_type) { { 'auto_assign_public_ip' => true } } + context 'when subnet is provided by manual (explicit or implicit) or dynamic networks in networks_spec' do + let(:networks_spec) do + { + 'net1' => { + 'type' => 'dynamic', + 'cloud_properties' => { 'subnet' => dynamic_subnet_id } + }, + 'net2' => { + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id } + } + } + end let(:input) do { vm_type: vm_cloud_props, networks_spec: network_cloud_props, + subnet_az_mapping: { + dynamic_subnet_id => 'region-1a', + manual_subnet_id => 'region-1b' + } } end let(:output) do { + placement: { + availability_zone: 'region-1a' + }, network_interfaces: [ { - associate_public_ip_address: true, + subnet_id: dynamic_subnet_id, device_index: 0 } ] } end - it 'adds the option to the output' do - expect(mapping(input)).to eq(output) - end - end - - context 'when associate_public_ip_address is false' do - let(:vm_type) { { 'auto_assign_public_ip' => false } } - let(:input) do - { - vm_type: vm_cloud_props, - networks_spec: network_cloud_props, - } - end - let(:output) do - { - network_interfaces: [ - { - associate_public_ip_address: false, - device_index: 0 - } - ] - } - end - - it 'adds the option to the output' do + it 'maps subnet from the first matching network to subnet_id' do expect(mapping(input)).to eq(output) end end - context 'when the one manual network address is IPv6' do - let(:user_data) { Base64.encode64("{'networks' => #{agent_network_spec(network_cloud_props)}}".to_json).strip } + context 'when two manual subnets with the same subnet_id are provided' do let(:networks_spec) do { 'net1' => { - 'type' => 'dynamic' + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '1.1.1.1' }, 'net2' => { 'type' => 'manual', - 'ip' => '2006::1' + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '2600::1' } } end - let(:input) do { + let(:input) do + { vm_type: vm_cloud_props, networks_spec: network_cloud_props, - user_data: user_data } end let(:output) do { - network_interfaces: [{ - ipv_6_addresses: [{ipv_6_address: '2006::1' }], + network_interfaces: [ + { + subnet_id: manual_subnet_id, + ipv_6_addresses: [{ + ipv_6_address: '2600::1', + }, + ], + private_ip_address: '1.1.1.1', device_index: 0 - }], - user_data: user_data + } + ] } end - it 'maps the first (explicit) manual network IP address to ipv_6_addresses' do + it 'attaches an ipv4 and an ipv6 to the nic' do + allow(logger).to receive(:info) + expect(mapping(input)).to eq(output) - end + + expect(logger).to have_received(:info).with("Running in dual stack mode") end end - end - describe 'Subnet options' do - context 'when subnet is provided by manual (explicit or implicit)' do + + context 'when two manual subnets with different subnet_ids are provided' do let(:networks_spec) do { 'net1' => { - 'type' => 'vip', - 'cloud_properties' => { 'subnet' => 'vip-subnet' } + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '1.1.1.1' }, 'net2' => { - 'cloud_properties' => { 'subnet' => manual_subnet_id } + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => 'different-subnet' }, + 'ip' => '2600::1' } } end @@ -581,9 +606,6 @@ module Bosh::AwsCloud { vm_type: vm_cloud_props, networks_spec: network_cloud_props, - subnet_az_mapping: { - manual_subnet_id => 'region-1b' - } } end let(:output) do @@ -591,28 +613,30 @@ module Bosh::AwsCloud network_interfaces: [ { subnet_id: manual_subnet_id, + private_ip_address: '1.1.1.1', device_index: 0 } - ], - placement: { availability_zone: 'region-1b' } + ] } end - it 'maps subnet from the first matching network to subnet_id' do + it 'attaches an ipv4 and an ipv6 to the nic' do expect(mapping(input)).to eq(output) end end - context 'when subnet is provided by manual (explicit or implicit) or dynamic networks in networks_spec' do + context 'when two manual subnets with different subnet_ids are provided' do let(:networks_spec) do { 'net1' => { - 'type' => 'dynamic', - 'cloud_properties' => { 'subnet' => dynamic_subnet_id } + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '1.1.1.1' }, 'net2' => { 'type' => 'manual', - 'cloud_properties' => { 'subnet' => manual_subnet_id } + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '2.2.2.2' } } end @@ -620,28 +644,39 @@ module Bosh::AwsCloud { vm_type: vm_cloud_props, networks_spec: network_cloud_props, - subnet_az_mapping: { - dynamic_subnet_id => 'region-1a', - manual_subnet_id => 'region-1b' - } } end - let(:output) do + + it 'raises an error message if no ipv6 address is provided' do + expect{ mapping(input)}.to raise_error Bosh::Clouds::CloudError, /Dual Stack Mode: No ipv6 address assigned/ + end + end + + + context 'when two manual subnets with different subnet_ids are provided' do + let(:networks_spec) do { - placement: { - availability_zone: 'region-1a' + 'net1' => { + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '2600::1' }, - network_interfaces: [ - { - subnet_id: dynamic_subnet_id, - device_index: 0 - } - ] + 'net2' => { + 'type' => 'manual', + 'cloud_properties' => { 'subnet' => manual_subnet_id }, + 'ip' => '2600::2' + } + } + end + let(:input) do + { + vm_type: vm_cloud_props, + networks_spec: network_cloud_props, } end - it 'maps subnet from the first matching network to subnet_id' do - expect(mapping(input)).to eq(output) + it 'raises an error message if no ipv6 address is applied' do + expect{ mapping(input)}.to raise_error Bosh::Clouds::CloudError, /Dual Stack Mode: No ipv4 address assigned/ end end end