From b2e26e2b6c36c263a2a891bd462ee02fed85702c Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Mon, 14 Apr 2025 17:05:30 +0200 Subject: [PATCH 01/17] introduce prefix property for networks --- .../ip_provider/ip_provider.rb | 5 ++ .../deployment_plan/ip_provider/ip_repo.rb | 48 +++++++++++++++++++ .../deployment_plan/manual_network_subnet.rb | 13 ++++- src/bosh-director/lib/bosh/director/errors.rb | 1 + .../lib/bosh/director/ip_addr_or_cidr.rb | 2 +- .../lib/bosh/director/network_reservation.rb | 9 +++- 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb index b4de47f1aad..172f15136bb 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb @@ -69,6 +69,11 @@ def reserve_manual(reservation) @logger.debug("Reserving dynamic IP '#{ip}' for manual network '#{reservation.network.name}'") reservation.resolve_ip(ip) reservation.resolve_type(:dynamic) + unless subnet.prefix.nil? + prefix = @ip_repo.allocate_dynamic_prefix(reservation, subnet) + @logger.debug("Reserving dynamic prefix '#{prefix}' for manual network '#{reservation.network.name}'") + reservation.resolve_prefix(prefix) + end break end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 8d37093a18a..30e2a820556 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -54,6 +54,23 @@ def allocate_dynamic_ip(reservation, subnet) ip_address.to_i end + def allocate_dynamic_prefix(reservation, subnet) + begin + cidr = try_to_allocate_dynamic_prefix(reservation, subnet) + rescue NoMoreIPsAvailableAndStopRetrying + @logger.debug('Failed to allocate dynamic prefix: no more available') + return nil + rescue IpFoundInDatabaseAndCanBeRetried + @logger.debug('Retrying to allocate dynamic prefix: probably a race condition with another deployment') + # IP can be taken by other deployment that runs in parallel + # retry until succeeds or out of range + retry + end + + @logger.debug("Allocated dynamic cidr '#{cidr}' for #{reservation.network.name}") + cidr.to_i + end + def allocate_vip_ip(reservation, subnet) begin ip_address = try_to_allocate_vip_ip(reservation, subnet) @@ -95,6 +112,33 @@ def try_to_allocate_dynamic_ip(reservation, subnet) ip_address end + def try_to_allocate_dynamic_prefix(reservation, subnet) + @logger.debug("Allocating dynamic prefix for #{reservation.network.name}") + addresses_in_use = Set.new(all_ip_addresses) + + first_range_address = subnet.range.to_range.first.to_i - 1 + addresses_we_cant_allocate = addresses_in_use + addresses_we_cant_allocate << first_range_address + + addresses_we_cant_allocate.merge(subnet.restricted_ips.to_a) unless subnet.restricted_ips.empty? + addresses_we_cant_allocate.merge(subnet.static_ips.to_a) unless subnet.static_ips.empty? + + addr = find_first_available_address(addresses_we_cant_allocate, first_range_address) + prefix = get_first_available_prefix(addr, subnet.prefix) + + @logger.debug("FIRST AVAILABLE PREFIX '#{prefix}' for #{reservation.network.name}") + + cidr = Bosh::Director::IpAddrOrCidr.new(prefix) + + unless subnet.range == cidr || subnet.range.include?(cidr) + raise NoMoreIPsAvailableAndStopRetrying + end + + save_ip(cidr, reservation, false) + + cidr + end + def find_first_available_address(addresses_we_cant_allocate, first_address) last_address_we_cant_use = addresses_we_cant_allocate .to_a @@ -104,6 +148,10 @@ def find_first_available_address(addresses_we_cant_allocate, first_address) last_address_we_cant_use + 1 end + def get_first_available_prefix(first_available_addr, prefix) + "#{first_available_addr}/#{prefix}" + end + def try_to_allocate_vip_ip(reservation, subnet) addresses_in_use = Set.new(all_ip_addresses) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb index 7d282155b07..36b20af1a61 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb @@ -5,7 +5,7 @@ class ManualNetworkSubnet < Subnet extend IpUtil attr_reader :network_name, :name, :dns, - :availability_zone_names, :netmask_bits + :availability_zone_names, :netmask_bits, :prefix attr_accessor :cloud_properties, :range, :gateway, :restricted_ips, :static_ips, :netmask @@ -17,6 +17,7 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) ignore_missing_gateway = Bosh::Director::Config.ignore_missing_gateway gateway_property = safe_property(subnet_spec, 'gateway', class: String, optional: ignore_missing_gateway || managed) reserved_property = safe_property(subnet_spec, 'reserved', optional: true) + prefix = safe_property(subnet_spec, 'prefix', optional: true) restricted_ips = Set.new static_ips = Set.new @@ -75,6 +76,12 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) static_ips.add(ip) end + + unless prefix.nil? + if range.prefix > prefix + raise NetworkPrefixSizeTooBig, "Prefix size '#{prefix}' is larger than range prefix '#{range.prefix}'" + end + end end name_server_parser = NetworkParser::NameServersParser.new @@ -95,10 +102,11 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) static_ips, sn_name, netmask_bits, + prefix ) end - def initialize(network_name, range, gateway, name_servers, cloud_properties, netmask, availability_zone_names, restricted_ips, static_ips, subnet_name = nil, netmask_bits = nil) + def initialize(network_name, range, gateway, name_servers, cloud_properties, netmask, availability_zone_names, restricted_ips, static_ips, subnet_name = nil, netmask_bits = nil, prefix) @network_name = network_name @name = subnet_name @netmask_bits = netmask_bits @@ -110,6 +118,7 @@ def initialize(network_name, range, gateway, name_servers, cloud_properties, net @availability_zone_names = availability_zone_names @restricted_ips = restricted_ips @static_ips = static_ips + @prefix = prefix end def overlaps?(subnet) diff --git a/src/bosh-director/lib/bosh/director/errors.rb b/src/bosh-director/lib/bosh/director/errors.rb index 00e2699d8ba..f51ba6446c4 100644 --- a/src/bosh-director/lib/bosh/director/errors.rb +++ b/src/bosh-director/lib/bosh/director/errors.rb @@ -224,6 +224,7 @@ def self.err(error_code, response_code = BAD_REQUEST) NetworkInvalidIpRangeFormat = err(160010) NetworkDeletingUnorphanedError = err(160011) NetworkNotFoundError = err(16012) + NetworkPrefixSizeTooBig = err(16013) # ResourcePool ResourcePoolUnknownNetwork = err(170001) diff --git a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb index 4a9f4154821..086e30d0b05 100644 --- a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb +++ b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb @@ -3,7 +3,7 @@ module Bosh module Director class IpAddrOrCidr - delegate :==, :include?, :ipv4?, :ipv6?, :netmask, :to_i, :to_range, :to_string, to: :@ipaddr + delegate :==, :include?, :ipv4?, :ipv6?, :netmask, :to_i, :to_range, :to_string, :prefix, to: :@ipaddr alias :to_s :to_string def initialize(ip_or_cidr) diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index f1f99702dda..77515d075ed 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -1,6 +1,6 @@ module Bosh::Director class NetworkReservation - attr_reader :ip, :instance_model, :network, :type + attr_reader :ip, :instance_model, :network, :type, :prefix def initialize(instance_model, network) @instance_model = instance_model @@ -26,9 +26,10 @@ def formatted_ip class ExistingNetworkReservation < NetworkReservation attr_reader :network_type, :obsolete - def initialize(instance_model, network, ip, network_type) + def initialize(instance_model, network, ip, network_type, prefix) super(instance_model, network) @ip = IpAddrOrCidr.new(ip).to_i if ip + @prefix = IpAddrOrCidr.new(prefix).to_i if prefix @network_type = network_type @obsolete = network.instance_of? Bosh::Director::DeploymentPlan::Network end @@ -65,6 +66,10 @@ def resolve_ip(ip) @ip = IpAddrOrCidr.new(ip).to_i end + def resolve_prefix(prefix) # prefix assignment is only dynamic, therefore initialization is not possible + @prefix = IpAddrOrCidr.new(prefix) + end + def resolve_type(type) if @type != type raise NetworkReservationWrongType, From 1e123e33590ddf91bd90888f3fbf592300159556 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Tue, 15 Apr 2025 09:03:07 +0200 Subject: [PATCH 02/17] create seperate reservation for prefix --- .../ip_provider/ip_provider.rb | 18 +++++++++-------- .../deployment_plan/ip_provider/ip_repo.rb | 12 +++++------ .../network_planner/planner.rb | 20 +++++++++++++++++-- .../availability_zone_picker.rb | 2 +- .../lib/bosh/director/network_reservation.rb | 17 +++++++--------- 5 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb index 172f15136bb..7f0fd0a5a14 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb @@ -65,22 +65,24 @@ def reserve_manual(reservation) @logger.debug("Allocating dynamic ip for manual network '#{reservation.network.name}'") filter_subnet_by_instance_az(reservation).each do |subnet| - if (ip = @ip_repo.allocate_dynamic_ip(reservation, subnet)) - @logger.debug("Reserving dynamic IP '#{ip}' for manual network '#{reservation.network.name}'") + ip = if reservation.is_prefix_reservation + @ip_repo.allocate_dynamic_prefix(reservation, subnet) + else + @ip_repo.allocate_dynamic_ip(reservation, subnet) + end + + @logger.debug("Reserving dynamic IP/prefix '#{ip}' for manual network '#{reservation.network.name}'") + + if ip reservation.resolve_ip(ip) reservation.resolve_type(:dynamic) - unless subnet.prefix.nil? - prefix = @ip_repo.allocate_dynamic_prefix(reservation, subnet) - @logger.debug("Reserving dynamic prefix '#{prefix}' for manual network '#{reservation.network.name}'") - reservation.resolve_prefix(prefix) - end break end end if reservation.ip.nil? raise NetworkReservationNotEnoughCapacity, - "Failed to reserve IP for '#{reservation.instance_model}' for manual network '#{reservation.network.name}': no more available" + "Failed to reserve IP/Prefix for '#{reservation.instance_model}' for manual network '#{reservation.network.name}': no more available" end else @logger.debug("Reserving #{reservation.desc} for manual network '#{reservation.network.name}'") diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 30e2a820556..a037ef58e06 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -123,12 +123,12 @@ def try_to_allocate_dynamic_prefix(reservation, subnet) addresses_we_cant_allocate.merge(subnet.restricted_ips.to_a) unless subnet.restricted_ips.empty? addresses_we_cant_allocate.merge(subnet.static_ips.to_a) unless subnet.static_ips.empty? - addr = find_first_available_address(addresses_we_cant_allocate, first_range_address) - prefix = get_first_available_prefix(addr, subnet.prefix) + addr = Bosh::Director::IpAddrOrCidr.new(find_first_available_address(addresses_we_cant_allocate, first_range_address)).to_s + cidr_string = get_first_available_prefix(addr, subnet.prefix) - @logger.debug("FIRST AVAILABLE PREFIX '#{prefix}' for #{reservation.network.name}") + @logger.debug("FIRST AVAILABLE PREFIX '#{cidr_string}' for #{reservation.network.name}") - cidr = Bosh::Director::IpAddrOrCidr.new(prefix) + cidr = Bosh::Director::IpAddrOrCidr.new(cidr_string) unless subnet.range == cidr || subnet.range.include?(cidr) raise NoMoreIPsAvailableAndStopRetrying @@ -149,7 +149,7 @@ def find_first_available_address(addresses_we_cant_allocate, first_address) end def get_first_available_prefix(first_available_addr, prefix) - "#{first_available_addr}/#{prefix}" + "#{first_available_addr}/#{prefix}" end def try_to_allocate_vip_ip(reservation, subnet) @@ -214,7 +214,7 @@ def save_ip(ip, reservation, is_static) rescue Sequel::ValidationFailed, Sequel::DatabaseError => e error_message = e.message.downcase if error_message.include?('unique') || error_message.include?('duplicate') - raise IpFoundInDatabaseAndCanBeRetried + raise IpFoundInDatabaseAndCanBeRetried, e.inspect else raise e end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb index c38834b0f5e..a40639fb367 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb @@ -6,15 +6,31 @@ def initialize(logger) end def network_plan_with_dynamic_reservation(instance_plan, job_network) + plans = [] reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network) @logger.debug("Creating new dynamic reservation #{reservation} for instance '#{instance_plan.instance}'") - Plan.new(reservation: reservation) + plans << Plan.new(reservation: reservation) + job_network.deployment_network.subnets.each do |subnet| + unless subnet.prefix.nil? + prefix_reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network, true) + plans << Plan.new(reservation: prefix_reservation) + end + end + plans end def network_plan_with_static_reservation(instance_plan, job_network, static_ip) + plans = [] reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_plan.instance.model, job_network.deployment_network, static_ip) @logger.debug("Creating new static reservation #{reservation} for instance '#{instance_plan.instance}'") - Plan.new(reservation: reservation) + plans << Plan.new(reservation: reservation) + job_network.deployment_network.subnets.each do |subnet| + unless subnet.prefix.nil? + prefix_reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network, true) + plans << Plan.new(reservation: prefix_reservation) + end + end + plans end end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb index 2c11a70b02c..632c3e59431 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb @@ -123,7 +123,7 @@ def desired_new_instance_plans(new_desired_instances) def populate_network_plans(instance_plan) @networks.each do |network| - instance_plan.network_plans << @network_planner.network_plan_with_dynamic_reservation(instance_plan, network) + @network_planner.network_plan_with_dynamic_reservation(instance_plan, network).each {|plan| instance_plan.network_plans << plan } end end diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 77515d075ed..7506841e53a 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -1,6 +1,6 @@ module Bosh::Director class NetworkReservation - attr_reader :ip, :instance_model, :network, :type, :prefix + attr_reader :ip, :instance_model, :network, :type, :is_prefix_reservation def initialize(instance_model, network) @instance_model = instance_model @@ -26,10 +26,10 @@ def formatted_ip class ExistingNetworkReservation < NetworkReservation attr_reader :network_type, :obsolete - def initialize(instance_model, network, ip, network_type, prefix) + def initialize(instance_model, network, ip, network_type) super(instance_model, network) @ip = IpAddrOrCidr.new(ip).to_i if ip - @prefix = IpAddrOrCidr.new(prefix).to_i if prefix + @is_prefix_reservation = IpAddrOrCidr.new(ip).prefix != 32 @network_type = network_type @obsolete = network.instance_of? Bosh::Director::DeploymentPlan::Network end @@ -48,28 +48,25 @@ def to_s end class DesiredNetworkReservation < NetworkReservation - def self.new_dynamic(instance_model, network) - new(instance_model, network, nil, :dynamic) + def self.new_dynamic(instance_model, network, prefix_reservation = false) + new(instance_model, network, nil, :dynamic, prefix_reservation) end def self.new_static(instance_model, network, ip) new(instance_model, network, ip, :static) end - def initialize(instance_model, network, ip, type) + def initialize(instance_model, network, ip, type, prefix_reservation) super(instance_model, network) @ip = IpAddrOrCidr.new(ip).to_i if ip @type = type + @is_prefix_reservation = prefix_reservation end def resolve_ip(ip) @ip = IpAddrOrCidr.new(ip).to_i end - def resolve_prefix(prefix) # prefix assignment is only dynamic, therefore initialization is not possible - @prefix = IpAddrOrCidr.new(prefix) - end - def resolve_type(type) if @type != type raise NetworkReservationWrongType, From 7cdb7042d8e855d4bc13b17977defb820fb273b3 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Tue, 15 Apr 2025 19:46:19 +0200 Subject: [PATCH 03/17] introduce new prefix column in ip addresses table --- .../deployment_plan/dynamic_network.rb | 2 +- .../ip_provider/ip_provider.rb | 18 +- .../deployment_plan/ip_provider/ip_repo.rb | 161 ++++++++++-------- .../director/deployment_plan/job_network.rb | 4 +- .../deployment_plan/manual_network.rb | 11 ++ .../deployment_plan/manual_network_subnet.rb | 12 +- .../network_planner/planner.rb | 24 +-- .../availability_zone_picker.rb | 2 +- .../networks_to_static_ips.rb | 2 +- .../static_ips_availability_zone_picker.rb | 10 +- src/bosh-director/lib/bosh/director/errors.rb | 1 + .../lib/bosh/director/models/ip_address.rb | 25 ++- .../lib/bosh/director/network_reservation.rb | 29 ++-- src/bosh-director/spec/factories.rb | 2 +- .../deployment_plan/dynamic_network_spec.rb | 2 +- .../instance_group_networks_parser_spec.rb | 97 ++++++++++- .../ip_provider/ip_provider_spec.rb | 6 +- .../ip_provider/ip_repo_spec.rb | 86 +++++++--- .../deployment_plan/manual_network_spec.rb | 28 +++ .../manual_network_subnet_spec.rb | 33 ++++ ...tatic_ips_availability_zone_picker_spec.rb | 76 ++++----- 21 files changed, 426 insertions(+), 205 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb index e80045480b1..0f845bbdff2 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb @@ -94,7 +94,7 @@ def initialize(name, subnets, logger) def network_settings(reservation, default_properties = Network::REQUIRED_DEFAULTS, availability_zone = nil) unless reservation.dynamic? raise NetworkReservationWrongType, - "IP '#{format_ip(reservation.ip)}' on network '#{reservation.network.name}' does not belong to dynamic pool" + "IP '#{reservation.ip}' on network '#{reservation.network.name}' does not belong to dynamic pool" end if availability_zone.nil? diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb index 7f0fd0a5a14..1ef1a07c11b 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb @@ -65,15 +65,8 @@ def reserve_manual(reservation) @logger.debug("Allocating dynamic ip for manual network '#{reservation.network.name}'") filter_subnet_by_instance_az(reservation).each do |subnet| - ip = if reservation.is_prefix_reservation - @ip_repo.allocate_dynamic_prefix(reservation, subnet) - else - @ip_repo.allocate_dynamic_ip(reservation, subnet) - end - - @logger.debug("Reserving dynamic IP/prefix '#{ip}' for manual network '#{reservation.network.name}'") - - if ip + if (ip = @ip_repo.allocate_dynamic_ip(reservation, subnet)) + @logger.debug("Reserving dynamic IP '#{ip}' for manual network '#{reservation.network.name}'") reservation.resolve_ip(ip) reservation.resolve_type(:dynamic) break @@ -82,13 +75,13 @@ def reserve_manual(reservation) if reservation.ip.nil? raise NetworkReservationNotEnoughCapacity, - "Failed to reserve IP/Prefix for '#{reservation.instance_model}' for manual network '#{reservation.network.name}': no more available" + "Failed to reserve IP for '#{reservation.instance_model}' for manual network '#{reservation.network.name}': no more available" end else @logger.debug("Reserving #{reservation.desc} for manual network '#{reservation.network.name}'") if (subnet = reservation.network.find_subnet_containing(reservation.ip)) - if subnet.restricted_ips.include?(reservation.ip) + if subnet.restricted_ips.include?(reservation.ip.to_i) message = "Failed to reserve IP '#{format_ip(reservation.ip)}' for network '#{subnet.network_name}': IP belongs to reserved range" @logger.error(message) raise Bosh::Director::NetworkReservationIpReserved, message @@ -106,7 +99,8 @@ def reserve_manual_with_subnet(reservation, subnet) @ip_repo.add(reservation) subnet_az_names = subnet.availability_zone_names.to_a.join(', ') - if subnet.static_ips.include?(reservation.ip) + + if subnet.static_ips.include?(reservation.ip.to_i) reservation.resolve_type(:static) @logger.debug("Found subnet with azs '#{subnet_az_names}' for #{format_ip(reservation.ip)}. Reserved as static network reservation.") else diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index a037ef58e06..deb16373fa4 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -3,6 +3,7 @@ class IpRepo include Bosh::Director::IpUtil class IpFoundInDatabaseAndCanBeRetried < StandardError; end class NoMoreIPsAvailableAndStopRetrying < StandardError; end + class PrefixOutOfRange < StandardError; end def initialize(logger) @logger = Bosh::Director::TaggedLogger.new(logger, 'network-configuration') @@ -11,7 +12,7 @@ def initialize(logger) def delete(ip) ip_or_cidr = Bosh::Director::IpAddrOrCidr.new(ip) - ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip_or_cidr.to_i.to_s) + ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip_or_cidr.to_cidr_s) if ip_address @logger.debug("Releasing ip '#{ip_or_cidr}'") @@ -22,18 +23,19 @@ def delete(ip) end def add(reservation) - ip_or_cidr = Bosh::Director::IpAddrOrCidr.new(reservation.ip) + ip_or_cidr = Bosh::Director::IpAddrOrCidr.new(reservation.ip.to_cidr_s) reservation_type = reservation.network.ip_type(ip_or_cidr) reserve_with_instance_validation( reservation.instance_model, - ip_or_cidr, + ip_or_cidr.to_cidr_s, reservation, reservation_type.eql?(:static), ) reservation.resolve_type(reservation_type) + @logger.debug("Reserved ip '#{ip_or_cidr}' for #{reservation.network.name} as #{reservation_type}") end @@ -51,24 +53,7 @@ def allocate_dynamic_ip(reservation, subnet) end @logger.debug("Allocated dynamic IP '#{ip_address}' for #{reservation.network.name}") - ip_address.to_i - end - - def allocate_dynamic_prefix(reservation, subnet) - begin - cidr = try_to_allocate_dynamic_prefix(reservation, subnet) - rescue NoMoreIPsAvailableAndStopRetrying - @logger.debug('Failed to allocate dynamic prefix: no more available') - return nil - rescue IpFoundInDatabaseAndCanBeRetried - @logger.debug('Retrying to allocate dynamic prefix: probably a race condition with another deployment') - # IP can be taken by other deployment that runs in parallel - # retry until succeeds or out of range - retry - end - - @logger.debug("Allocated dynamic cidr '#{cidr}' for #{reservation.network.name}") - cidr.to_i + ip_address end def allocate_vip_ip(reservation, subnet) @@ -91,90 +76,120 @@ def allocate_vip_ip(reservation, subnet) private + def all_ip_addresses + Bosh::Director::Models::IpAddress.select(:address_str).all.map { |a| a.address } + end + + def get_prefix(ip) + if Bosh::Director::IpAddrOrCidr.new(ip).ipv6? + 128 + else + 32 + end + end + def try_to_allocate_dynamic_ip(reservation, subnet) addresses_in_use = Set.new(all_ip_addresses) - first_range_address = subnet.range.to_range.first.to_i - 1 + + first_range_address = Bosh::Director::IpAddrOrCidr.new(subnet.range.to_range.first.to_i - 1) + addresses_we_cant_allocate = addresses_in_use - addresses_we_cant_allocate << first_range_address - addresses_we_cant_allocate.merge(subnet.restricted_ips.to_a) unless subnet.restricted_ips.empty? - addresses_we_cant_allocate.merge(subnet.static_ips.to_a) unless subnet.static_ips.empty? - addr = find_first_available_address(addresses_we_cant_allocate, first_range_address) - ip_address = Bosh::Director::IpAddrOrCidr.new(addr) + addresses_we_cant_allocate.merge(subnet.restricted_ips.map { |int_ip| Bosh::Director::IpAddrOrCidr.new(int_ip)}) unless subnet.restricted_ips.empty? + addresses_we_cant_allocate.merge(subnet.static_ips.map { |int_ip| Bosh::Director::IpAddrOrCidr.new(int_ip)}) unless subnet.static_ips.empty? + + if subnet.range.ipv6? + addresses_we_cant_allocate.delete_if { |ipaddr| ipaddr.ipv4? } + if subnet.prefix.nil? + prefix = 128 + else + prefix = subnet.prefix + end + else + addresses_we_cant_allocate.delete_if { |ipaddr| ipaddr.ipv6? } + if subnet.prefix.nil? + prefix = 32 + else + prefix = subnet.prefix + end + end + + ip_address = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) unless subnet.range == ip_address || subnet.range.include?(ip_address) raise NoMoreIPsAvailableAndStopRetrying end - save_ip(ip_address, reservation, false) + unless prefix + if ip_address.ipv6? + host_bits = 128 - prefix + else + host_bits = 32 - prefix + end + no_of_addresses = 2**host_bits + if subnet.range.last < (ip_address.to_i + no_of_addresses) + raise NoMoreIPsAvailableAndStopRetrying + end + end + + save_ip(ip_address.to_cidr_s, reservation, false) ip_address end - def try_to_allocate_dynamic_prefix(reservation, subnet) - @logger.debug("Allocating dynamic prefix for #{reservation.network.name}") - addresses_in_use = Set.new(all_ip_addresses) - - first_range_address = subnet.range.to_range.first.to_i - 1 - addresses_we_cant_allocate = addresses_in_use - addresses_we_cant_allocate << first_range_address + def find_next_available_ip(ip_entries, first_range_address, prefix) + filtered_ips = ip_entries.sort_by { |ip| ip.to_i }.reject { |ip| ip.to_i < first_range_address.to_i } #remove ips that are below subnet range - addresses_we_cant_allocate.merge(subnet.restricted_ips.to_a) unless subnet.restricted_ips.empty? - addresses_we_cant_allocate.merge(subnet.static_ips.to_a) unless subnet.static_ips.empty? + if prefix == 32 || prefix == 128 + available_ip = filtered_ips.find { |ip| !filtered_ips.include?(Bosh::Director::IpAddrOrCidr.new(ip.to_i + 1)) }.to_i + 1 - addr = Bosh::Director::IpAddrOrCidr.new(find_first_available_address(addresses_we_cant_allocate, first_range_address)).to_s - cidr_string = get_first_available_prefix(addr, subnet.prefix) - - @logger.debug("FIRST AVAILABLE PREFIX '#{cidr_string}' for #{reservation.network.name}") - - cidr = Bosh::Director::IpAddrOrCidr.new(cidr_string) - - unless subnet.range == cidr || subnet.range.include?(cidr) - raise NoMoreIPsAvailableAndStopRetrying + found_cidr = Bosh::Director::IpAddrOrCidr.new("#{Bosh::Director::IpAddrOrCidr.new(available_ip)}/#{prefix}") + else + current_ip = Bosh::Director::IpAddrOrCidr.new(first_range_address.to_i + 1) + found = false + + while found == false + current_prefix = Bosh::Director::IpAddrOrCidr.new("#{current_ip}/#{prefix}") + + if filtered_ips.any? { |ip| current_prefix.include?(ip) } + current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count) + filtered_ips.reject{ |ip| ip.to_i < current_ip.to_i } + else + found_cidr = current_prefix + found = true + end + end end - save_ip(cidr, reservation, false) - - cidr - end - - def find_first_available_address(addresses_we_cant_allocate, first_address) - last_address_we_cant_use = addresses_we_cant_allocate - .to_a - .reject { |a| a < first_address } - .sort - .find { |a| !addresses_we_cant_allocate.include?(a + 1) } - last_address_we_cant_use + 1 - end - - def get_first_available_prefix(first_available_addr, prefix) - "#{first_available_addr}/#{prefix}" + found_cidr end def try_to_allocate_vip_ip(reservation, subnet) - addresses_in_use = Set.new(all_ip_addresses) + addresses_in_use = Set.new(all_ip_addresses.map { |ip| ip.to_i }) + + if Bosh::Director::IpAddrOrCidr.new(subnet.static_ips.first.to_i).ipv6? + prefix = 128 + else + prefix = 32 + end available_ips = subnet.static_ips - addresses_in_use raise NoMoreIPsAvailableAndStopRetrying if available_ips.empty? - ip_address = Bosh::Director::IpAddrOrCidr.new(available_ips.first) + ip_address = Bosh::Director::IpAddrOrCidr.new("#{Bosh::Director::IpAddrOrCidr.new(available_ips.first)}/#{prefix}") - save_ip(ip_address, reservation, false) + save_ip(ip_address.to_cidr_s, reservation, false) ip_address end - def all_ip_addresses - Bosh::Director::Models::IpAddress.select(:address_str).all.map { |a| a.address_str.to_i } - end - def reserve_with_instance_validation(instance_model, ip, reservation, is_static) # try to save IP first before validating its instance to prevent race conditions save_ip(ip, reservation, is_static) rescue IpFoundInDatabaseAndCanBeRetried - ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip.to_i.to_s) + ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip.to_s) retry unless ip_address @@ -204,15 +219,17 @@ def validate_instance_and_update_reservation_type(instance_model, ip, ip_address end def save_ip(ip, reservation, is_static) + @logger.debug("Adding IP Address: #{ip} from reservation: #{reservation}") ip_address = Bosh::Director::Models::IpAddress.new( - address_str: ip.to_i.to_s, + address_str: ip, network_name: reservation.network.name, task_id: Bosh::Director::Config.current_job.task_id, static: is_static, - ) + ) reservation.instance_model.add_ip_address(ip_address) rescue Sequel::ValidationFailed, Sequel::DatabaseError => e error_message = e.message.downcase + @logger.debug("ERROR!!! #{error_message}") if error_message.include?('unique') || error_message.include?('duplicate') raise IpFoundInDatabaseAndCanBeRetried, e.inspect else diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb index 534f05e876c..e62adc3edb2 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb @@ -1,13 +1,15 @@ module Bosh::Director module DeploymentPlan class JobNetwork - attr_reader :name, :static_ips, :deployment_network + attr_reader :name, :static_ips, :deployment_network, :prefix def initialize(name, static_ips, default_for, deployment_network) @name = name @static_ips = static_ips @default_for = default_for @deployment_network = deployment_network + subnet = deployment_network.subnets.first + @prefix = subnet.prefix end def availability_zones diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb index 26c76cd5964..9f03224e56a 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb @@ -13,10 +13,16 @@ def self.parse(network_spec, availability_zones, logger) managed = Config.network_lifecycle_enabled? && safe_property(network_spec, 'managed', default: false) subnet_specs = safe_property(network_spec, 'subnets', class: Array) subnets = [] + prefix = nil subnet_specs.each do |subnet_spec| new_subnet = ManualNetworkSubnet.parse(name, subnet_spec, availability_zones, managed) subnets.each do |subnet| raise NetworkOverlappingSubnets, "Network '#{name}' has overlapping subnets" if subnet.overlaps?(new_subnet) + if prefix.nil? + prefix = subnet.prefix + elsif prefix != subnet.prefix + raise NetworkPrefixSizesDiffer, "Network '#{name}' has subnets that define different prefixes" + end end subnets << new_subnet end @@ -52,9 +58,14 @@ def network_settings(reservation, default_properties = REQUIRED_DEFAULTS, availa raise NetworkReservationInvalidIp, "Provided IP '#{ip_or_cidr}' does not belong to any subnet" end + unless subnet.prefix == ip_or_cidr.prefix + raise NetworkReservationInvalidPRefix, "Subnet Prefix #{subnet.prefix} and ip reservation prefix #{ip_or_cidr.prefix} do not match" + end + config = { "type" => "manual", "ip" => ip_or_cidr.to_s, + "prefix" => subnet.prefix.to_s, "netmask" => subnet.netmask, "cloud_properties" => subnet.cloud_properties } diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb index 36b20af1a61..143d4b0d2a2 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb @@ -77,8 +77,14 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) static_ips.add(ip) end - unless prefix.nil? - if range.prefix > prefix + if prefix.nil? + if range.ipv6? + prefix = 128 + else + prefix = 32 + end + else + if range.prefix > prefix.to_i raise NetworkPrefixSizeTooBig, "Prefix size '#{prefix}' is larger than range prefix '#{range.prefix}'" end end @@ -106,7 +112,7 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) ) end - def initialize(network_name, range, gateway, name_servers, cloud_properties, netmask, availability_zone_names, restricted_ips, static_ips, subnet_name = nil, netmask_bits = nil, prefix) + def initialize(network_name, range, gateway, name_servers, cloud_properties, netmask, availability_zone_names, restricted_ips, static_ips, subnet_name = nil, netmask_bits = nil, prefix = nil) @network_name = network_name @name = subnet_name @netmask_bits = netmask_bits diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb index a40639fb367..0959bb920f0 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb @@ -6,31 +6,15 @@ def initialize(logger) end def network_plan_with_dynamic_reservation(instance_plan, job_network) - plans = [] - reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network) + reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network) @logger.debug("Creating new dynamic reservation #{reservation} for instance '#{instance_plan.instance}'") - plans << Plan.new(reservation: reservation) - job_network.deployment_network.subnets.each do |subnet| - unless subnet.prefix.nil? - prefix_reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network, true) - plans << Plan.new(reservation: prefix_reservation) - end - end - plans + Plan.new(reservation: reservation) end def network_plan_with_static_reservation(instance_plan, job_network, static_ip) - plans = [] - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_plan.instance.model, job_network.deployment_network, static_ip) + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_plan.instance.model, job_network, static_ip) @logger.debug("Creating new static reservation #{reservation} for instance '#{instance_plan.instance}'") - plans << Plan.new(reservation: reservation) - job_network.deployment_network.subnets.each do |subnet| - unless subnet.prefix.nil? - prefix_reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network, true) - plans << Plan.new(reservation: prefix_reservation) - end - end - plans + Plan.new(reservation: reservation) end end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb index 632c3e59431..2c11a70b02c 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/availability_zone_picker.rb @@ -123,7 +123,7 @@ def desired_new_instance_plans(new_desired_instances) def populate_network_plans(instance_plan) @networks.each do |network| - @network_planner.network_plan_with_dynamic_reservation(instance_plan, network).each {|plan| instance_plan.network_plans << plan } + instance_plan.network_plans << @network_planner.network_plan_with_dynamic_reservation(instance_plan, network) end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb index 994ce32a727..65847bf81cd 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb @@ -91,7 +91,7 @@ def next_ip_for_network(network) def claim_in_az(ip, az_name) @networks_to_static_ips.each do |_, static_ips_to_azs| static_ips_to_azs.each do |static_ip_to_azs| - if static_ip_to_azs.ip == ip + if static_ip_to_azs.ip == ip.to_i static_ip_to_azs.claimed = true static_ip_to_azs.az_names = [az_name] end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb index 6a0cd6173c8..70fffeaee04 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb @@ -141,13 +141,15 @@ def create_network_plan_with_az(instance_plan, network, instance_plans) def create_instance_plan_based_on_existing_ips(desired_instances, existing_instance_model) instance_plan = nil + @job_networks.each do |network| next unless network.static? instance_ips_on_network = find_instance_ips_on_network(existing_instance_model, network) network_plan = nil instance_ips_on_network.each do |instance_ip| - ip_address = instance_ip.address + ip_address = instance_ip.address.to_i + # Instance is using IP in static IPs list, we have to use this instance @logger.debug("Existing instance '#{instance_name(existing_instance_model)}' is using static IP '#{format_ip(ip_address)}' on network '#{network.name}'") if instance_plan.nil? @@ -192,7 +194,8 @@ def create_existing_instance_plan_with_az(desired_instance, existing_instance_mo end def assign_az_based_on_ip(desired_instance, existing_instance_model, network, ip_address) - ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address).az_names + ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address.to_i).az_names + if ip_az_names.include?(existing_instance_model.availability_zone) az_name = existing_instance_model.availability_zone @logger.debug("Instance '#{instance_name(existing_instance_model)}' belongs to az '#{az_name}' that is in subnet az list, reusing instance az.") @@ -246,7 +249,8 @@ def create_existing_instance_plan(desired_instance, existing_instance_model) def create_network_plan_with_ip(instance_plan, network, ip_address) instance_az = instance_plan.desired_instance.az instance_az_name = instance_az.nil? ? nil : instance_az.name - ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address).az_names + + ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address.to_i).az_names if ip_az_names.include?(instance_az_name) instance_plan.network_plans << @network_planner.network_plan_with_static_reservation(instance_plan, network, ip_address) end diff --git a/src/bosh-director/lib/bosh/director/errors.rb b/src/bosh-director/lib/bosh/director/errors.rb index f51ba6446c4..80660c5e5b4 100644 --- a/src/bosh-director/lib/bosh/director/errors.rb +++ b/src/bosh-director/lib/bosh/director/errors.rb @@ -225,6 +225,7 @@ def self.err(error_code, response_code = BAD_REQUEST) NetworkDeletingUnorphanedError = err(160011) NetworkNotFoundError = err(16012) NetworkPrefixSizeTooBig = err(16013) + NetworkPrefixSizesDiffer = err(16014) # ResourcePool ResourcePoolUnknownNetwork = err(170001) diff --git a/src/bosh-director/lib/bosh/director/models/ip_address.rb b/src/bosh-director/lib/bosh/director/models/ip_address.rb index ecc441816a8..9a6fdc9b6b6 100644 --- a/src/bosh-director/lib/bosh/director/models/ip_address.rb +++ b/src/bosh-director/lib/bosh/director/models/ip_address.rb @@ -23,7 +23,7 @@ def info [ "#{instance.deployment.name}.#{instance.job}/#{instance.index}", network_name, - "#{Bosh::Director::IpAddrOrCidr.new(address_str.to_i)} (#{type})" + "#{Bosh::Director::IpAddrOrCidr.new(address_str)} (#{type})" ].join(' - ') end @@ -35,17 +35,24 @@ def type static ? 'static' : 'dynamic' end + def address_int_and_prefix + address_and_prefix = address.split('/') + [Bosh::Director::IpAddrOrCidr.new(address_and_prefix[0]).to_i, address_and_prefix[1]] + end + def address - unless address_str.match?(/\A\d+\z/) - info_display = '' - begin - info_display = info - rescue StandardError - info_display = 'missing_info' + if address_str.include?('/') + return Bosh::Director::IpAddrOrCidr.new(address_str) + else + ip = Bosh::Director::IpAddrOrCidr.new(address_str.to_i) + if ip.ipv6? + prefix = 132 + else + prefix = 32 end - raise "Unexpected address '#{address_str}' (#{info_display})" + address_str = Bosh::Director::IpAddrOrCidr.new("#{ip}/#{prefix}") + return address_str end - address_str.to_i end def to_s diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 7506841e53a..473befb2a37 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -1,6 +1,6 @@ module Bosh::Director class NetworkReservation - attr_reader :ip, :instance_model, :network, :type, :is_prefix_reservation + attr_reader :ip, :instance_model, :network, :type def initialize(instance_model, network) @instance_model = instance_model @@ -19,7 +19,7 @@ def dynamic? private def formatted_ip - IpAddrOrCidr.new(@ip).to_s if @ip + IpAddrOrCidr.new(@ip).to_cidr_s if @ip end end @@ -28,8 +28,7 @@ class ExistingNetworkReservation < NetworkReservation def initialize(instance_model, network, ip, network_type) super(instance_model, network) - @ip = IpAddrOrCidr.new(ip).to_i if ip - @is_prefix_reservation = IpAddrOrCidr.new(ip).prefix != 32 + @ip = IpAddrOrCidr.new(ip) if ip @network_type = network_type @obsolete = network.instance_of? Bosh::Director::DeploymentPlan::Network end @@ -48,23 +47,31 @@ def to_s end class DesiredNetworkReservation < NetworkReservation - def self.new_dynamic(instance_model, network, prefix_reservation = false) - new(instance_model, network, nil, :dynamic, prefix_reservation) + def self.new_dynamic(instance_model, network) + new(instance_model, network.deployment_network, nil, :dynamic) end def self.new_static(instance_model, network, ip) - new(instance_model, network, ip, :static) + cidr_ip = "#{IpAddrOrCidr.new(ip)}/#{network.prefix}" + new(instance_model, network.deployment_network, cidr_ip, :static) end - def initialize(instance_model, network, ip, type, prefix_reservation) + def initialize(instance_model, network, ip, type) super(instance_model, network) - @ip = IpAddrOrCidr.new(ip).to_i if ip + @ip = resolve_ip(ip) if ip @type = type - @is_prefix_reservation = prefix_reservation end def resolve_ip(ip) - @ip = IpAddrOrCidr.new(ip).to_i +# if !ip.to_s.match?(/\//) + @ip = IpAddrOrCidr.new(ip) +# else +# if IpAddrOrCidr.new(ip).ipv6? +# @ip = IpAddrOrCidr.new("#{ip}/128") +# else +# @ip = IpAddrOrCidr.new("#{ip}/32") +# end +# end end def resolve_type(type) diff --git a/src/bosh-director/spec/factories.rb b/src/bosh-director/spec/factories.rb index c0ebfa335e3..06193b0a3b7 100644 --- a/src/bosh-director/spec/factories.rb +++ b/src/bosh-director/spec/factories.rb @@ -170,7 +170,7 @@ factory :models_ip_address, class: Bosh::Director::Models::IpAddress do sequence(:network_name) { |i| "ip-address-network-name-#{i}" } sequence(:task_id) { |i| "ip-address-task-id-#{i}" } - address_str { IPAddr.new(Random.rand(IPAddr::IN4MASK.to_i), Socket::AF_INET).to_i.to_s } + address_str { IPAddr.new(Random.rand(IPAddr::IN4MASK), Socket::AF_INET).to_s } static { false } created_at { Time.now } association :instance, factory: :models_instance, strategy: :create diff --git a/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb b/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb index 4dae3f9b50a..4a0a81ca9ec 100644 --- a/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb @@ -341,7 +341,7 @@ end it 'should fail when for static reservation' do - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, @network, 1) + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, @network, "1.1.1.1") expect { @network.network_settings(reservation) }.to raise_error Bosh::Director::NetworkReservationWrongType diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb index cf0331b2d70..39d0d398a75 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb @@ -71,6 +71,24 @@ module Bosh::Director::DeploymentPlan end context 'when called with a valid instance group spec' do + let(:subnet_spec) do + { + 'range' => '192.168.1.0/24', + 'gateway' => '192.168.1.1' + } + end + let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } + + let(:instance_group_spec) do + instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first + instance_group['networks'] = [{ + 'name' => 'a', + 'static_ips' => ['192.168.1.1', '192.168.1.2'], + }] + instance_group + end + + it 'adds static ips to instance group networks in order as they are in manifest' do networks = instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) @@ -80,18 +98,91 @@ module Bosh::Director::DeploymentPlan name: 'a', static_ips: ['192.168.1.1', '192.168.1.2'], default_for: %w[dns gateway], - deployment_network: manifest_networks.first, + deployment_network: manifest_networks.first ), + ) + expect(networks.first.static_ips).to eq([to_ipaddr('192.168.1.1'), to_ipaddr('192.168.1.2')]) + expect(networks.first.prefix).to eq(32) + end + end + + context 'when called with a valid instance group spec and the subnet defines a prefix' do + let(:subnet_spec) do + { + 'range' => '192.168.1.0/24', + 'gateway' => '192.168.1.1', + 'prefix' => '25', + } + end + let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } + + let(:instance_group_spec) do + instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first + instance_group['networks'] = [{ + 'name' => 'a', + 'static_ips' => ['192.168.1.1', '192.168.1.2'], + }] + instance_group + end + + + it 'adds the prefix' do + networks = instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) + + expect(networks.count).to eq(1) + expect(networks.first).to be_an_instance_group_network( + FactoryBot.build(:deployment_plan_job_network, + name: 'a', + static_ips: ['192.168.1.1', '192.168.1.2'], + default_for: %w[dns gateway], + deployment_network: manifest_networks.first ), ) expect(networks.first.static_ips).to eq([ip_to_i('192.168.1.1'), ip_to_i('192.168.1.2')]) + expect(networks.first.prefix).to eq('25') + end + end + + context 'when called with a valid instance group spec for ipv6 without prefix' do + let(:subnet_spec) do + { + 'range' => '2001:0db8:1234::/32', + 'gateway' => '2001:0db8:1234::1', + } + end + let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } + + let(:instance_group_spec) do + instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first + instance_group['networks'] = [{ + 'name' => 'a', + 'static_ips' => ['2001:0db8:1234::2', '2001:0db8:1234::3'], + }] + instance_group + end + + + it 'adds the /128 prefix for a single ip address' do + networks = instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) + + expect(networks.count).to eq(1) + expect(networks.first).to be_an_instance_group_network( + FactoryBot.build(:deployment_plan_job_network, + name: 'a', + static_ips: ['2001:0db8:1234:0000:0000:0000:0000:0002', '2001:0db8:1234:0000:0000:0000:0000:0003'], + default_for: %w[dns gateway], + deployment_network: manifest_networks.first + ), + ) + expect(networks.first.static_ips).to eq([to_ipaddr('2001:0db8:1234:0000:0000:0000:0000:0002'), to_ipaddr('2001:0db8:1234:0000:0000:0000:0000:0003')]) + expect(networks.first.prefix).to eq(128) end end RSpec::Matchers.define :be_an_instance_group_network do |expected| match do |actual| actual.name == expected.name && - actual.static_ips == expected.static_ips.map { |ip_to_i| IPAddr.new(ip_to_i) } && - actual.deployment_network == expected.deployment_network + actual.static_ips == expected.static_ips.map { |ip| IPAddr.new(ip) } && + actual.deployment_network == expected.deployment_network && actual.prefix == expected.prefix end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb index 23b00411e7f..bce788c6ddd 100644 --- a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb @@ -289,7 +289,7 @@ module Bosh::Director::DeploymentPlan it 'raises an error' do reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, manual_network) - reservation.resolve_ip(IPAddr.new('192.168.1.11').to_i) + reservation.resolve_ip(Bosh::Director::IpAddrOrCidr.new('192.168.1.11')) expect do ip_provider.reserve(reservation) end.to raise_error Bosh::Director::NetworkReservationIpReserved, @@ -307,7 +307,7 @@ module Bosh::Director::DeploymentPlan expect { ip_provider.reserve(reservation) }.to raise_error Bosh::Director::NetworkReservationWrongType, - "IP '192.168.1.2' on network 'my-manual-network' does not belong to dynamic pool" + "IP '192.168.1.2/32' on network 'my-manual-network' does not belong to dynamic pool" end end end @@ -348,7 +348,7 @@ module Bosh::Director::DeploymentPlan expect { ip_provider.reserve(reservation) }.to raise_error Bosh::Director::NetworkReservationWrongType, - "IP '192.168.1.2' on network 'my-manual-network' does not belong to dynamic pool" + "IP '192.168.1.2/32' on network 'my-manual-network' does not belong to dynamic pool" end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb index 946047a9ce9..f5110e72a64 100644 --- a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb @@ -28,6 +28,7 @@ module Bosh::Director::DeploymentPlan per_spec_logger ) end + let(:job_network) {JobNetwork.new(nil, nil, nil, network)} let(:subnet) do ManualNetworkSubnet.parse( network.name, @@ -36,6 +37,22 @@ module Bosh::Director::DeploymentPlan ) end + let(:subnet_with_prefix) do + ManualNetworkSubnet.parse( + network.name, + network_spec['subnets'].first.merge('prefix' => '31'), + availability_zones, + ) + end + + let(:subnet_with_too_big_prefix) do + ManualNetworkSubnet.parse( + network.name, + network_spec['subnets'].first.merge('prefix' => '30'), + availability_zones, + ) + end + let(:other_network_spec) { network_spec.merge('name' => 'my-other-manual-network') } let(:other_network) do ManualNetwork.parse( @@ -44,6 +61,7 @@ module Bosh::Director::DeploymentPlan per_spec_logger ) end + let(:job_other_network) {JobNetwork.new(nil, nil, nil, other_network)} let(:other_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, other_network) } let(:other_subnet) do ManualNetworkSubnet.parse( @@ -56,12 +74,12 @@ module Bosh::Director::DeploymentPlan before { fake_job } def cidr_ip(ip) - IPAddr.new(ip).to_i + Bosh::Director::IpAddrOrCidr.new(ip) end context :add do def dynamic_reservation_with_ip(ip) - reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network_without_static_pool) + reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, JobNetwork.new(nil, nil, nil, network_without_static_pool)) reservation.resolve_ip(ip) ip_repo.add(reservation) @@ -77,7 +95,7 @@ def dynamic_reservation_with_ip(ip) context 'from Static to Dynamic' do it 'updates type of reservation' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -97,6 +115,7 @@ def dynamic_reservation_with_ip(ip) context 'from Dynamic to Static' do it 'update type of reservation' do dynamic_reservation = dynamic_reservation_with_ip('192.168.1.5') + ip_repo.add(dynamic_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -104,7 +123,7 @@ def dynamic_reservation_with_ip(ip) expect(original_address.static).to eq(false) network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -138,7 +157,7 @@ def dynamic_reservation_with_ip(ip) context 'when reservation changes network' do it 'updates network name' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -146,7 +165,7 @@ def dynamic_reservation_with_ip(ip) expect(original_address.static).to eq(true) expect(original_address.network_name).to eq(network.name) - static_reservation_on_another_network = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, other_network, '192.168.1.5') + static_reservation_on_another_network = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_other_network, '192.168.1.5') ip_repo.add(static_reservation_on_another_network) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -165,11 +184,11 @@ def dynamic_reservation_with_ip(ip) end network_spec['subnets'].first['static'] = ['192.168.1.5'] - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(reservation) saved_address = Bosh::Director::Models::IpAddress.order(:address_str).last - expect(saved_address.address_str).to eq(cidr_ip('192.168.1.5').to_s) + expect(saved_address.address_str).to eq(cidr_ip('192.168.1.5').to_cidr_s) expect(saved_address.network_name).to eq('my-manual-network') expect(saved_address.task_id).to eq('fake-task-id') expect(saved_address.created_at).to_not be_nil @@ -181,8 +200,8 @@ def dynamic_reservation_with_ip(ip) network_spec['subnets'].first['static'] = ['192.168.1.5'] other_instance_model = FactoryBot.create(:models_instance, availability_zone: 'az-2') - original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') - new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, network, '192.168.1.5') + original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, job_network, '192.168.1.5') ip_repo.add(original_static_network_reservation) @@ -195,8 +214,8 @@ def dynamic_reservation_with_ip(ip) network_spec['subnets'].first['static'] = ['192.168.1.5'] other_instance_model = FactoryBot.create(:models_instance, availability_zone: 'az-2') - original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') - new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, network, '192.168.1.5') + original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, job_network, '192.168.1.5') ip_repo.add(original_static_network_reservation) @@ -211,7 +230,7 @@ def dynamic_reservation_with_ip(ip) it 'should succeed if it is reserved by the same instance' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(static_network_reservation) @@ -223,7 +242,7 @@ def dynamic_reservation_with_ip(ip) end describe :allocate_dynamic_ip do - let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } context 'when there are no IPs reserved' do it 'returns the first in the range' do @@ -273,13 +292,13 @@ def dynamic_reservation_with_ip(ip) it 'returns first non-reserved IP' do network_spec['subnets'].first['static'] = ['192.168.1.2', '192.168.1.4'] - reservation_1 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2') - reservation_2 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.4') + reservation_1 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.2') + reservation_2 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.4') ip_repo.add(reservation_1) ip_repo.add(reservation_2) - reservation_3 = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) + reservation_3 = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) ip_address = ip_repo.allocate_dynamic_ip(reservation_3, subnet) expect(ip_address).to eq(cidr_ip('192.168.1.3')) @@ -310,22 +329,39 @@ def dynamic_reservation_with_ip(ip) end end + context 'when a prefix is assigned to the subnet' do + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } + it 'reserves the prefix' do + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet_with_prefix) + + expect(ip_address).to eq(cidr_ip('192.168.1.2/31')) + end + + it 'should stop retrying and return nil if no sufficient range is available' do + expect do + ip = ip_repo.allocate_dynamic_ip(reservation, subnet_with_too_big_prefix) + expect(ip).to be_nil + end.not_to(change { Bosh::Director::Models::IpAddress.count }) + end + end + + context 'when reserving IP fails' do def fail_saving_ips(ips, fail_error) original_saves = {} ips.each do |ip| ip_address = Bosh::Director::Models::IpAddress.new( - address_str: ip.to_s, + address_str: ip.to_cidr_s, network_name: 'my-manual-network', instance: instance_model, task_id: Bosh::Director::Config.current_job.task_id ) original_save = ip_address.method(:save) - original_saves[ip.to_s] = original_save + original_saves[ip.to_cidr_s] = original_save end allow_any_instance_of(Bosh::Director::Models::IpAddress).to receive(:save) do |model| - if ips.map(&:to_s).include?(model.address_str) + if ips.map(&:to_cidr_s).include?(model.address_str) original_save = original_saves[model.address_str] original_save.call raise fail_error @@ -394,7 +430,7 @@ def fail_saving_ips(ips, fail_error) end describe :allocate_vip_ip do - let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } let(:network) do VipNetwork.parse( @@ -425,7 +461,7 @@ def fail_saving_ips(ips, fail_error) end.to change { Bosh::Director::Models::IpAddress.count }.by(1) ip_address = instance_model.ip_addresses.first - expect(ip_address.address_str.to_i).to eq(cidr_ip('69.69.69.69')) + expect(ip_address.address_str).to eq(cidr_ip('69.69.69.69').to_cidr_s) end context 'when there are no vips defined in the network' do @@ -452,7 +488,7 @@ def fail_saving_ips(ips, fail_error) end context 'when there are no available vips' do - let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } + let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } before do ip_repo.allocate_vip_ip(existing_reservation, network.subnets.first) @@ -500,13 +536,13 @@ def fail_saving_ips(ips, fail_error) before do network_spec['subnets'].first['static'] = ['192.168.1.5'] - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') ip_repo.add(reservation) end it 'deletes IP address' do expect { - ip_repo.delete('192.168.1.5') + ip_repo.delete('192.168.1.5/32') }.to change { Bosh::Director::Models::IpAddress.all.size }.by(-1) diff --git a/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb index d09ba135424..866d99667cf 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb @@ -133,6 +133,7 @@ expect(manual_network.network_settings(reservation, [])).to eq( 'type' => 'manual', 'ip' => '192.168.1.2', + 'prefix' => '32', 'netmask' => '255.255.255.0', 'cloud_properties' => {}, 'gateway' => '192.168.1.1', @@ -151,6 +152,7 @@ expect(manual_network.network_settings(reservation)).to eq( 'type' => 'manual', 'ip' => '192.168.1.2', + 'prefix' => '32', 'netmask' => '255.255.255.0', 'cloud_properties' => {}, 'gateway' => '192.168.1.1', @@ -159,6 +161,32 @@ ) end + context 'when a prefix is maintained for a subnet' do + let(:network_spec) do + cloud_config_hash['networks'].first['prefix'] = '30' + cloud_config_hash['networks'].first + end + + it 'should set the correct prefix if one is maintained for the subnet' do + reservation = Bosh::Director::DesiredNetworkReservation.new_static( + instance_model, + manual_network, + '192.168.1.2', + ) + + expect(manual_network.network_settings(reservation)).to eq( + 'type' => 'manual', + 'ip' => '192.168.1.2', + 'prefix' => '30', + 'netmask' => '255.255.255.0', + 'cloud_properties' => {}, + 'gateway' => '192.168.1.1', + 'dns' => ['192.168.1.1', '192.168.1.2'], + 'default' => %w[dns gateway], + ) + end + end + it 'should fail when there is no IP' do reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic( instance_model, diff --git a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb index 185363dccb1..651e6d13b1f 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb @@ -328,6 +328,39 @@ def make_managed_subnet(properties, availability_zones) expect(subnet.restricted_ips).to include(ip1.to_i) expect(subnet.restricted_ips).to include(ip2.to_i) end + + it 'should create a subnet spec with prefix' do + subnet = make_subnet( + { + 'range' => '192.168.0.0/24', + 'gateway' => '192.168.0.254', + 'cloud_properties' => {'foo' => 'bar'}, + 'prefix' => 25 + }, + [], + ) + + expect(subnet.range.to_cidr_s).to eq('192.168.0.0/24') + expect(subnet.netmask).to eq('255.255.255.0') + expect(subnet.gateway).to eq('192.168.0.254') + expect(subnet.prefix).to eq(25) + expect(subnet.dns).to eq(nil) + end + + + it 'should fail if the prefix size is larger than the range' do + expect { + make_subnet( + { + 'range' => '192.168.0.0/24', + 'gateway' => '192.168.0.254', + 'cloud_properties' => {'foo' => 'bar'}, + 'prefix' => 23 + }, + [], + )}.to raise_error(Bosh::Director::NetworkPrefixSizeTooBig, + "Prefix size '23' is larger than range prefix '24'") + end end describe :overlaps? do diff --git a/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb b/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb index 693138623b8..a759ac87272 100644 --- a/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb @@ -361,8 +361,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.10', '192.168.2.10'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32']), ] end @@ -382,8 +382,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.10', '192.168.2.10'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32']), ] end let(:instance_group_availability_zones) { ['zone1'] } @@ -408,8 +408,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.14', '192.168.2.14'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32']), ] end let(:instance_group_availability_zones) { %w[zone1 zone2] } @@ -487,7 +487,7 @@ def make_subnet_spec(range, static_ips, zone_names) end let(:new_subnet_azs) { %w[zone2 zone1] } let(:static_ips) { ['192.168.1.10'] } - let(:existing_instances) { [existing_instance_with_az_and_ips('zone1', ['192.168.1.10'])] } + let(:existing_instances) { [existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32'])] } it 'reuses AZ that existing instance with static IP belongs to' do expect(new_instance_plans).to eq([]) @@ -518,8 +518,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.10', '192.168.1.11', '192.168.1.12', '192.168.1.13'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10']), - existing_instance_with_az_and_ips('zone1', ['192.168.1.12']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.12/32']), ] end it 'should distribute the instances across the azs taking into account the existing instances' do @@ -541,8 +541,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.10', '192.168.2.11'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32']), ] end @@ -577,10 +577,10 @@ def make_subnet_spec(range, static_ips, zone_names) context 'when all existing instances match specified static ips' do let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10', '10.10.2.10']), - existing_instance_with_az_and_ips('zone1', ['192.168.1.11', '10.10.1.11']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.11', '10.10.2.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32', '10.10.2.10/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.11/32', '10.10.1.11/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.11/32', '10.10.2.11/32']), ] end @@ -614,10 +614,10 @@ def make_subnet_spec(range, static_ips, zone_names) context 'when some existing instances have IPs that are different from the instance group static IPs' do let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.14', '10.10.2.14']), - existing_instance_with_az_and_ips('zone1', ['192.168.1.14', '10.10.1.14']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.11', '10.10.2.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.14/32', '10.10.2.14/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.14/32', '10.10.1.14/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.11/32', '10.10.2.11/32']), ] end @@ -655,7 +655,7 @@ def make_subnet_spec(range, static_ips, zone_names) let(:desired_instance_count) { 1 } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.2.10/32']), ] end let(:a_static_ips) { ['192.168.1.10'] } @@ -676,8 +676,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:desired_instance_count) { 3 } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips('zone1', ['192.168.1.11', '10.10.1.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.11/32', '10.10.1.11/32']), ] end let(:a_static_ips) { ['192.168.1.10 - 192.168.1.11', '192.168.2.10'] } @@ -698,9 +698,9 @@ def make_subnet_spec(range, static_ips, zone_names) let(:desired_instance_count) { 2 } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips('zone1', ['192.168.1.11', '10.10.1.11']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10', '10.10.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.11/32', '10.10.1.11/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32', '10.10.2.10/32']), ] end let(:a_static_ips) { ['192.168.1.10', '192.168.2.10'] } @@ -725,8 +725,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.2.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10', '10.10.2.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.2.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32', '10.10.2.11/32']), ] end let(:a_static_ips) { ['192.168.1.10', '192.168.2.10'] } @@ -765,8 +765,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:desired_instance_count) { 2 } let(:existing_instances) do [ - existing_instance_with_az_and_ips(nil, ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips(nil, ['192.168.2.10', '10.10.2.11']), + existing_instance_with_az_and_ips(nil, ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips(nil, ['192.168.2.10/32', '10.10.2.11/32']), ] end let(:a_static_ips) { ['192.168.1.10', '192.168.2.10'] } @@ -780,8 +780,8 @@ def make_subnet_spec(range, static_ips, zone_names) context 'when existing instances have AZs' do let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '10.10.1.10']), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10', '10.10.2.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '10.10.1.10/32']), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32', '10.10.2.11/32']), ] end @@ -816,8 +816,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:desired_instance_count) { 2 } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.5.10', '10.10.5.10']), - existing_instance_with_az_and_ips('zone1', ['192.168.6.10', '10.10.6.11']), + existing_instance_with_az_and_ips('zone1', ['192.168.5.10/32', '10.10.5.10/32']), + existing_instance_with_az_and_ips('zone1', ['192.168.6.10/32', '10.10.6.11/32']), ] end let(:a_static_ips) { ['192.168.1.10', '192.168.2.10'] } @@ -848,7 +848,7 @@ def make_subnet_spec(range, static_ips, zone_names) end let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '192.168.2.10/32']), ] end let(:a_static_ips) { ['192.168.1.10 - 192.168.1.11'] } @@ -882,7 +882,7 @@ def make_subnet_spec(range, static_ips, zone_names) let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10', '192.168.2.10']), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32', '192.168.2.10/32']), ] end @@ -909,8 +909,8 @@ def make_subnet_spec(range, static_ips, zone_names) let(:static_ips) { ['192.168.1.10', '192.168.2.10'] } let(:existing_instances) do [ - existing_instance_with_az_and_ips('zone1', ['192.168.1.10'], 'old-network-name'), - existing_instance_with_az_and_ips('zone2', ['192.168.2.10'], 'old-network-name'), + existing_instance_with_az_and_ips('zone1', ['192.168.1.10/32'], 'old-network-name'), + existing_instance_with_az_and_ips('zone2', ['192.168.2.10/32'], 'old-network-name'), ] end @@ -952,7 +952,7 @@ def existing_instance_with_az_and_ips(az, ips, network_name = 'a') ips.each do |ip| instance.add_ip_address( FactoryBot.create(:models_ip_address, - address_str: IPAddr.new(ip).to_i.to_s, + address_str: ip, network_name: network_name, ), ) From 914029002de594c1d2c70df1794f7720ba33bc5e Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 09:36:07 +0200 Subject: [PATCH 04/17] remove prefix from job network --- .../director/deployment_plan/job_network.rb | 4 +- .../bosh/director/deployment_plan/network.rb | 4 + .../network_planner/planner.rb | 4 +- .../lib/bosh/director/network_reservation.rb | 4 +- .../instance_group_networks_parser_spec.rb | 76 +------------------ .../ip_provider/ip_repo_spec.rb | 40 +++++----- 6 files changed, 29 insertions(+), 103 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb index e62adc3edb2..534f05e876c 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/job_network.rb @@ -1,15 +1,13 @@ module Bosh::Director module DeploymentPlan class JobNetwork - attr_reader :name, :static_ips, :deployment_network, :prefix + attr_reader :name, :static_ips, :deployment_network def initialize(name, static_ips, default_for, deployment_network) @name = name @static_ips = static_ips @default_for = default_for @deployment_network = deployment_network - subnet = deployment_network.subnets.first - @prefix = subnet.prefix end def availability_zones diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/network.rb index f970fc75d2e..55d470b94b8 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/network.rb @@ -87,6 +87,10 @@ def has_azs?(az_names) def availability_zones @subnets.map(&:availability_zone_names).flatten.uniq end + + def prefix # for now the prefix should be considered the same for all subnets + @subnets.first.prefix + end end end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb index 0959bb920f0..c38834b0f5e 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/network_planner/planner.rb @@ -6,13 +6,13 @@ def initialize(logger) end def network_plan_with_dynamic_reservation(instance_plan, job_network) - reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network) + reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_plan.instance.model, job_network.deployment_network) @logger.debug("Creating new dynamic reservation #{reservation} for instance '#{instance_plan.instance}'") Plan.new(reservation: reservation) end def network_plan_with_static_reservation(instance_plan, job_network, static_ip) - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_plan.instance.model, job_network, static_ip) + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_plan.instance.model, job_network.deployment_network, static_ip) @logger.debug("Creating new static reservation #{reservation} for instance '#{instance_plan.instance}'") Plan.new(reservation: reservation) end diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 473befb2a37..0ed0e5487c5 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -48,12 +48,12 @@ def to_s class DesiredNetworkReservation < NetworkReservation def self.new_dynamic(instance_model, network) - new(instance_model, network.deployment_network, nil, :dynamic) + new(instance_model, network, nil, :dynamic) end def self.new_static(instance_model, network, ip) cidr_ip = "#{IpAddrOrCidr.new(ip)}/#{network.prefix}" - new(instance_model, network.deployment_network, cidr_ip, :static) + new(instance_model, network, cidr_ip, :static) end def initialize(instance_model, network, ip, type) diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb index 39d0d398a75..0491f698634 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb @@ -101,80 +101,6 @@ module Bosh::Director::DeploymentPlan deployment_network: manifest_networks.first ), ) expect(networks.first.static_ips).to eq([to_ipaddr('192.168.1.1'), to_ipaddr('192.168.1.2')]) - expect(networks.first.prefix).to eq(32) - end - end - - context 'when called with a valid instance group spec and the subnet defines a prefix' do - let(:subnet_spec) do - { - 'range' => '192.168.1.0/24', - 'gateway' => '192.168.1.1', - 'prefix' => '25', - } - end - let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } - - let(:instance_group_spec) do - instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first - instance_group['networks'] = [{ - 'name' => 'a', - 'static_ips' => ['192.168.1.1', '192.168.1.2'], - }] - instance_group - end - - - it 'adds the prefix' do - networks = instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) - - expect(networks.count).to eq(1) - expect(networks.first).to be_an_instance_group_network( - FactoryBot.build(:deployment_plan_job_network, - name: 'a', - static_ips: ['192.168.1.1', '192.168.1.2'], - default_for: %w[dns gateway], - deployment_network: manifest_networks.first - ), - ) - expect(networks.first.static_ips).to eq([ip_to_i('192.168.1.1'), ip_to_i('192.168.1.2')]) - expect(networks.first.prefix).to eq('25') - end - end - - context 'when called with a valid instance group spec for ipv6 without prefix' do - let(:subnet_spec) do - { - 'range' => '2001:0db8:1234::/32', - 'gateway' => '2001:0db8:1234::1', - } - end - let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } - - let(:instance_group_spec) do - instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first - instance_group['networks'] = [{ - 'name' => 'a', - 'static_ips' => ['2001:0db8:1234::2', '2001:0db8:1234::3'], - }] - instance_group - end - - - it 'adds the /128 prefix for a single ip address' do - networks = instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) - - expect(networks.count).to eq(1) - expect(networks.first).to be_an_instance_group_network( - FactoryBot.build(:deployment_plan_job_network, - name: 'a', - static_ips: ['2001:0db8:1234:0000:0000:0000:0000:0002', '2001:0db8:1234:0000:0000:0000:0000:0003'], - default_for: %w[dns gateway], - deployment_network: manifest_networks.first - ), - ) - expect(networks.first.static_ips).to eq([to_ipaddr('2001:0db8:1234:0000:0000:0000:0000:0002'), to_ipaddr('2001:0db8:1234:0000:0000:0000:0000:0003')]) - expect(networks.first.prefix).to eq(128) end end @@ -182,7 +108,7 @@ module Bosh::Director::DeploymentPlan match do |actual| actual.name == expected.name && actual.static_ips == expected.static_ips.map { |ip| IPAddr.new(ip) } && - actual.deployment_network == expected.deployment_network && actual.prefix == expected.prefix + actual.deployment_network == expected.deployment_network end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb index f5110e72a64..1967452f05d 100644 --- a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb @@ -28,7 +28,6 @@ module Bosh::Director::DeploymentPlan per_spec_logger ) end - let(:job_network) {JobNetwork.new(nil, nil, nil, network)} let(:subnet) do ManualNetworkSubnet.parse( network.name, @@ -61,7 +60,6 @@ module Bosh::Director::DeploymentPlan per_spec_logger ) end - let(:job_other_network) {JobNetwork.new(nil, nil, nil, other_network)} let(:other_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, other_network) } let(:other_subnet) do ManualNetworkSubnet.parse( @@ -79,7 +77,7 @@ def cidr_ip(ip) context :add do def dynamic_reservation_with_ip(ip) - reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, JobNetwork.new(nil, nil, nil, network_without_static_pool)) + reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network_without_static_pool) reservation.resolve_ip(ip) ip_repo.add(reservation) @@ -95,7 +93,7 @@ def dynamic_reservation_with_ip(ip) context 'from Static to Dynamic' do it 'updates type of reservation' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -123,7 +121,7 @@ def dynamic_reservation_with_ip(ip) expect(original_address.static).to eq(false) network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -157,7 +155,7 @@ def dynamic_reservation_with_ip(ip) context 'when reservation changes network' do it 'updates network name' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + static_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(static_reservation) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -165,7 +163,7 @@ def dynamic_reservation_with_ip(ip) expect(original_address.static).to eq(true) expect(original_address.network_name).to eq(network.name) - static_reservation_on_another_network = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_other_network, '192.168.1.5') + static_reservation_on_another_network = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, other_network, '192.168.1.5') ip_repo.add(static_reservation_on_another_network) expect(Bosh::Director::Models::IpAddress.count).to eq(1) @@ -184,7 +182,7 @@ def dynamic_reservation_with_ip(ip) end network_spec['subnets'].first['static'] = ['192.168.1.5'] - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(reservation) saved_address = Bosh::Director::Models::IpAddress.order(:address_str).last @@ -200,8 +198,8 @@ def dynamic_reservation_with_ip(ip) network_spec['subnets'].first['static'] = ['192.168.1.5'] other_instance_model = FactoryBot.create(:models_instance, availability_zone: 'az-2') - original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') - new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, job_network, '192.168.1.5') + original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, network, '192.168.1.5') ip_repo.add(original_static_network_reservation) @@ -214,8 +212,8 @@ def dynamic_reservation_with_ip(ip) network_spec['subnets'].first['static'] = ['192.168.1.5'] other_instance_model = FactoryBot.create(:models_instance, availability_zone: 'az-2') - original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') - new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, job_network, '192.168.1.5') + original_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') + new_static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(other_instance_model, network, '192.168.1.5') ip_repo.add(original_static_network_reservation) @@ -230,7 +228,7 @@ def dynamic_reservation_with_ip(ip) it 'should succeed if it is reserved by the same instance' do network_spec['subnets'].first['static'] = ['192.168.1.5'] - static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + static_network_reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(static_network_reservation) @@ -242,7 +240,7 @@ def dynamic_reservation_with_ip(ip) end describe :allocate_dynamic_ip do - let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } context 'when there are no IPs reserved' do it 'returns the first in the range' do @@ -292,13 +290,13 @@ def dynamic_reservation_with_ip(ip) it 'returns first non-reserved IP' do network_spec['subnets'].first['static'] = ['192.168.1.2', '192.168.1.4'] - reservation_1 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.2') - reservation_2 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.4') + reservation_1 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2') + reservation_2 = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.4') ip_repo.add(reservation_1) ip_repo.add(reservation_2) - reservation_3 = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) + reservation_3 = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) ip_address = ip_repo.allocate_dynamic_ip(reservation_3, subnet) expect(ip_address).to eq(cidr_ip('192.168.1.3')) @@ -330,7 +328,7 @@ def dynamic_reservation_with_ip(ip) end context 'when a prefix is assigned to the subnet' do - let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } it 'reserves the prefix' do ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet_with_prefix) @@ -430,7 +428,7 @@ def fail_saving_ips(ips, fail_error) end describe :allocate_vip_ip do - let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } + let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } let(:network) do VipNetwork.parse( @@ -488,7 +486,7 @@ def fail_saving_ips(ips, fail_error) end context 'when there are no available vips' do - let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, job_network) } + let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } before do ip_repo.allocate_vip_ip(existing_reservation, network.subnets.first) @@ -536,7 +534,7 @@ def fail_saving_ips(ips, fail_error) before do network_spec['subnets'].first['static'] = ['192.168.1.5'] - reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, job_network, '192.168.1.5') + reservation = Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.5') ip_repo.add(reservation) end From 6ab0e04c2ce84515849486fbd3464b04f3bd5d32 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 09:48:42 +0200 Subject: [PATCH 05/17] fixes --- src/bosh-director/lib/bosh/director/errors.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bosh-director/lib/bosh/director/errors.rb b/src/bosh-director/lib/bosh/director/errors.rb index 80660c5e5b4..f601fb8d0f6 100644 --- a/src/bosh-director/lib/bosh/director/errors.rb +++ b/src/bosh-director/lib/bosh/director/errors.rb @@ -165,6 +165,7 @@ def self.err(error_code, response_code = BAD_REQUEST) NetworkReservationIpOutsideSubnet = err(130012) NetworkReservationIpReserved = err(130013) NetworkReservationVipMisconfigured = err(130014) + NetworkReservationInvalidPrefix = err(130015) # Manifest parsing: instance group section InstanceGroupMissingRelease = err(140001) From 58232e817a85e11d526e2163c635437a25740406 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 12:39:42 +0200 Subject: [PATCH 06/17] fixes --- .../bosh/director/deployment_plan/manual_network.rb | 10 ++++++---- .../lib/bosh/director/network_reservation.rb | 2 +- .../spec/unit/deployment_plan/instance_plan_spec.rb | 10 ++++++---- .../spec/unit/deployment_plan/manual_network_spec.rb | 4 ++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb index 9f03224e56a..ba77e03252a 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb @@ -52,16 +52,18 @@ def network_settings(reservation, default_properties = REQUIRED_DEFAULTS, availa "Can't generate network settings without an IP" end - ip_or_cidr = Bosh::Director::IpAddrOrCidr.new(reservation.ip) + ip_or_cidr = reservation.ip subnet = find_subnet_containing(reservation.ip) + unless subnet raise NetworkReservationInvalidIp, "Provided IP '#{ip_or_cidr}' does not belong to any subnet" end - - unless subnet.prefix == ip_or_cidr.prefix - raise NetworkReservationInvalidPRefix, "Subnet Prefix #{subnet.prefix} and ip reservation prefix #{ip_or_cidr.prefix} do not match" + + unless subnet.prefix.to_i == ip_or_cidr.prefix.to_i + raise NetworkReservationInvalidPrefix, "Subnet Prefix #{subnet.prefix} and ip reservation prefix #{ip_or_cidr.prefix} do not match" end + config = { "type" => "manual", "ip" => ip_or_cidr.to_s, diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 0ed0e5487c5..80a5a0dd671 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -64,7 +64,7 @@ def initialize(instance_model, network, ip, type) def resolve_ip(ip) # if !ip.to_s.match?(/\//) - @ip = IpAddrOrCidr.new(ip) + @ip = IpAddrOrCidr.new(ip.to_cidr_s) # else # if IpAddrOrCidr.new(ip).ipv6? # @ip = IpAddrOrCidr.new("#{ip}/128") diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb index 656ac4ac98e..ffce5b36a42 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb @@ -295,7 +295,7 @@ module Bosh::Director::DeploymentPlan allow(per_spec_logger).to receive(:debug) expect(per_spec_logger).to receive(:debug).with( 'networks_changed? obsolete reservations: ' \ - "[{type=dynamic, ip=10.0.0.5, network=existing-network, instance=#{instance_model}}]", + "[{type=dynamic, ip=10.0.0.5/32, network=existing-network, instance=#{instance_model}}]", ) instance_plan.networks_changed? end @@ -317,7 +317,7 @@ module Bosh::Director::DeploymentPlan allow(per_spec_logger).to receive(:debug) expect(per_spec_logger).to receive(:debug).with( 'networks_changed? desired reservations: ' \ - "[{type=dynamic, ip=10.0.0.5, network=existing-network, instance=#{instance_model}}]", + "[{type=dynamic, ip=10.0.0.5/32, network=existing-network, instance=#{instance_model}}]", ) instance_plan.networks_changed? end @@ -373,6 +373,7 @@ module Bosh::Director::DeploymentPlan 'a' => { 'type' => 'manual', 'ip' => '192.168.1.3', + 'prefix' => '32', 'netmask' => '255.255.255.0', 'cloud_properties' => {}, 'default' => %w[dns gateway], @@ -1545,6 +1546,7 @@ module Bosh::Director::DeploymentPlan 'a' => { 'type' => 'manual', 'ip' => '192.168.1.3', + 'prefix' => '32', 'netmask' => '255.255.255.0', 'cloud_properties' => {}, 'dns' => ['192.168.1.1', '192.168.1.2'], @@ -2061,8 +2063,8 @@ module Bosh::Director::DeploymentPlan let(:network_plans) { [plan1, plan2, plan3, plan4] } - let(:ip1) { IPAddr.new('192.168.1.25').to_i } - let(:ip2) { IPAddr.new('192.168.1.26').to_i } + let(:ip1) { IPAddr.new('192.168.1.25/32') } + let(:ip2) { IPAddr.new('192.168.1.26/32') } let(:ip_address1) { FactoryBot.create(:models_ip_address, address_str: ip1.to_s) } let(:ip_address2) { FactoryBot.create(:models_ip_address, address_str: ip2.to_s) } diff --git a/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb index 866d99667cf..4deadcea483 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manual_network_spec.rb @@ -163,7 +163,7 @@ context 'when a prefix is maintained for a subnet' do let(:network_spec) do - cloud_config_hash['networks'].first['prefix'] = '30' + cloud_config_hash['networks'].first['subnets'].first['prefix'] = '30' cloud_config_hash['networks'].first end @@ -176,7 +176,7 @@ expect(manual_network.network_settings(reservation)).to eq( 'type' => 'manual', - 'ip' => '192.168.1.2', + 'ip' => '192.168.1.0', 'prefix' => '30', 'netmask' => '255.255.255.0', 'cloud_properties' => {}, From 3b0a14abb104e2ce6ea8bd38dbc39ed87cf1eab0 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 14:36:46 +0200 Subject: [PATCH 07/17] fixes --- .../lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb | 4 ++-- .../steps/commit_instance_network_settings_step.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index deb16373fa4..1f19952eb5e 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -219,9 +219,9 @@ def validate_instance_and_update_reservation_type(instance_model, ip, ip_address end def save_ip(ip, reservation, is_static) - @logger.debug("Adding IP Address: #{ip} from reservation: #{reservation}") + @logger.debug("Adding IP Address: #{ip.to_cidr_s} from reservation: #{reservation}") ip_address = Bosh::Director::Models::IpAddress.new( - address_str: ip, + address_str: ip.to_cidr_s, network_name: reservation.network.name, task_id: Bosh::Director::Config.current_job.task_id, static: is_static, diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/steps/commit_instance_network_settings_step.rb b/src/bosh-director/lib/bosh/director/deployment_plan/steps/commit_instance_network_settings_step.rb index eb2dccc8742..2dad78d0d20 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/steps/commit_instance_network_settings_step.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/steps/commit_instance_network_settings_step.rb @@ -11,7 +11,7 @@ def perform(report) next if ip.nil? - ip_model = Models::IpAddress.find(address_str: ip.to_s) + ip_model = Models::IpAddress.find(address_str: ip.to_cidr_s) ip_model&.update(vm: report.vm) end end From ee13616c2466005280096e47e9a9c727df05860b Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 15:09:07 +0200 Subject: [PATCH 08/17] fixes --- .../lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb | 4 ++-- src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 1f19952eb5e..deb16373fa4 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -219,9 +219,9 @@ def validate_instance_and_update_reservation_type(instance_model, ip, ip_address end def save_ip(ip, reservation, is_static) - @logger.debug("Adding IP Address: #{ip.to_cidr_s} from reservation: #{reservation}") + @logger.debug("Adding IP Address: #{ip} from reservation: #{reservation}") ip_address = Bosh::Director::Models::IpAddress.new( - address_str: ip.to_cidr_s, + address_str: ip, network_name: reservation.network.name, task_id: Bosh::Director::Config.current_job.task_id, static: is_static, diff --git a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb index 086e30d0b05..98cbf967e87 100644 --- a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb +++ b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb @@ -22,7 +22,7 @@ def count end def to_cidr_s - "#{@ipaddr}/#{@ipaddr.prefix}" + "#{@ipaddr.to_string}/#{@ipaddr.prefix}" end private From 5dc5e87aff29cd4df89d3f999c2b4059859a3e72 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 15:20:53 +0200 Subject: [PATCH 09/17] fixes --- .../director/deployment_plan/ip_provider/ip_provider.rb | 2 +- .../bosh/director/deployment_plan/ip_provider/ip_repo.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb index 1ef1a07c11b..c6e2e17169f 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_provider.rb @@ -66,7 +66,7 @@ def reserve_manual(reservation) filter_subnet_by_instance_az(reservation).each do |subnet| if (ip = @ip_repo.allocate_dynamic_ip(reservation, subnet)) - @logger.debug("Reserving dynamic IP '#{ip}' for manual network '#{reservation.network.name}'") + @logger.debug("Reserving dynamic IP '#{ip.to_cidr_s}' for manual network '#{reservation.network.name}'") reservation.resolve_ip(ip) reservation.resolve_type(:dynamic) break diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index deb16373fa4..20591776493 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -23,7 +23,7 @@ def delete(ip) end def add(reservation) - ip_or_cidr = Bosh::Director::IpAddrOrCidr.new(reservation.ip.to_cidr_s) + ip_or_cidr = reservation.ip reservation_type = reservation.network.ip_type(ip_or_cidr) @@ -52,7 +52,7 @@ def allocate_dynamic_ip(reservation, subnet) retry end - @logger.debug("Allocated dynamic IP '#{ip_address}' for #{reservation.network.name}") + @logger.debug("Allocated dynamic IP '#{ip_address.to_cidr_s}' for #{reservation.network.name}") ip_address end @@ -187,9 +187,9 @@ def try_to_allocate_vip_ip(reservation, subnet) def reserve_with_instance_validation(instance_model, ip, reservation, is_static) # try to save IP first before validating its instance to prevent race conditions - save_ip(ip, reservation, is_static) + save_ip(ip.to_cidr_s, reservation, is_static) rescue IpFoundInDatabaseAndCanBeRetried - ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip.to_s) + ip_address = Bosh::Director::Models::IpAddress.first(address_str: ip.to_cidr_s) retry unless ip_address From 06085b1fe78267d8a0b9ceaf56b966adfa3a7aad Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 24 Apr 2025 16:05:03 +0200 Subject: [PATCH 10/17] fixes --- .../lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb | 2 +- src/bosh-director/lib/bosh/director/network_reservation.rb | 4 ++-- .../spec/unit/deployment_plan/instance_plan_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 20591776493..e6ff5c7dfed 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -29,7 +29,7 @@ def add(reservation) reserve_with_instance_validation( reservation.instance_model, - ip_or_cidr.to_cidr_s, + ip_or_cidr, reservation, reservation_type.eql?(:static), ) diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 80a5a0dd671..1fcbcf27006 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -28,7 +28,7 @@ class ExistingNetworkReservation < NetworkReservation def initialize(instance_model, network, ip, network_type) super(instance_model, network) - @ip = IpAddrOrCidr.new(ip) if ip + @ip = ip if ip @network_type = network_type @obsolete = network.instance_of? Bosh::Director::DeploymentPlan::Network end @@ -64,7 +64,7 @@ def initialize(instance_model, network, ip, type) def resolve_ip(ip) # if !ip.to_s.match?(/\//) - @ip = IpAddrOrCidr.new(ip.to_cidr_s) + @ip = IpAddrOrCidr.new(ip) # else # if IpAddrOrCidr.new(ip).ipv6? # @ip = IpAddrOrCidr.new("#{ip}/128") diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb index ffce5b36a42..9ed4113c381 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb @@ -120,7 +120,7 @@ module Bosh::Director::DeploymentPlan let(:network) { ManualNetwork.parse(network_spec, [availability_zone], per_spec_logger) } let(:reservation) do reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) - reservation.resolve_ip('192.168.1.3') + reservation.resolve_ip('192.168.1.3/32') reservation end let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } @@ -2035,7 +2035,7 @@ module Bosh::Director::DeploymentPlan describe '#remove_network_plans_for_ips' do let(:plan1) do reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) - reservation.resolve_ip('192.168.1.25') + reservation.resolve_ip('192.168.1.25/32') NetworkPlanner::Plan.new(reservation: reservation, existing: false, obsolete: true) end From 35845c835aca05ac32de2eb978faebec4504ba38 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Mon, 28 Apr 2025 14:14:13 +0200 Subject: [PATCH 11/17] unit test fixes + adaptions --- .../deployment_plan/dynamic_network.rb | 25 +++++++++-- .../deployment_plan/dynamic_network_subnet.rb | 5 ++- .../instance_network_reservations.rb | 2 +- .../deployment_plan/ip_provider/ip_repo.rb | 12 +----- .../deployment_plan/manual_network.rb | 3 +- .../bosh/director/deployment_plan/network.rb | 2 +- .../networks_to_static_ips.rb | 2 +- .../static_ips_availability_zone_picker.rb | 2 +- .../director/deployment_plan/vip_network.rb | 13 ++++-- .../deployment_plan/vip_network_subnet.rb | 12 ++++-- .../lib/bosh/director/models/ip_address.rb | 20 +++++++-- .../lib/bosh/director/models/vm.rb | 2 +- .../lib/bosh/director/network_reservation.rb | 2 +- .../deployments_controller_spec.rb | 10 ++--- .../deployment_plan/dynamic_network_spec.rb | 4 +- .../ip_provider/ip_provider_spec.rb | 6 +-- .../deployment_plan/network_settings_spec.rb | 42 +++++++------------ .../placement_planner/plan_spec.rb | 12 ++++-- ...tatic_ips_availability_zone_picker_spec.rb | 21 ++++++++++ ...mit_instance_network_settings_step_spec.rb | 12 +++--- .../steps/create_vm_step_spec.rb | 4 +- .../spec/unit/ip_addr_or_cidr_spec.rb | 2 +- .../spec/unit/jobs/vm_state_spec.rb | 40 +++++++++--------- .../spec/unit/models/ip_address_spec.rb | 4 +- src/bosh-director/spec/unit/models/vm_spec.rb | 8 ++-- .../spec/unit/orphaned_vm_deleter_spec.rb | 4 +- 26 files changed, 160 insertions(+), 111 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb index 0f845bbdff2..1902e91dc78 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network.rb @@ -17,20 +17,36 @@ def self.parse(network_spec, availability_zones, logger) if network_spec.has_key?('subnets') validate_network_has_no_key_while_subnets_present('dns', name, network_spec) validate_network_has_no_key_while_subnets_present('cloud_properties', name, network_spec) + validate_network_has_no_key_while_subnets_present('prefix', name, network_spec) subnets = network_spec['subnets'].map do |subnet_properties| name_servers = name_server_parser.parse(subnet_properties['name'], subnet_properties) cloud_properties = safe_property(subnet_properties, 'cloud_properties', class: Hash, default: {}) + prefix = safe_property(subnet_properties, 'prefix', class: Hash, default: {}) + if prefix.empty? || prefix.nil? + prefix = 32 + end subnet_availability_zones = parse_availability_zones(subnet_properties, availability_zones, name) - DynamicNetworkSubnet.new(name_servers, cloud_properties, subnet_availability_zones) + DynamicNetworkSubnet.new(name_servers, cloud_properties, subnet_availability_zones, prefix) end else cloud_properties = safe_property(network_spec, 'cloud_properties', class: Hash, default: {}) + prefix = safe_property(network_spec, 'prefix', class: Hash, default: {}) + if prefix.empty? || prefix.nil? + prefix = 32 + end name_servers = name_server_parser.parse(network_spec['name'], network_spec) - subnets = [DynamicNetworkSubnet.new(name_servers, cloud_properties, nil)] + subnets = [DynamicNetworkSubnet.new(name_servers, cloud_properties, nil, prefix)] end - new(name, subnets, logger) + unless subnets.empty? + prefix = subnets.first.prefix + else + prefix = 32 + end + + + new(name, subnets, logger, prefix) end def self.validate_network_has_no_key_while_subnets_present(key, name, network_spec) @@ -77,9 +93,10 @@ def self.check_validity_of_availability_zone(availability_zone, availability_zon end end - def initialize(name, subnets, logger) + def initialize(name, subnets, prefix, logger) super(name, logger) @subnets = subnets + @prefix = prefix end attr_reader :subnets diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network_subnet.rb index bcd6bb04f34..5867fd669cb 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/dynamic_network_subnet.rb @@ -1,13 +1,14 @@ module Bosh::Director module DeploymentPlan class DynamicNetworkSubnet - def initialize(dns, cloud_properties, availability_zone_names) + def initialize(dns, cloud_properties, availability_zone_names, prefix) @dns = dns @cloud_properties = cloud_properties @availability_zone_names = availability_zone_names.nil? ? nil : availability_zone_names + @prefix = prefix end - attr_reader :dns, :cloud_properties, :availability_zone_names + attr_reader :dns, :cloud_properties, :availability_zone_names, :prefix end end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb index de51af3e03b..ce0385f1353 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb @@ -24,7 +24,7 @@ def self.create_from_db(instance_model, deployment, logger) # Dynamic network reservations are not saved in DB, recreating from instance spec instance_model.spec.fetch('networks', []).each do |network_name, network_config| next unless network_config['type'] == 'dynamic' - reservations.add_existing(instance_model, deployment, network_name, network_config['ip'], network_config['type']) + reservations.add_existing(instance_model, deployment, network_name, Bosh::Director::IpAddrOrCidr.new("#{network_config['ip']}/99"), network_config['type']) end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index e6ff5c7dfed..35ce9138253 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -101,20 +101,12 @@ def try_to_allocate_dynamic_ip(reservation, subnet) if subnet.range.ipv6? addresses_we_cant_allocate.delete_if { |ipaddr| ipaddr.ipv4? } - if subnet.prefix.nil? - prefix = 128 - else - prefix = subnet.prefix - end else addresses_we_cant_allocate.delete_if { |ipaddr| ipaddr.ipv6? } - if subnet.prefix.nil? - prefix = 32 - else - prefix = subnet.prefix - end end + prefix = subnet.prefix + ip_address = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) unless subnet.range == ip_address || subnet.range.include?(ip_address) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb index ba77e03252a..90b9aeaf0ba 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb @@ -63,11 +63,10 @@ def network_settings(reservation, default_properties = REQUIRED_DEFAULTS, availa raise NetworkReservationInvalidPrefix, "Subnet Prefix #{subnet.prefix} and ip reservation prefix #{ip_or_cidr.prefix} do not match" end - config = { "type" => "manual", "ip" => ip_or_cidr.to_s, - "prefix" => subnet.prefix.to_s, + "prefix" => ip_or_cidr.prefix.to_s, "netmask" => subnet.netmask, "cloud_properties" => subnet.cloud_properties } diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/network.rb index 55d470b94b8..64fd643433a 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/network.rb @@ -89,7 +89,7 @@ def availability_zones end def prefix # for now the prefix should be considered the same for all subnets - @subnets.first.prefix + @subnets.first.prefix end end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb index 65847bf81cd..994ce32a727 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/networks_to_static_ips.rb @@ -91,7 +91,7 @@ def next_ip_for_network(network) def claim_in_az(ip, az_name) @networks_to_static_ips.each do |_, static_ips_to_azs| static_ips_to_azs.each do |static_ip_to_azs| - if static_ip_to_azs.ip == ip.to_i + if static_ip_to_azs.ip == ip static_ip_to_azs.claimed = true static_ip_to_azs.az_names = [az_name] end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb index 70fffeaee04..f1b015dd870 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/placement_planner/static_ips_availability_zone_picker.rb @@ -250,7 +250,7 @@ def create_network_plan_with_ip(instance_plan, network, ip_address) instance_az = instance_plan.desired_instance.az instance_az_name = instance_az.nil? ? nil : instance_az.name - ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address.to_i).az_names + ip_az_names = @networks_to_static_ips.find_by_network_and_ip(network, ip_address).az_names if ip_az_names.include?(instance_az_name) instance_plan.network_plans << @network_planner.network_plan_with_static_reservation(instance_plan, network, ip_address) end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network.rb index 4e1125523c1..b3dece66c5b 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network.rb @@ -6,7 +6,7 @@ class VipNetwork < NetworkWithSubnets # @return [Hash] Network cloud properties attr_reader :cloud_properties - attr_reader :subnets + attr_reader :subnets, :prefix def self.parse(network_spec, availability_zones, logger) name = safe_property(network_spec, 'name', class: String) @@ -15,8 +15,14 @@ def self.parse(network_spec, availability_zones, logger) DeploymentPlan::VipNetworkSubnet.parse(subnet_spec, name, availability_zones) end + unless subnets.empty? + prefix = subnets.first.prefix + else + prefix = 32 + end + cloud_properties = safe_property(network_spec, 'cloud_properties', class: Hash, default: {}) - new(name, cloud_properties, subnets, logger) + new(name, cloud_properties, subnets, prefix, logger) end ## @@ -25,10 +31,11 @@ def self.parse(network_spec, availability_zones, logger) # @param [Hash] network_spec parsed from the cloud config # @param [VipNetworkSubnet] vip network subnets parsed from the cloud config # @param [Logger] logger - def initialize(name, cloud_properties, subnets, logger) + def initialize(name, cloud_properties, subnets, prefix, logger) super(name, logger) @cloud_properties = cloud_properties @subnets = subnets + @prefix = prefix @logger = TaggedLogger.new(logger, 'network-configuration') end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb index 50e4f608127..000d0f96237 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb @@ -4,11 +4,16 @@ class VipNetworkSubnet < Subnet extend ValidationHelper extend IpUtil - attr_reader :static_ips, :availability_zone_names + attr_reader :static_ips, :availability_zone_names, :prefix def self.parse(subnet_spec, network_name, azs) static_ips = Set.new + prefix = safe_property(subnet_spec, 'prefix', optional: true) + if prefix.nil? + prefix = 32 + end + static_property = safe_property(subnet_spec, 'static', class: Array, default: []) each_ip(static_property) do |ip| static_ips.add(ip) @@ -16,12 +21,13 @@ def self.parse(subnet_spec, network_name, azs) availability_zone_names = parse_availability_zones(subnet_spec, network_name, azs) - new(static_ips, availability_zone_names) + new(static_ips, availability_zone_names, prefix) end - def initialize(static_ips, availability_zone_names) + def initialize(static_ips, availability_zone_names, prefix) @static_ips = static_ips @availability_zone_names = availability_zone_names + @prefix = prefix end def is_reservable?(ip) diff --git a/src/bosh-director/lib/bosh/director/models/ip_address.rb b/src/bosh-director/lib/bosh/director/models/ip_address.rb index 9a6fdc9b6b6..5dbfcf8e164 100644 --- a/src/bosh-director/lib/bosh/director/models/ip_address.rb +++ b/src/bosh-director/lib/bosh/director/models/ip_address.rb @@ -23,12 +23,16 @@ def info [ "#{instance.deployment.name}.#{instance.job}/#{instance.index}", network_name, - "#{Bosh::Director::IpAddrOrCidr.new(address_str)} (#{type})" + "#{Bosh::Director::IpAddrOrCidr.new(address_str).to_cidr_s} (#{type})" ].join(' - ') end def formatted_ip - Bosh::Director::IpAddrOrCidr.new(address).to_s + address.to_cidr_s + end + + def base_address + address.to_string end def type @@ -41,9 +45,19 @@ def address_int_and_prefix end def address + unless address_str.include?('/') || address_str.match?(/\A\d+\z/) + info_display = '' + begin + info_display = info + rescue StandardError + info_display = 'missing_info' + end + raise "Unexpected address '#{address_str}' (#{info_display})" + end + if address_str.include?('/') return Bosh::Director::IpAddrOrCidr.new(address_str) - else + else address_str.match?(/\A\d+\z/) ip = Bosh::Director::IpAddrOrCidr.new(address_str.to_i) if ip.ipv6? prefix = 132 diff --git a/src/bosh-director/lib/bosh/director/models/vm.rb b/src/bosh-director/lib/bosh/director/models/vm.rb index a7465642fc9..8c5f8bdb378 100644 --- a/src/bosh-director/lib/bosh/director/models/vm.rb +++ b/src/bosh-director/lib/bosh/director/models/vm.rb @@ -25,7 +25,7 @@ def ips private def manual_or_vip_ips - ip_addresses.map(&:formatted_ip) + ip_addresses.map(&:base_address) end def dynamic_ips diff --git a/src/bosh-director/lib/bosh/director/network_reservation.rb b/src/bosh-director/lib/bosh/director/network_reservation.rb index 1fcbcf27006..0ed0e5487c5 100644 --- a/src/bosh-director/lib/bosh/director/network_reservation.rb +++ b/src/bosh-director/lib/bosh/director/network_reservation.rb @@ -28,7 +28,7 @@ class ExistingNetworkReservation < NetworkReservation def initialize(instance_model, network, ip, network_type) super(instance_model, network) - @ip = ip if ip + @ip = IpAddrOrCidr.new(ip) if ip @network_type = network_type @obsolete = network.instance_of? Bosh::Director::DeploymentPlan::Network end diff --git a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb index ea7469d0b56..c2b215790db 100644 --- a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb +++ b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb @@ -1453,7 +1453,7 @@ def manifest_with_errand(deployment_name='errand') ip_addresses_params = { 'instance_id' => instance.id, 'task_id' => i.to_s, - 'address_str' => ip_to_i("1.2.#{i}.#{j}").to_s, + 'address_str' => ("1.2.#{i}.#{j}/32").to_s, 'vm_id' => vm.id, } Models::IpAddress.create(ip_addresses_params) @@ -1504,7 +1504,7 @@ def manifest_with_errand(deployment_name='errand') 'cid' => "cid-#{i}", 'instance_id' => instance.id, 'created_at' => time, - 'network_spec' => {'network1' => {'ip' => "1.2.3.#{i}"}}, + 'network_spec' => {'network1' => {'ip' => "1.2.3.#{i}", 'prefix' => '32'}}, } vm = Models::Vm.create(vm_params) @@ -1558,8 +1558,8 @@ def manifest_with_errand(deployment_name='errand') 'created_at' => time, 'instance_id' => instance.id, 'network_spec' => { - 'network1' => { 'ip' => network_spec_ip }, - 'network2' => { 'ip' => vip }, + 'network1' => { 'ip' => network_spec_ip, 'prefix' => '32' }, + 'network2' => { 'ip' => vip, 'prefix' => '32'}, }, } @@ -1567,7 +1567,7 @@ def manifest_with_errand(deployment_name='errand') instance.active_vm = vm ip_addresses_params = { - 'address_str' => ip_to_i(vip).to_s, + 'address_str' => "#{vip}/32", 'instance_id' => instance.id, 'task_id' => '1', 'vm_id' => vm.id, diff --git a/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb b/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb index 4a0a81ca9ec..adcf735148f 100644 --- a/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/dynamic_network_spec.rb @@ -537,7 +537,7 @@ describe :validate_reference_from_job do it 'returns true if job has a valid network spec' do - dynamic_network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('dynamic', [], logger) + dynamic_network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('dynamic', [], '32', logger) job_network_spec = {'name' => 'dynamic'} expect { @@ -547,7 +547,7 @@ context 'when network is dynamic but job network spec uses static ips' do it 'raises StaticIPNotSupportedOnDynamicNetwork' do - dynamic_network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('dynamic', [], logger) + dynamic_network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('dynamic', [], '32', logger) job_network_spec = { 'name' => 'dynamic', 'static_ips' => ['192.168.1.10'] diff --git a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb index bce788c6ddd..1f9543ec54a 100644 --- a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_provider_spec.rb @@ -161,7 +161,7 @@ module Bosh::Director::DeploymentPlan Bosh::Director::ExistingNetworkReservation.new( instance_model, manual_network, - '192.168.1.2', + '192.168.1.2/32', 'manual', ) end @@ -468,7 +468,7 @@ module Bosh::Director::DeploymentPlan it 'adds the ip address to the ip repository' do ip_provider.reserve(reservation) - expect(reservation.ip).to eq(IPAddr.new('1.1.1.1').to_i) + expect(reservation.ip).to eq('1.1.1.1') end end @@ -477,7 +477,7 @@ module Bosh::Director::DeploymentPlan it 'allocates an ip address for the reservation' do ip_provider.reserve(reservation) - expect(reservation.ip).to eq(IPAddr.new('1.1.1.1').to_i) + expect(reservation.ip).to eq('1.1.1.1') end context 'and there are no available vips' do diff --git a/src/bosh-director/spec/unit/deployment_plan/network_settings_spec.rb b/src/bosh-director/spec/unit/deployment_plan/network_settings_spec.rb index 8b5ca81aa51..b1048f0090e 100644 --- a/src/bosh-director/spec/unit/deployment_plan/network_settings_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/network_settings_spec.rb @@ -49,6 +49,10 @@ module Bosh::Director::DeploymentPlan let(:plan) { instance_double(Planner, name: 'fake-deployment') } let(:use_short_dns_addresses) { false } let(:use_link_dns_addresses) { false } + let(:dynamic_network) do + subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], {'foo' => 'bar'}, 'az-1', '32')] + DynamicNetwork.new('net_a', subnets, '32', per_spec_logger) + end before do allow_any_instance_of(Bosh::Director::DnsEncoder).to receive(:num_for_uuid) @@ -61,11 +65,6 @@ module Bosh::Director::DeploymentPlan describe '#to_hash' do context 'dynamic network' do - let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], {'foo' => 'bar'}, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) - end - let(:reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)] } it 'returns the network settings plus current IP, Netmask & Gateway from agent state' do @@ -130,10 +129,6 @@ module Bosh::Director::DeploymentPlan end context 'when it is a dynamic network' do - let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], {'foo' => 'bar'}, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) - end let(:reservations) {[Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)]} context 'when local dns is disabled' do @@ -251,10 +246,6 @@ module Bosh::Director::DeploymentPlan end context 'when it is a dynamic network' do - let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], {'foo' => 'bar'}, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) - end let(:reservations) {[Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)]} context 'when local dns is disabled' do @@ -291,11 +282,6 @@ module Bosh::Director::DeploymentPlan describe '#network_addresses' do context 'dynamic network' do - let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], {'foo' => 'bar'}, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) - end - let(:reservations) {[Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)]} context 'when DNS entries are requested' do it 'includes the network name and domain record' do @@ -379,8 +365,8 @@ module Bosh::Director::DeploymentPlan context 'when it is a dynamic network' do let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) + subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1', '32')] + DynamicNetwork.new('net_a', subnets, '32', per_spec_logger) end let(:reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)] } @@ -493,8 +479,8 @@ module Bosh::Director::DeploymentPlan context 'when it is a dynamic network' do let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) + subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1', '32')] + DynamicNetwork.new('net_a', subnets, '32', per_spec_logger) end let(:reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)] } @@ -531,13 +517,13 @@ module Bosh::Director::DeploymentPlan context 'when there are multiple network reservations' do let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) + subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1', '32')] + DynamicNetwork.new('net_a', subnets, '32', per_spec_logger) end let(:dynamic_network2) do - subnets = [DynamicNetworkSubnet.new(['9.8.7.6'], { 'bob' => 'joe' }, 'az-1')] - DynamicNetwork.new('net_b', subnets, per_spec_logger) + subnets = [DynamicNetworkSubnet.new(['9.8.7.6'], { 'bob' => 'joe' }, 'az-1', '32')] + DynamicNetwork.new('net_b', subnets, '32', per_spec_logger) end let(:reservations) do @@ -561,8 +547,8 @@ module Bosh::Director::DeploymentPlan context 'dynamic network' do let(:dynamic_network) do - subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1')] - DynamicNetwork.new('net_a', subnets, per_spec_logger) + subnets = [DynamicNetworkSubnet.new(['1.2.3.4'], { 'foo' => 'bar' }, 'az-1', '32')] + DynamicNetwork.new('net_a', subnets, '32', per_spec_logger) end let(:reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(nil, dynamic_network)] } diff --git a/src/bosh-director/spec/unit/deployment_plan/placement_planner/plan_spec.rb b/src/bosh-director/spec/unit/deployment_plan/placement_planner/plan_spec.rb index 3c7f5d6c11c..5e1fc2b2107 100644 --- a/src/bosh-director/spec/unit/deployment_plan/placement_planner/plan_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/placement_planner/plan_spec.rb @@ -59,7 +59,9 @@ module Bosh::Director::DeploymentPlan 192.168.1.12 192.168.1.13 192.168.1.14 - ] + ], + nil, nil, + '32' ), ManualNetworkSubnet.new( 'network_A', @@ -71,7 +73,9 @@ module Bosh::Director::DeploymentPlan 10.10.1.12 10.10.1.13 10.10.1.14 - ] + ], + nil, nil, + '32' ), ManualNetworkSubnet.new( 'network_A', @@ -83,7 +87,9 @@ module Bosh::Director::DeploymentPlan 10.0.1.12 10.0.1.13 10.0.1.14 - ] + ], + nil, nil, + '32' ), ] end diff --git a/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb b/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb index a759ac87272..9c24b503d33 100644 --- a/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/placement_planner/static_ips_availability_zone_picker_spec.rb @@ -377,6 +377,27 @@ def make_subnet_spec(range, static_ips, zone_names) end end + context 'when all existing instances match static IPs and AZs but the ips are still stored as integer format' do + let(:desired_instance_count) { 2 } + let(:static_ips) { ['192.168.1.10', '192.168.2.10'] } + let(:existing_instances) do + [ + existing_instance_with_az_and_ips('zone1', ['3232235786']), + existing_instance_with_az_and_ips('zone2', ['3232236042']), + ] + end + + it 'reuses existing instances' do + expect(new_instance_plans).to eq([]) + expect(obsolete_instance_plans).to eq([]) + expect(existing_instance_plans.size).to eq(2) + expect(existing_instance_plans[0].desired_instance.az.name).to eq('zone1') + expect(existing_instance_plans[0].network_plans.map(&:reservation).map(&:ip)).to eq([ip_to_i('192.168.1.10')]) + expect(existing_instance_plans[1].desired_instance.az.name).to eq('zone2') + expect(existing_instance_plans[1].network_plans.map(&:reservation).map(&:ip)).to eq([ip_to_i('192.168.2.10')]) + end + end + context 'when existing instance static IP was moved to another AZ' do let(:desired_instance_count) { 2 } let(:static_ips) { ['192.168.1.10', '192.168.2.10'] } diff --git a/src/bosh-director/spec/unit/deployment_plan/steps/commit_instance_network_settings_step_spec.rb b/src/bosh-director/spec/unit/deployment_plan/steps/commit_instance_network_settings_step_spec.rb index a80027ed712..4d4760368e9 100644 --- a/src/bosh-director/spec/unit/deployment_plan/steps/commit_instance_network_settings_step_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/steps/commit_instance_network_settings_step_spec.rb @@ -14,13 +14,13 @@ module Steps ] end - let(:existing_ip_address_string) { 'existing_ip_address_string' } - let(:obsolete_ip_address_string) { 'obsolete_ip_address_string' } - let(:desired_ip_address_string) { 'desired_ip_address_string' } + let(:existing_ip_address_string) { '1.1.1.1/32' } + let(:obsolete_ip_address_string) { '2.2.2.2/32' } + let(:desired_ip_address_string) { '3.3.3.3/32' } - let(:existing_reservation) { instance_double(NetworkReservation, ip: existing_ip_address_string) } - let(:obsolete_reservation) { instance_double(NetworkReservation, ip: obsolete_ip_address_string) } - let(:desired_reservation) { instance_double(NetworkReservation, ip: desired_ip_address_string) } + let(:existing_reservation) { instance_double(NetworkReservation, ip: Bosh::Director::IpAddrOrCidr.new(existing_ip_address_string)) } + let(:obsolete_reservation) { instance_double(NetworkReservation, ip: Bosh::Director::IpAddrOrCidr.new(obsolete_ip_address_string)) } + let(:desired_reservation) { instance_double(NetworkReservation, ip: Bosh::Director::IpAddrOrCidr.new(desired_ip_address_string)) } let!(:existing_ip_address) { FactoryBot.create(:models_ip_address, address_str: existing_ip_address_string) } let!(:obsolete_ip_address) { FactoryBot.create(:models_ip_address, address_str: obsolete_ip_address_string) } diff --git a/src/bosh-director/spec/unit/deployment_plan/steps/create_vm_step_spec.rb b/src/bosh-director/spec/unit/deployment_plan/steps/create_vm_step_spec.rb index e28048a5253..ef11db74bb9 100644 --- a/src/bosh-director/spec/unit/deployment_plan/steps/create_vm_step_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/steps/create_vm_step_spec.rb @@ -146,8 +146,8 @@ module Steps end let(:reservation) do - subnet = Bosh::Director::DeploymentPlan::DynamicNetworkSubnet.new('dns', network_cloud_properties, ['az-1']) - network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('name', [subnet], per_spec_logger) + subnet = Bosh::Director::DeploymentPlan::DynamicNetworkSubnet.new('dns', network_cloud_properties, ['az-1'], '32') + network = Bosh::Director::DeploymentPlan::DynamicNetwork.new('name', [subnet], '32', per_spec_logger) reservation = Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) reservation end diff --git a/src/bosh-director/spec/unit/ip_addr_or_cidr_spec.rb b/src/bosh-director/spec/unit/ip_addr_or_cidr_spec.rb index 9f097e31b99..ed272de954c 100644 --- a/src/bosh-director/spec/unit/ip_addr_or_cidr_spec.rb +++ b/src/bosh-director/spec/unit/ip_addr_or_cidr_spec.rb @@ -70,7 +70,7 @@ module Bosh::Director end context 'IPv6' do - let(:input) { 'fd00::/8' } + let(:input) { 'fd00:0000:0000:0000:0000:0000:0000:0000/8' } it 'returns a string representing the IP' do expect(ip_addr_or_cidr.to_cidr_s).to eq(input) diff --git a/src/bosh-director/spec/unit/jobs/vm_state_spec.rb b/src/bosh-director/spec/unit/jobs/vm_state_spec.rb index 36f54d641b6..304d9fcd4e8 100644 --- a/src/bosh-director/spec/unit/jobs/vm_state_spec.rb +++ b/src/bosh-director/spec/unit/jobs/vm_state_spec.rb @@ -55,12 +55,12 @@ def stub_agent_get_state_to_return_state_with_vitals FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('1.1.1.1').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.1').to_cidr_s, task_id: '12345', ) expect(agent).to receive(:get_state).with('full').and_return( 'vm_cid' => 'fake-vm-cid', - 'networks' => { 'test' => { 'ip' => '1.1.1.1' } }, + 'networks' => { 'test' => { 'ip' => '1.1.1.1/32' } }, 'agent_id' => 'fake-agent-id', 'job_state' => 'running', ) @@ -68,7 +68,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1']) + expect(status['ips']).to eq(['1.1.1.1/32']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -82,13 +82,13 @@ def stub_agent_get_state_to_return_state_with_vitals FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('1.1.1.1').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.1').to_cidr_s, task_id: '12345', ) FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('2.2.2.2').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('2.2.2.2').to_cidr_s, task_id: '12345', ) end @@ -98,13 +98,13 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1', '2.2.2.2']) + expect(status['ips']).to eq(['1.1.1.1/32', '2.2.2.2/32']) end end context "when 'ip_addresses' is empty for instance" do before do - vm.network_spec = { 'a' => { 'ip' => '3.3.3.3' }, 'b' => { 'ip' => '4.4.4.4' } } + vm.network_spec = { 'a' => { 'ip' => '3.3.3.3/32' }, 'b' => { 'ip' => '4.4.4.4/32' } } vm.save instance.spec = { 'networks' => { 'a' => { 'ip' => '1.1.1.1' }, 'b' => { 'ip' => '2.2.2.2' } } } instance.save @@ -116,7 +116,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['3.3.3.3', '4.4.4.4']) + expect(status['ips']).to eq(['3.3.3.3/32', '4.4.4.4/32']) end end @@ -125,19 +125,19 @@ def stub_agent_get_state_to_return_state_with_vitals FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('1.1.1.1').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.1').to_cidr_s, task_id: '12345', ) FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('2.2.2.2').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('2.2.2.2').to_cidr_s, task_id: '12345', ) vm.network_spec = { - 'a' => { 'ip' => '3.3.3.3' }, - 'b' => { 'ip' => '4.4.4.4' }, + 'a' => { 'ip' => '3.3.3.3/32' }, + 'b' => { 'ip' => '4.4.4.4/32' }, } vm.save @@ -158,8 +158,8 @@ def stub_agent_get_state_to_return_state_with_vitals status = JSON.parse(Models::Task.first(id: task.id).result_output) expect(status['ips']).to contain_exactly( - '1.1.1.1', '2.2.2.2', # Static - '3.3.3.3', '4.4.4.4', # Dynamic + '1.1.1.1/32', '2.2.2.2/32', # Static + '3.3.3.3/32', '4.4.4.4/32', # Dynamic ) end end @@ -168,7 +168,7 @@ def stub_agent_get_state_to_return_state_with_vitals FactoryBot.create(:models_ip_address, instance_id: instance.id, vm_id: vm.id, - address_str: IPAddr.new('1.1.1.1').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.1').to_cidr_s, task_id: '12345', ) stub_agent_get_state_to_return_state_with_vitals @@ -176,7 +176,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1']) + expect(status['ips']).to eq(['1.1.1.1/32']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -380,7 +380,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1']) + expect(status['ips']).to eq(['1.1.1.1/32']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -465,7 +465,7 @@ def stub_agent_get_state_to_return_state_with_vitals results.sort_by! { |r| r['ips'][0] } status = results[0] - expect(status['ips']).to eq(['1.1.1.1']) + expect(status['ips']).to eq(['1.1.1.1/32']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -475,7 +475,7 @@ def stub_agent_get_state_to_return_state_with_vitals { 'name' => 'fake-process-2', 'state' => 'failing' }]) status = results[1] - expect(status['ips']).to eq(['1.1.1.2']) + expect(status['ips']).to eq(['1.1.1.2/32']) expect(status['vm_cid']).to eq('fake-vm-cid-2') expect(status['active']).to eq(false) expect(status['agent_id']).to eq('other_agent_id') @@ -495,7 +495,7 @@ def stub_agent_get_state_to_return_state_with_vitals expect(results.length).to eq(1) status = JSON.parse(results[0]) - expect(status['ips']).to eq(['1.1.1.1']) + expect(status['ips']).to eq(['1.1.1.1/32']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') diff --git a/src/bosh-director/spec/unit/models/ip_address_spec.rb b/src/bosh-director/spec/unit/models/ip_address_spec.rb index 6531d13e5e5..968d0cb9375 100644 --- a/src/bosh-director/spec/unit/models/ip_address_spec.rb +++ b/src/bosh-director/spec/unit/models/ip_address_spec.rb @@ -5,7 +5,7 @@ module Bosh::Director::Models subject(:ip_address) do IpAddress.new(instance: instance, network_name: 'foonetwork', - address_str: IPAddr.new('10.10.0.1').to_i.to_s, + address_str: Bosh::Director::IpAddrOrCidr.new('10.10.0.1/32').to_cidr_s, task_id: 'fake-task-id', static: true, vm: vm) @@ -16,7 +16,7 @@ module Bosh::Director::Models context '#info' do it 'should display debugging information (job, index, network name and ip address)' do - expect(ip_address.info).to eq('foodeployment.foojob/1 - foonetwork - 10.10.0.1 (static)') + expect(ip_address.info).to eq('foodeployment.foojob/1 - foonetwork - 10.10.0.1/32 (static)') end end diff --git a/src/bosh-director/spec/unit/models/vm_spec.rb b/src/bosh-director/spec/unit/models/vm_spec.rb index b7c5a11f6ba..e3301d545bd 100644 --- a/src/bosh-director/spec/unit/models/vm_spec.rb +++ b/src/bosh-director/spec/unit/models/vm_spec.rb @@ -39,15 +39,15 @@ module Bosh::Director::Models end describe '#ips' do - let!(:ip_address) { FactoryBot.create(:models_ip_address, vm: vm, address_str: IPAddr.new('1.1.1.1').to_i.to_s) } - let!(:ip_address2) { FactoryBot.create(:models_ip_address, vm: vm, address_str: IPAddr.new('1.1.1.2').to_i.to_s) } + let!(:ip_address) { FactoryBot.create(:models_ip_address, vm: vm, address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.1/32').to_cidr_s) } + let!(:ip_address2) { FactoryBot.create(:models_ip_address, vm: vm, address_str: Bosh::Director::IpAddrOrCidr.new('1.1.1.2/32').to_cidr_s) } before do - vm.network_spec = { 'some' => { 'ip' => '1.1.1.3' } } + vm.network_spec = { 'some' => { 'ip' => '1.1.1.3/32' } } end it 'returns all ips for the vm' do - expect(vm.ips).to match_array(['1.1.1.1', '1.1.1.2', '1.1.1.3']) + expect(vm.ips).to match_array(['1.1.1.1/32', '1.1.1.2/32', '1.1.1.3/32']) end end end diff --git a/src/bosh-director/spec/unit/orphaned_vm_deleter_spec.rb b/src/bosh-director/spec/unit/orphaned_vm_deleter_spec.rb index e3d74802c8a..8fda2239eac 100644 --- a/src/bosh-director/spec/unit/orphaned_vm_deleter_spec.rb +++ b/src/bosh-director/spec/unit/orphaned_vm_deleter_spec.rb @@ -20,7 +20,7 @@ module Director Bosh::Director::Models::IpAddress.create( orphaned_vm: orphaned_vm1, network_name: 'my-manual-network', - address_str: IPAddr.new('127.0.0.2').to_i, + address_str: Bosh::Director::IpAddrOrCidr.new('127.0.0.2/32').to_cidr_s, task_id: 1, ) end @@ -38,7 +38,7 @@ module Director Bosh::Director::Models::IpAddress.create( orphaned_vm: orphaned_vm2, network_name: 'my-manual-network', - address_str: IPAddr.new('127.0.0.1').to_i, + address_str: Bosh::Director::IpAddrOrCidr.new('127.0.0.1/32').to_cidr_s, task_id: 1, ) end From 93d3f92a52a192310575c27f5b7c6fa664cad347 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Mon, 28 Apr 2025 14:35:18 +0200 Subject: [PATCH 12/17] return prefix in create_vm_response --- src/bosh-director/lib/bosh/director/models/vm.rb | 4 ++-- .../api/controllers/deployments_controller_spec.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/models/vm.rb b/src/bosh-director/lib/bosh/director/models/vm.rb index 8c5f8bdb378..c464ef76abf 100644 --- a/src/bosh-director/lib/bosh/director/models/vm.rb +++ b/src/bosh-director/lib/bosh/director/models/vm.rb @@ -25,11 +25,11 @@ def ips private def manual_or_vip_ips - ip_addresses.map(&:base_address) + ip_addresses.map(&:formatted_ip) end def dynamic_ips - network_spec.map { |_, network| network['ip'] } + network_spec.map { |_, network| "#{network['ip']}/#{network['prefix']}" } end end end diff --git a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb index c2b215790db..acbee621aaf 100644 --- a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb +++ b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb @@ -1356,7 +1356,7 @@ def manifest_with_errand(deployment_name='errand') 'state' => 'started', 'uuid' => "instance-#{i}", 'variable_set_id' => (Models::VariableSet.create(deployment: deployment).id), - 'spec' => {'networks' => {'network1' => {'ip' => "#{i}.#{i}.#{i}.#{i}"}}}, + 'spec' => {'networks' => {'network1' => {'ip' => "#{i}.#{i}.#{i}.#{i}", 'prefix' => '32'}}}, } instance_params['availability_zone'] = 'az0' if i == 0 @@ -1368,7 +1368,7 @@ def manifest_with_errand(deployment_name='errand') 'cid' => "cid-#{i}-#{j}", 'instance_id' => instance.id, 'created_at' => time, - 'network_spec' => {'network1' => {'ip' => "#{i}.#{i}.#{j}.#{j}"}}, + 'network_spec' => {'network1' => {'ip' => "#{i}.#{i}.#{j}.#{j}", 'prefix' => '32'}}, } vm = Models::Vm.create(vm_params) @@ -1396,7 +1396,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{instance_idx}", 'active' => vm_is_active, 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[instance_idx], - 'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}"], + 'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1478,7 +1478,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{instance_idx}", 'active' => vm_is_active, 'az' => { 0 => 'az0', 1 => 'az1', nil => nil }[instance_idx], - 'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}"], + 'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1529,7 +1529,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'active' => vm_is_active, 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], - 'ips' => ["1.2.3.#{i}"], + 'ips' => ["1.2.3.#{i}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1587,7 +1587,7 @@ def manifest_with_errand(deployment_name='errand') 'cid' => 'cid', 'id' => 'instance-id', 'index' => 0, - 'ips' => [vip, network_spec_ip], + 'ips' => ["#{vip}/32", "#{network_spec_ip}/32"], 'job' => 'job', 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, From 44b2184c1aa63a8a47dbaef89643f6e8469396b3 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Mon, 28 Apr 2025 17:05:50 +0200 Subject: [PATCH 13/17] more work --- .../director/api/controllers/deployments_controller.rb | 6 +++++- .../deployment_plan/instance_network_reservations.rb | 6 +++--- src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb | 2 +- src/bosh-director/lib/bosh/director/ip_util.rb | 6 +++++- .../lib/bosh/director/models/ip_address.rb | 5 ----- .../instance_network_reservations_spec.rb | 10 +++++----- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb b/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb index 745e526edc3..eb0dc8f37af 100644 --- a/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb +++ b/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb @@ -618,6 +618,9 @@ def create_instances_response_with_vm_expected(instances) end def create_vm_response(instance, vm) + cidr_ips = vm&.ips || [] + ips = cidr_ips.map{ | cidr_ip | cidr_ip.split('/')[0] } + { 'agent_id' => vm&.agent_id, 'cid' => vm&.cid, @@ -625,7 +628,8 @@ def create_vm_response(instance, vm) 'index' => instance.index, 'id' => instance.uuid, 'az' => instance.availability_zone, - 'ips' => vm&.ips || [], + 'ips' => ips, + 'ips_cidr' => cidr_ips, 'vm_created_at' => vm&.created_at&.utc&.iso8601, } end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb index ce0385f1353..b2273f91333 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb @@ -75,17 +75,17 @@ def find_network(deployment, cidr_ip, network_name, instance_model) ip_in_subnet = network.subnets.find { |snet| snet.is_reservable?(cidr_ip) } next unless ip_in_subnet - @logger.debug("Registering existing reservation with IP '#{format_ip(cidr_ip)}' for instance '#{instance_model}'"\ + @logger.debug("Registering existing reservation with IP '#{format_cidr_ip(cidr_ip)}' for instance '#{instance_model}'"\ "on network '#{network.name}'") return network end elsif network_match_on_name # dynamic and static vip - @logger.debug("Registering existing reservation with IP '#{format_ip(cidr_ip)}' for instance '#{instance_model}'"\ + @logger.debug("Registering existing reservation with IP '#{format_cidr_ip(cidr_ip)}' for instance '#{instance_model}'"\ "on network '#{network_name}'") return network_match_on_name end - @logger.debug("Failed to find network #{network_name} or a network with valid subnets for #{format_ip(cidr_ip)},"\ + @logger.debug("Failed to find network #{network_name} or a network with valid subnets for #{format_cidr_ip(cidr_ip)},"\ 'reservation will be marked as obsolete') Network.new(network_name, nil) end diff --git a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb index 98cbf967e87..73472506567 100644 --- a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb +++ b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb @@ -9,7 +9,7 @@ class IpAddrOrCidr def initialize(ip_or_cidr) @ipaddr = if ip_or_cidr.kind_of?(IpAddrOrCidr) - IPAddr.new(ip_or_cidr.to_s) + IPAddr.new(ip_or_cidr.to_cidr_s) elsif ip_or_cidr.kind_of?(Integer) IPAddr.new(ip_or_cidr, inet_type_for(ip_or_cidr)) else diff --git a/src/bosh-director/lib/bosh/director/ip_util.rb b/src/bosh-director/lib/bosh/director/ip_util.rb index 62c67f667c4..69910018904 100644 --- a/src/bosh-director/lib/bosh/director/ip_util.rb +++ b/src/bosh-director/lib/bosh/director/ip_util.rb @@ -21,7 +21,11 @@ def to_ipaddr(ip) # @param [Integer] ip Integer IP representation # @return [String] Human-readable IP representation def format_ip(ip) - to_ipaddr(ip).to_s + to_ipaddr(ip).to_cidr_s + end + + def format_cidr_ip(ip) + ip.to_cidr_s end def ip_address?(ip) diff --git a/src/bosh-director/lib/bosh/director/models/ip_address.rb b/src/bosh-director/lib/bosh/director/models/ip_address.rb index 5dbfcf8e164..0a8855f2776 100644 --- a/src/bosh-director/lib/bosh/director/models/ip_address.rb +++ b/src/bosh-director/lib/bosh/director/models/ip_address.rb @@ -39,11 +39,6 @@ def type static ? 'static' : 'dynamic' end - def address_int_and_prefix - address_and_prefix = address.split('/') - [Bosh::Director::IpAddrOrCidr.new(address_and_prefix[0]).to_i, address_and_prefix[1]] - end - def address unless address_str.include?('/') || address_str.match?(/\A\d+\z/) info_display = '' diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_network_reservations_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_network_reservations_spec.rb index 978dbb72031..033fecea4a4 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_network_reservations_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_network_reservations_spec.rb @@ -54,15 +54,15 @@ module Bosh::Director describe 'create_from_db' do context 'when there are IP addresses in db' do - let(:ip1) { IPAddr.new('192.168.0.1').to_i } - let(:ip2) { IPAddr.new('192.168.0.2').to_i } + let(:ip1) { Bosh::Director::IpAddrOrCidr.new('192.168.0.1/32') } + let(:ip2) { Bosh::Director::IpAddrOrCidr.new('192.168.0.2/32') } let(:ip_model1) do - FactoryBot.create(:models_ip_address, address_str: ip1.to_s, instance: instance_model, network_name: 'fake-network') + FactoryBot.create(:models_ip_address, address_str: ip1.to_cidr_s, instance: instance_model, network_name: 'fake-network') end let(:ip_model2) do - FactoryBot.create(:models_ip_address, address_str: ip2.to_s, instance: instance_model, network_name: 'fake-network') + FactoryBot.create(:models_ip_address, address_str: ip2.to_cidr_s, instance: instance_model, network_name: 'fake-network') end context 'when there is a last VM with IP addresses' do @@ -209,7 +209,7 @@ module Bosh::Director end let(:dynamic_network) do - DeploymentPlan::DynamicNetwork.new('dynamic-network', [], per_spec_logger) + DeploymentPlan::DynamicNetwork.new('dynamic-network', [], '32', per_spec_logger) end before do allow(deployment).to receive(:network).with('dynamic-network').and_return(dynamic_network) From 802b5ba55a03bbc59d296c034a8e24b49252d0a0 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Tue, 29 Apr 2025 12:58:16 +0200 Subject: [PATCH 14/17] introduce static ip check for prefixes --- .../deployment_plan/ip_provider/ip_repo.rb | 37 ++++++------------- .../deployment_plan/manual_network_subnet.rb | 12 +++++- src/bosh-director/lib/bosh/director/errors.rb | 1 + .../lib/bosh/director/ip_addr_or_cidr.rb | 18 +++++++++ .../manual_network_subnet_spec.rb | 15 ++++++++ 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 35ce9138253..460ee29fcb1 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -79,19 +79,10 @@ def allocate_vip_ip(reservation, subnet) def all_ip_addresses Bosh::Director::Models::IpAddress.select(:address_str).all.map { |a| a.address } end - - def get_prefix(ip) - if Bosh::Director::IpAddrOrCidr.new(ip).ipv6? - 128 - else - 32 - end - end def try_to_allocate_dynamic_ip(reservation, subnet) addresses_in_use = Set.new(all_ip_addresses) - first_range_address = Bosh::Director::IpAddrOrCidr.new(subnet.range.to_range.first.to_i - 1) addresses_we_cant_allocate = addresses_in_use @@ -133,24 +124,18 @@ def try_to_allocate_dynamic_ip(reservation, subnet) def find_next_available_ip(ip_entries, first_range_address, prefix) filtered_ips = ip_entries.sort_by { |ip| ip.to_i }.reject { |ip| ip.to_i < first_range_address.to_i } #remove ips that are below subnet range - if prefix == 32 || prefix == 128 - available_ip = filtered_ips.find { |ip| !filtered_ips.include?(Bosh::Director::IpAddrOrCidr.new(ip.to_i + 1)) }.to_i + 1 + current_ip = Bosh::Director::IpAddrOrCidr.new(first_range_address.to_i + 1) + found = false - found_cidr = Bosh::Director::IpAddrOrCidr.new("#{Bosh::Director::IpAddrOrCidr.new(available_ip)}/#{prefix}") - else - current_ip = Bosh::Director::IpAddrOrCidr.new(first_range_address.to_i + 1) - found = false - - while found == false - current_prefix = Bosh::Director::IpAddrOrCidr.new("#{current_ip}/#{prefix}") - - if filtered_ips.any? { |ip| current_prefix.include?(ip) } - current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count) - filtered_ips.reject{ |ip| ip.to_i < current_ip.to_i } - else - found_cidr = current_prefix - found = true - end + while found == false + current_prefix = Bosh::Director::IpAddrOrCidr.new("#{current_ip}/#{prefix}") + + if filtered_ips.any? { |ip| current_prefix.include?(ip) } + current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count) + filtered_ips.reject{ |ip| ip.to_i < current_ip.to_i } + else + found_cidr = current_prefix + found = true end end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb index 143d4b0d2a2..1fbbb33b85e 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb @@ -52,7 +52,7 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) each_ip(reserved_property) do |ip| unless range.include?(ip) - raise NetworkReservedIpOutOfRange, "Reserved IP '#{format_ip(ip)}' is out of " \ + raise NetworkReservedIpOutOfRange, "Reserved IP '#{to_ipaddr(ip)}' is out of " \ "network '#{network_name}' range" end @@ -87,6 +87,16 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) if range.prefix > prefix.to_i raise NetworkPrefixSizeTooBig, "Prefix size '#{prefix}' is larger than range prefix '#{range.prefix}'" end + # if a prefix is provided the static ips can only be the base_addresses of the prefix otherwise we through an error + static_ips.each do |static_ip| + range.each_base_address(prefix) do |base_address_int| + if static_ip == base_address_int + break + elsif static_ip < base_address_int + raise NetworkPrefixStaticIpNotBaseAddress, "Static IP '#{to_ipaddr(static_ip)}' is not a base address of the prefix '#{prefix}'" + end + end + end end end diff --git a/src/bosh-director/lib/bosh/director/errors.rb b/src/bosh-director/lib/bosh/director/errors.rb index f601fb8d0f6..4756d9904ce 100644 --- a/src/bosh-director/lib/bosh/director/errors.rb +++ b/src/bosh-director/lib/bosh/director/errors.rb @@ -227,6 +227,7 @@ def self.err(error_code, response_code = BAD_REQUEST) NetworkNotFoundError = err(16012) NetworkPrefixSizeTooBig = err(16013) NetworkPrefixSizesDiffer = err(16014) + NetworkPrefixStaticIpNotBaseAddress = err(16015) # ResourcePool ResourcePoolUnknownNetwork = err(170001) diff --git a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb index 73472506567..581c55af1c2 100644 --- a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb +++ b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb @@ -17,6 +17,24 @@ def initialize(ip_or_cidr) end end + def each_base_address(prefix_length) + # Determine the base address for the given prefix_length + # We assume the prefix_length is a valid integer within the subnet + if @ipaddr.ipv4? + bits = 32 + elsif @ipaddr.ipv6? + bits = 128 + end + step_size = 2**(bits - prefix_length) # Calculate number of addresses per subnet + base_address_int = @ipaddr.to_i + + # Yield each base address in this network + while base_address_int <= @ipaddr.to_range.last.to_i + yield base_address_int + base_address_int += step_size + end + end + def count (@ipaddr.to_range.last.to_i - @ipaddr.to_range.first.to_i) + 1 end diff --git a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb index 651e6d13b1f..a9fb8067077 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb @@ -361,6 +361,21 @@ def make_managed_subnet(properties, availability_zones) )}.to raise_error(Bosh::Director::NetworkPrefixSizeTooBig, "Prefix size '23' is larger than range prefix '24'") end + + it 'should fail if a static ip provided is not a base address of the prefix' do + expect { + make_subnet( + { + 'range' => '192.168.0.0/24', + 'gateway' => '192.168.0.254', + 'static' => ['192.168.0.64','192.168.0.128','192.168.0.191'], + 'cloud_properties' => {'foo' => 'bar'}, + 'prefix' => 26 + }, + [], + )}.to raise_error(Bosh::Director::NetworkPrefixStaticIpNotBaseAddress, + "Static IP '192.168.0.191' is not a base address of the prefix '26'") + end end describe :overlaps? do From 4b6f351f2ceb2638a3cd4e51d18e7b246d793729 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Wed, 30 Apr 2025 09:29:36 +0200 Subject: [PATCH 15/17] export ips_cidr in different field --- .../api/controllers/deployments_controller.rb | 7 +- .../compilation_instance_pool.rb | 1 + .../deployment_plan/ip_provider/ip_repo.rb | 32 +++----- .../deployment_plan/manual_network.rb | 5 +- .../deployment_plan/manual_network_subnet.rb | 8 +- .../deployment_plan/vip_network_subnet.rb | 2 +- .../lib/bosh/director/jobs/vm_state.rb | 1 + .../lib/bosh/director/models/vm.rb | 4 + .../deployments_controller_spec.rb | 82 ++++++++++++++++++- .../deployment_plan/instance_plan_spec.rb | 33 +++----- .../ip_provider/ip_repo_spec.rb | 47 ++++++++++- .../manual_network_subnet_spec.rb | 18 ++-- .../reservation_reconciler_spec.rb | 56 +++++++------ 13 files changed, 210 insertions(+), 86 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb b/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb index eb0dc8f37af..b0dc7c5848b 100644 --- a/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb +++ b/src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb @@ -618,9 +618,6 @@ def create_instances_response_with_vm_expected(instances) end def create_vm_response(instance, vm) - cidr_ips = vm&.ips || [] - ips = cidr_ips.map{ | cidr_ip | cidr_ip.split('/')[0] } - { 'agent_id' => vm&.agent_id, 'cid' => vm&.cid, @@ -628,8 +625,8 @@ def create_vm_response(instance, vm) 'index' => instance.index, 'id' => instance.uuid, 'az' => instance.availability_zone, - 'ips' => ips, - 'ips_cidr' => cidr_ips, + 'ips' => vm&.ips || [], + 'ips_cidr' => vm&.ips_cidr || [], 'vm_created_at' => vm&.created_at&.utc&.iso8601, } end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/compilation_instance_pool.rb b/src/bosh-director/lib/bosh/director/deployment_plan/compilation_instance_pool.rb index 333218c53f4..882039a607e 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/compilation_instance_pool.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/compilation_instance_pool.rb @@ -189,6 +189,7 @@ def create_instance_plan(stemcell) compilation_network = @deployment_plan.network(@deployment_plan.compilation.network_name) reservation = DesiredNetworkReservation.new_dynamic(instance.model, compilation_network) + @logger.debug("Creating new dynamic reservation #{reservation.inspect} for instance '#{instance}' and compile instance group '#{compile_instance_group}'") desired_instance = DeploymentPlan::DesiredInstance.new(compile_instance_group) instance_plan = DeploymentPlan::InstancePlan.new( existing_instance: instance.model, diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 460ee29fcb1..81d9f29f846 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -98,27 +98,16 @@ def try_to_allocate_dynamic_ip(reservation, subnet) prefix = subnet.prefix - ip_address = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) + ip_address_cidr = find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) - unless subnet.range == ip_address || subnet.range.include?(ip_address) - raise NoMoreIPsAvailableAndStopRetrying + if !(subnet.range == ip_address_cidr || subnet.range.include?(ip_address_cidr)) || + subnet.range.to_range.last.to_i < (ip_address_cidr.to_i + ip_address_cidr.count) + raise NoMoreIPsAvailableAndStopRetrying end - unless prefix - if ip_address.ipv6? - host_bits = 128 - prefix - else - host_bits = 32 - prefix - end - no_of_addresses = 2**host_bits - if subnet.range.last < (ip_address.to_i + no_of_addresses) - raise NoMoreIPsAvailableAndStopRetrying - end - end + save_ip(ip_address_cidr.to_cidr_s, reservation, false) - save_ip(ip_address.to_cidr_s, reservation, false) - - ip_address + ip_address_cidr end def find_next_available_ip(ip_entries, first_range_address, prefix) @@ -131,8 +120,13 @@ def find_next_available_ip(ip_entries, first_range_address, prefix) current_prefix = Bosh::Director::IpAddrOrCidr.new("#{current_ip}/#{prefix}") if filtered_ips.any? { |ip| current_prefix.include?(ip) } - current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count) - filtered_ips.reject{ |ip| ip.to_i < current_ip.to_i } + filtered_ips.reject! { |ip| ip.to_i < current_prefix.to_i } + actual_ip_prefix = filtered_ips.first.count + if actual_ip_prefix > current_prefix.count + current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + actual_ip_prefix) + else + current_ip = Bosh::Director::IpAddrOrCidr.new(current_ip.to_i + current_prefix.count) + end else found_cidr = current_prefix found = true diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb index 90b9aeaf0ba..74a81c1aa65 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb @@ -27,16 +27,17 @@ def self.parse(network_spec, availability_zones, logger) subnets << new_subnet end validate_all_subnets_use_azs(subnets, name) - new(name, subnets, logger, managed) + new(name, subnets, prefix, logger, managed) end def managed? @managed end - def initialize(name, subnets, logger, managed = false) + def initialize(name, subnets, prefix, logger, managed = false) super(name, TaggedLogger.new(logger, 'network-configuration')) @subnets = subnets + @prefix = prefix @managed = managed end diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb index 1fbbb33b85e..a6661eab774 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb @@ -148,7 +148,13 @@ def overlaps?(subnet) end def is_reservable?(ip) - range.include?(ip) && !restricted_ips.include?(ip.to_i) + restricted_ips.reject! { |restricted_ip| restricted_ip.to_i < ip.to_range.first.to_i } + restricted_ips.reject! { |restricted_ip| restricted_ip.to_i > ip.to_range.last.to_i } + restricted_ips_contain_ip_from_prefix = false + if !restricted_ips.empty? + restricted_ips_contain_ip_from_prefix = true + end + range.include?(ip.to_cidr_s) && !restricted_ips_contain_ip_from_prefix end def self.parse_properties_from_database(network_name, subnet_name) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb index 000d0f96237..6e077b4e228 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/vip_network_subnet.rb @@ -31,7 +31,7 @@ def initialize(static_ips, availability_zone_names, prefix) end def is_reservable?(ip) - @static_ips.include?(ip) + @static_ips.include?(ip.to_i) end end end diff --git a/src/bosh-director/lib/bosh/director/jobs/vm_state.rb b/src/bosh-director/lib/bosh/director/jobs/vm_state.rb index efa1d4cef9a..6d487355529 100644 --- a/src/bosh-director/lib/bosh/director/jobs/vm_state.rb +++ b/src/bosh-director/lib/bosh/director/jobs/vm_state.rb @@ -58,6 +58,7 @@ def process_vm_for_instance(instance, vm) disk_cid: instance.managed_persistent_disk_cid, disk_cids: instance.active_persistent_disks.collection.map { |d| d.model.disk_cid }, ips: vm&.ips || [], + ips_cidr: vm&.ips_cidr || [], agent_id: vm&.agent_id, job_name: instance.job, index: instance.index, diff --git a/src/bosh-director/lib/bosh/director/models/vm.rb b/src/bosh-director/lib/bosh/director/models/vm.rb index c464ef76abf..a2f82b7c3ea 100644 --- a/src/bosh-director/lib/bosh/director/models/vm.rb +++ b/src/bosh-director/lib/bosh/director/models/vm.rb @@ -19,6 +19,10 @@ def network_spec=(spec) end def ips + ips_cidr.map{ | cidr_ip | cidr_ip.split('/')[0] } + end + + def ips_cidr manual_or_vip_ips.concat(dynamic_ips).uniq end diff --git a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb index acbee621aaf..1ca6a4c1817 100644 --- a/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb +++ b/src/bosh-director/spec/unit/api/controllers/deployments_controller_spec.rb @@ -1396,7 +1396,8 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{instance_idx}", 'active' => vm_is_active, 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[instance_idx], - 'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}/32"], + 'ips' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}"], + 'ips_cidr' => ["#{instance_idx}.#{instance_idx}.#{vm_by_instance}.#{vm_by_instance}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1478,7 +1479,71 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{instance_idx}", 'active' => vm_is_active, 'az' => { 0 => 'az0', 1 => 'az1', nil => nil }[instance_idx], - 'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"], + 'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}"], + 'ips_cidr' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"], + 'vm_created_at' => time.utc.iso8601, + 'permanent_nats_credentials' => false, + ) + end + end + + it 'returns ip addresses with prefix as ips_cidr for a vm' do + 9.times do |i| + instance_params = { + 'deployment_id' => deployment.id, + 'job' => "job-#{i}", + 'index' => i, + 'state' => 'started', + 'uuid' => "instance-#{i}", + 'variable_set_id' => (Models::VariableSet.create(deployment: deployment).id) + } + + instance_params['availability_zone'] = 'az0' if i == 0 + instance_params['availability_zone'] = 'az1' if i == 1 + instance = Models::Instance.create(instance_params) + + vm_params = { + 'agent_id' => "agent-#{i}", + 'cid' => "cid-#{i}", + 'instance_id' => instance.id, + 'created_at' => time, + } + + vm = Models::Vm.create(vm_params) + + if j == 0 + instance.active_vm = vm + end + + ip_addresses_params = { + 'instance_id' => instance.id, + 'task_id' => i.to_s, + 'address_str' => ("1.2.#{i}.#{j}/32").to_s, + 'vm_id' => vm.id, + } + Models::IpAddress.create(ip_addresses_params) + end + + get '/test_deployment/vms' + + expect(last_response.status).to eq(200) + body = JSON.parse(last_response.body) + expect(body.size).to eq(18) + + body.sort_by { |instance| instance['agent_id'] }.each_with_index do |instance_with_vm, i| + instance_idx = i / 2 + vm_by_instance = i % 2 + vm_is_active = vm_by_instance == 0 + expect(instance_with_vm).to eq( + 'agent_id' => "agent-#{instance_idx}-#{vm_by_instance}", + 'job' => "job-#{instance_idx}", + 'index' => instance_idx, + 'cid' => "cid-#{instance_idx}-#{vm_by_instance}", + 'id' => "instance-#{instance_idx}", + 'active' => vm_is_active, + 'az' => { 0 => 'az0', 1 => 'az1', nil => nil }[instance_idx], + 'ips' => ["1.2.#{instance_idx}.#{vm_by_instance}"], + 'ips_cidr' => ["1.2.#{instance_idx}.#{vm_by_instance}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1529,7 +1594,8 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'active' => vm_is_active, 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], - 'ips' => ["1.2.3.#{i}/32"], + 'ips' => ["1.2.3.#{i}"], + 'ips_cidr' => ["1.2.3.#{i}/32"], 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, ) @@ -1587,7 +1653,8 @@ def manifest_with_errand(deployment_name='errand') 'cid' => 'cid', 'id' => 'instance-id', 'index' => 0, - 'ips' => ["#{vip}/32", "#{network_spec_ip}/32"], + 'ips' => ["#{vip}", "#{network_spec_ip}"], + 'ips_cidr' => ["#{vip}/32", "#{network_spec_ip}/32"], 'job' => 'job', 'vm_created_at' => time.utc.iso8601, 'permanent_nats_credentials' => false, @@ -1666,6 +1733,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => time.utc.iso8601, 'expects_vm' => true ) @@ -1678,6 +1746,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => true ) @@ -1725,6 +1794,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => true ) @@ -1763,6 +1833,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => "instance-#{i}", 'az' => {0 => 'az0', 1 => 'az1', nil => nil}[i], 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => true ) @@ -1806,6 +1877,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => 'instance-1', 'az' => nil, 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => true ) @@ -1830,6 +1902,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => 'instance-1', 'az' => nil, 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => false ) @@ -1856,6 +1929,7 @@ def manifest_with_errand(deployment_name='errand') 'id' => 'instance-1', 'az' => nil, 'ips' => [], + 'ips_cidr' => [], 'vm_created_at' => nil, 'expects_vm' => false ) diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb index 9ed4113c381..32d83688c4d 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_plan_spec.rb @@ -123,7 +123,7 @@ module Bosh::Director::DeploymentPlan reservation.resolve_ip('192.168.1.3/32') reservation end - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } + let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az'], '32') } let(:network_plans) do [ NetworkPlanner::Plan.new(reservation: reservation, existing: true), @@ -205,8 +205,7 @@ module Bosh::Director::DeploymentPlan describe 'networks_changed?' do context 'when the instance plan has desired network plans' do - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do [ @@ -335,8 +334,7 @@ module Bosh::Director::DeploymentPlan describe 'network_settings_changed?' do context 'when the instance plan has desired network plans' do - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do [ @@ -401,7 +399,7 @@ module Bosh::Director::DeploymentPlan }, } end - let(:subnet) { DynamicNetworkSubnet.new('8.8.8.8', subnet_cloud_properties, ['foo-az']) } + let(:subnet) { DynamicNetworkSubnet.new('8.8.8.8', subnet_cloud_properties, ['foo-az'], '32') } let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] } let(:subnet_cloud_properties) do {} @@ -518,8 +516,8 @@ module Bosh::Director::DeploymentPlan end context 'when the vm type name has changed' do - let(:subnet) { DynamicNetworkSubnet.new(['10.0.0.1'], {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) } + let(:subnet) { DynamicNetworkSubnet.new(['10.0.0.1'], {}, ['foo-az'], '32') } + let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] } @@ -561,8 +559,7 @@ module Bosh::Director::DeploymentPlan context 'when the network has changed' do # everything else should be the same let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') } - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do @@ -892,8 +889,7 @@ module Bosh::Director::DeploymentPlan end context 'when the vm type name has changed' do - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] } @@ -935,8 +931,7 @@ module Bosh::Director::DeploymentPlan context 'when the network has changed' do # everything else should be the same let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') } - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do @@ -1040,8 +1035,7 @@ module Bosh::Director::DeploymentPlan end context 'when the vm type name has changed' do - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) { [NetworkPlanner::Plan.new(reservation: existing_reservation, existing: true)] } @@ -1124,8 +1118,7 @@ module Bosh::Director::DeploymentPlan context 'when the network has changed' do # everything else should be the same let(:availability_zone) { AvailabilityZone.new('foo-az', 'old' => 'value') } - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], per_spec_logger) } + let(:existing_network) { DynamicNetwork.new('existing-network', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do @@ -1278,8 +1271,8 @@ module Bosh::Director::DeploymentPlan end describe '#persist_current_spec' do - let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az']) } - let(:existing_network) { DynamicNetwork.new('a', [subnet], per_spec_logger) } + let(:subnet) { DynamicNetworkSubnet.new('10.0.0.1', {}, ['foo-az'], '32') } + let(:existing_network) { DynamicNetwork.new('a', [subnet], '32', per_spec_logger) } let(:existing_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(existing_instance, existing_network) } let(:network_plans) do diff --git a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb index 1967452f05d..47cee418b16 100644 --- a/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/ip_provider/ip_repo_spec.rb @@ -329,12 +329,56 @@ def dynamic_reservation_with_ip(ip) context 'when a prefix is assigned to the subnet' do let(:reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } - it 'reserves the prefix' do + it 'reserves the prefix address' do ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet_with_prefix) expect(ip_address).to eq(cidr_ip('192.168.1.2/31')) end + it 'reserves the next available prefix address' do + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, other_subnet) + + expected_ip_address = cidr_ip('192.168.1.2') + expect(ip_address).to eq(expected_ip_address) + + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet_with_prefix) + + expected_ip_address = cidr_ip('192.168.1.4/31') + expect(ip_address).to eq(expected_ip_address) + + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, other_subnet) + + expected_ip_address = cidr_ip('192.168.1.3') + expect(ip_address).to eq(expected_ip_address) + + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, other_subnet) + + expected_ip_address = cidr_ip('192.168.1.6') + expect(ip_address).to eq(expected_ip_address) + end + + it 'should stop retrying and return nil if no sufficient range is available' do + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, other_subnet) + + expected_ip_address = cidr_ip('192.168.1.2') + expect(ip_address).to eq(expected_ip_address) + + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet_with_prefix) + + expected_ip_address = cidr_ip('192.168.1.4/31') + expect(ip_address).to eq(expected_ip_address) + + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, other_subnet) + + expected_ip_address = cidr_ip('192.168.1.3') + expect(ip_address).to eq(expected_ip_address) + + expect do + ip_address = ip_repo.allocate_dynamic_ip(other_reservation, subnet_with_prefix) + expect(ip_address).to be_nil + end.not_to(change { Bosh::Director::Models::IpAddress.count }) + end + it 'should stop retrying and return nil if no sufficient range is available' do expect do ip = ip_repo.allocate_dynamic_ip(reservation, subnet_with_too_big_prefix) @@ -343,7 +387,6 @@ def dynamic_reservation_with_ip(ip) end end - context 'when reserving IP fails' do def fail_saving_ips(ips, fail_error) original_saves = {} diff --git a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb index a9fb8067077..ef23ba23195 100644 --- a/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/manual_network_subnet_spec.rb @@ -349,7 +349,7 @@ def make_managed_subnet(properties, availability_zones) it 'should fail if the prefix size is larger than the range' do - expect { + expect { make_subnet( { 'range' => '192.168.0.0/24', @@ -445,26 +445,34 @@ def make_managed_subnet(properties, availability_zones) let(:reserved) { ['192.168.0.50-192.168.0.60'] } it 'returns false' do - expect(subnet.is_reservable?(IPAddr.new('192.168.0.55'))).to be_falsey + expect(subnet.is_reservable?(Bosh::Director::IpAddrOrCidr.new('192.168.0.55'))).to be_falsey end end context 'when subnet reserved does not include IP' do it 'returns true' do - expect(subnet.is_reservable?(IPAddr.new('192.168.0.55'))).to be_truthy + expect(subnet.is_reservable?(Bosh::Director::IpAddrOrCidr.new('192.168.0.55'))).to be_truthy + end + end + + context 'when subnet reserved does not include any address of a cidr' do + it 'returns true' do + let(:reserved) { ['192.168.0.50-192.168.0.60'] } + + expect(subnet.is_reservable?(Bosh::Director::IpAddrOrCidr.new('192.168.0.55'))).to be_truthy end end end context 'when subnet range does not include IP' do it 'returns false' do - expect(subnet.is_reservable?(IPAddr.new('192.168.10.55'))).to be_falsey + expect(subnet.is_reservable?(Bosh::Director::IpAddrOrCidr.new('192.168.10.55'))).to be_falsey end end context 'when subnet range is not the same IP version' do it 'returns false' do - expect(subnet.is_reservable?(IPAddr.new('f1ee:0000:0000:0000:0000:0000:0000:0001'))).to be_falsey + expect(subnet.is_reservable?(Bosh::Director::IpAddrOrCidr.new('f1ee:0000:0000:0000:0000:0000:0000:0001'))).to be_falsey end end end diff --git a/src/bosh-director/spec/unit/deployment_plan/network_planner/reservation_reconciler_spec.rb b/src/bosh-director/spec/unit/deployment_plan/network_planner/reservation_reconciler_spec.rb index 1b090973a70..4e41552e31d 100644 --- a/src/bosh-director/spec/unit/deployment_plan/network_planner/reservation_reconciler_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/network_planner/reservation_reconciler_spec.rb @@ -9,20 +9,20 @@ module Bosh::Director::DeploymentPlan let(:instance_model) { FactoryBot.create(:models_instance, availability_zone: initial_az) } let(:instance) { instance_double(Instance, model: instance_model) } let(:variables_interpolator) { instance_double(Bosh::Director::ConfigServer::VariablesInterpolator) } - let(:network) { ManualNetwork.new('my-network', subnets, per_spec_logger) } + let(:network) { ManualNetwork.new('my-network', subnets, '32', per_spec_logger) } let(:subnets) do [ ManualNetworkSubnet.new( 'my-network', IPAddr.new('192.168.1.0/24'), nil, nil, nil, nil, ['zone_1'], [], - ['192.168.1.10'] + ['192.168.1.10'], nil, nil, '32' ), ManualNetworkSubnet.new( 'my-network', IPAddr.new('192.168.2.0/24'), nil, nil, nil, nil, ['zone_2'], [], - ['192.168.2.10'] + ['192.168.2.10'], nil, nil, '32' ), ] end @@ -40,13 +40,13 @@ module Bosh::Director::DeploymentPlan let(:desired_az) { AvailabilityZone.new('zone_1', {}) } let(:existing_reservations) do [ - Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual'), - Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.3', 'manual'), + Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual'), + Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.3/32', 'manual'), ] end let(:dynamic_network_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } - let(:static_network_reservation) { Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2') } + let(:static_network_reservation) { Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2/32') } let(:should_create_swap_delete?) { false } @@ -55,9 +55,9 @@ module Bosh::Director::DeploymentPlan end context 'when the instance group is on a dynamic network' do - let(:network) { DynamicNetwork.new('my-network', subnets, per_spec_logger) } + let(:network) { DynamicNetwork.new('my-network', subnets, '32', per_spec_logger) } let(:desired_reservations) { [dynamic_network_reservation] } - let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'dynamic')] } + let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'dynamic')] } it 'uses the existing reservation' do existing_reservations.map { |reservation| reservation.resolve_type(:dynamic) } @@ -82,7 +82,7 @@ module Bosh::Director::DeploymentPlan let(:existing_reservations) do [ - Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual'), + Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual'), ] end @@ -178,8 +178,8 @@ module Bosh::Director::DeploymentPlan let(:existing_reservations) do [ - Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual'), - Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.3', 'manual'), + Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual'), + Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.3/32', 'manual'), ] end @@ -202,7 +202,7 @@ module Bosh::Director::DeploymentPlan end describe 'changes to specifications about the instances network' do - let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual')] } + let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual')] } let(:desired_reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network2)] } before do @@ -210,7 +210,7 @@ module Bosh::Director::DeploymentPlan end context 'when the existing ip is part of a vip static network but the desired network is a global vip network' do - let(:network) { VipNetwork.new('static-vip-network', nil, [], per_spec_logger) } + let(:network) { VipNetwork.new('static-vip-network', nil, [], '32', per_spec_logger) } let(:network_spec) do { 'name' => 'global-vip-network', @@ -221,7 +221,7 @@ module Bosh::Director::DeploymentPlan end let(:network2) { VipNetwork.parse(network_spec, [], per_spec_logger) } - let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'vip')] } + let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'vip')] } let(:initial_az) { 'zone_1' } it 'should keep the existing reservation' do network_plans = network_planner.reconcile(existing_reservations) @@ -241,7 +241,7 @@ module Bosh::Director::DeploymentPlan context 'when the network name changes' do let(:initial_az) { 'zone_1' } - let(:network2) { ManualNetwork.new('my-network-2', subnets, per_spec_logger) } + let(:network2) { ManualNetwork.new('my-network-2', subnets, '32', per_spec_logger) } it 'should keep existing reservations' do network_plans = network_planner.reconcile(existing_reservations) @@ -282,7 +282,7 @@ module Bosh::Director::DeploymentPlan 'my-network', IPAddr.new('192.168.1.0/24'), nil, nil, nil, nil, [], [], - ['192.168.1.10'] + ['192.168.1.10'], '32' ), ] end @@ -306,14 +306,14 @@ module Bosh::Director::DeploymentPlan context 'when the instance model has no az' do let(:initial_az) { '' } let(:desired_az) { nil } - let(:network2) { ManualNetwork.new('my-network-2', subnets, per_spec_logger) } + let(:network2) { ManualNetwork.new('my-network-2', subnets, '32', per_spec_logger) } let(:subnets) do [ ManualNetworkSubnet.new( 'my-network', IPAddr.new('192.168.1.0/24'), nil, nil, nil, nil, [], [], - ['192.168.1.10'] + ['192.168.1.10'], nil, nil, '32' ), ] end @@ -334,7 +334,7 @@ module Bosh::Director::DeploymentPlan end context 'when the network type changes to dynamic' do - let(:network2) { DynamicNetwork.new('my-network-2', subnets, per_spec_logger) } + let(:network2) { DynamicNetwork.new('my-network-2', subnets, '32', per_spec_logger) } it 'should have a new reservation' do network_plans = network_planner.reconcile(existing_reservations) @@ -352,8 +352,8 @@ module Bosh::Director::DeploymentPlan end context 'when the network type changes to manual' do - let(:network) { DynamicNetwork.new('my-network', subnets, per_spec_logger) } - let(:network2) { ManualNetwork.new('my-network-2', subnets, per_spec_logger) } + let(:network) { DynamicNetwork.new('my-network', subnets, '32', per_spec_logger) } + let(:network2) { ManualNetwork.new('my-network-2', subnets, '32', per_spec_logger) } it 'should have a new reservation' do network_plans = network_planner.reconcile(existing_reservations) @@ -428,7 +428,7 @@ module Bosh::Director::DeploymentPlan context 'when existing reservation availability zones do not match job availability zones' do let(:desired_az) { AvailabilityZone.new('zone_2', {}) } - let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual')] } + let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual')] } let(:desired_reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network)] } before { existing_reservations[0].resolve_type(:dynamic) } @@ -462,7 +462,7 @@ module Bosh::Director::DeploymentPlan context 'when existing reservation and job do not belong to any availability zone' do let(:desired_az) { nil } - let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2', 'manual')] } + let(:existing_reservations) { [Bosh::Director::ExistingNetworkReservation.new(instance_model, network, '192.168.1.2/32', 'manual')] } let(:desired_reservations) { [Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network)] } let(:subnets) do [ @@ -470,7 +470,9 @@ module Bosh::Director::DeploymentPlan 'my-network', IPAddr.new('192.168.1.0/24'), nil, nil, nil, nil, nil, [], - ['192.168.1.10'] + ['192.168.1.10'], + nil, nil, + '32' ), ] end @@ -485,7 +487,7 @@ module Bosh::Director::DeploymentPlan expect(obsolete_plans.count).to eq(0) expect(existing_plans.count).to eq(1) - expect(to_ipaddr(existing_plans.first.reservation.ip)).to eq('192.168.1.2') + expect(to_ipaddr(existing_plans.first.reservation.ip)).to eq('192.168.1.2/32') expect(desired_plans.count).to eq(0) end end @@ -494,8 +496,8 @@ module Bosh::Director::DeploymentPlan let(:dynamic_network_reservation) { Bosh::Director::DesiredNetworkReservation.new_dynamic(instance_model, network) } let(:desired_reservations) do [ - Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2'), - Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.4'), + Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.2/32'), + Bosh::Director::DesiredNetworkReservation.new_static(instance_model, network, '192.168.1.4/32'), dynamic_network_reservation, ] end From 07b52b7fc307185fed510bde2fdd3ef436a51de7 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Thu, 8 May 2025 16:11:16 +0200 Subject: [PATCH 16/17] continue --- .../instance_network_reservations.rb | 2 +- .../lib/bosh/director/ip_util.rb | 2 +- .../instance_group_networks_parser_spec.rb | 10 ++++----- .../instance_group_spec_parser_spec.rb | 6 ++--- .../instance_plan_factory_spec.rb | 2 +- .../instance_repository_spec.rb | 2 +- .../networks_to_static_ips_spec.rb | 2 +- src/bosh-director/spec/unit/ip_util_spec.rb | 2 +- .../spec/unit/jobs/vm_state_spec.rb | 22 +++++++++---------- src/bosh-director/spec/unit/models/vm_spec.rb | 2 +- 10 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb index b2273f91333..694f9112f18 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/instance_network_reservations.rb @@ -24,7 +24,7 @@ def self.create_from_db(instance_model, deployment, logger) # Dynamic network reservations are not saved in DB, recreating from instance spec instance_model.spec.fetch('networks', []).each do |network_name, network_config| next unless network_config['type'] == 'dynamic' - reservations.add_existing(instance_model, deployment, network_name, Bosh::Director::IpAddrOrCidr.new("#{network_config['ip']}/99"), network_config['type']) + reservations.add_existing(instance_model, deployment, network_name, Bosh::Director::IpAddrOrCidr.new("#{network_config['ip']}/32"), network_config['type']) end end diff --git a/src/bosh-director/lib/bosh/director/ip_util.rb b/src/bosh-director/lib/bosh/director/ip_util.rb index 69910018904..c6bd491ca3b 100644 --- a/src/bosh-director/lib/bosh/director/ip_util.rb +++ b/src/bosh-director/lib/bosh/director/ip_util.rb @@ -21,7 +21,7 @@ def to_ipaddr(ip) # @param [Integer] ip Integer IP representation # @return [String] Human-readable IP representation def format_ip(ip) - to_ipaddr(ip).to_cidr_s + to_ipaddr(ip) end def format_cidr_ip(ip) diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb index 0491f698634..b195a848def 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_group_networks_parser_spec.rb @@ -11,10 +11,10 @@ module Bosh::Director::DeploymentPlan instance_group_network['static_ips'] = ['192.168.1.1', '192.168.1.2'] instance_group end - let(:manifest_networks) { [ManualNetwork.new('a', [], per_spec_logger)] } + let(:manifest_networks) { [ManualNetwork.new('a', [], '32', per_spec_logger)] } context 'when instance group references a network not mentioned in the networks spec' do - let(:manifest_networks) { [ManualNetwork.new('my-network', [], per_spec_logger)] } + let(:manifest_networks) { [ManualNetwork.new('my-network', [], '32', per_spec_logger)] } it 'raises JobUnknownNetwork' do expect do @@ -38,7 +38,7 @@ module Bosh::Director::DeploymentPlan end context 'when instance group network spec references dynamic network with static IPs' do - let(:dynamic_network) { Bosh::Director::DeploymentPlan::DynamicNetwork.new('a', [], per_spec_logger) } + let(:dynamic_network) { Bosh::Director::DeploymentPlan::DynamicNetwork.new('a', [], '32', per_spec_logger) } let(:instance_group_spec) do instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first instance_group['networks'] = [{ @@ -66,7 +66,7 @@ module Bosh::Director::DeploymentPlan it 'raises an error' do expect do instance_group_networks_parser.parse(instance_group_spec, 'instance-group-name', manifest_networks) - end.to raise_error Bosh::Director::JobInvalidStaticIPs, "Instance group 'instance-group-name' specifies static IP '192.168.1.2' more than once" + end.to raise_error Bosh::Director::JobInvalidStaticIPs, "Instance group 'instance-group-name' specifies static IP '192.168.1.2/32' more than once" end end @@ -77,7 +77,7 @@ module Bosh::Director::DeploymentPlan 'gateway' => '192.168.1.1' } end - let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], per_spec_logger)] } + let(:manifest_networks) { [ManualNetwork.new('a', [ManualNetworkSubnet.parse('a', subnet_spec, "")], '32', per_spec_logger)] } let(:instance_group_spec) do instance_group = SharedSupport::DeploymentManifestHelper.simple_manifest_with_instance_groups['instance_groups'].first diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_group_spec_parser_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_group_spec_parser_spec.rb index fa8d9d62ba1..0c13aa1dc5c 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_group_spec_parser_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_group_spec_parser_spec.rb @@ -32,7 +32,7 @@ module DeploymentPlan ) end let(:deployment_model) { FactoryBot.create(:models_deployment) } - let(:network) { ManualNetwork.new('fake-network-name', [], per_spec_logger) } + let(:network) { ManualNetwork.new('fake-network-name', [], '32', per_spec_logger) } let(:task) { FactoryBot.create(:models_task, id: 42) } let(:task_writer) { Bosh::Director::TaskDBWriter.new(:event_output, task.id) } let(:event_log) { Bosh::Director::EventLog::Log.new(task_writer) } @@ -1002,7 +1002,7 @@ module DeploymentPlan instance_group_spec['instances'] = 3 instance_group_spec['networks'].first['default'] = %w[gateway dns] instance_group_spec['networks'] << instance_group_spec['networks'].first.merge('name' => 'duped-network') # dupe it - duped_network = ManualNetwork.new('duped-network', [], per_spec_logger) + duped_network = ManualNetwork.new('duped-network', [], '32', per_spec_logger) allow(deployment_plan).to receive(:networks).and_return([duped_network, network]) expect do @@ -1029,7 +1029,7 @@ module DeploymentPlan end context 'when there are two networks, each being a separate default' do - let(:network2) { ManualNetwork.new('fake-network-name-2', [], per_spec_logger) } + let(:network2) { ManualNetwork.new('fake-network-name-2', [], '32', per_spec_logger) } it 'picks the only network as default' do instance_group_spec['networks'].first['default'] = ['dns'] diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_plan_factory_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_plan_factory_spec.rb index f5c4f9a67b9..3bdf2da1489 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_plan_factory_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_plan_factory_spec.rb @@ -45,7 +45,7 @@ module DeploymentPlan let(:range) { IPAddr.new('192.168.1.1/24') } let(:manual_network_subnet) { ManualNetworkSubnet.new('name-7', range, nil, nil, nil, nil, nil, [], []) } - let(:network) { Bosh::Director::DeploymentPlan::ManualNetwork.new('name-7', [manual_network_subnet], per_spec_logger) } + let(:network) { Bosh::Director::DeploymentPlan::ManualNetwork.new('name-7', [manual_network_subnet], '32', per_spec_logger) } let(:ip_repo) { Bosh::Director::DeploymentPlan::IpRepo.new(per_spec_logger) } let(:deployment_plan) do instance_double( diff --git a/src/bosh-director/spec/unit/deployment_plan/instance_repository_spec.rb b/src/bosh-director/spec/unit/deployment_plan/instance_repository_spec.rb index 5d83da4aed9..e70b9ffb2d9 100644 --- a/src/bosh-director/spec/unit/deployment_plan/instance_repository_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/instance_repository_spec.rb @@ -7,7 +7,7 @@ subject(:instance_repository) { Bosh::Director::DeploymentPlan::InstanceRepository.new(logger, variables_interpolator) } let(:variables_interpolator) { instance_double(Bosh::Director::ConfigServer::VariablesInterpolator) } - let(:network) { Bosh::Director::DeploymentPlan::DynamicNetwork.new('name-7', [], logger) } + let(:network) { Bosh::Director::DeploymentPlan::DynamicNetwork.new('name-7', [], '32', logger) } let(:deployment_plan) do ip_repo = Bosh::Director::DeploymentPlan::IpRepo.new(logger) diff --git a/src/bosh-director/spec/unit/deployment_plan/placement_planner/networks_to_static_ips_spec.rb b/src/bosh-director/spec/unit/deployment_plan/placement_planner/networks_to_static_ips_spec.rb index 52550914189..3d443475619 100644 --- a/src/bosh-director/spec/unit/deployment_plan/placement_planner/networks_to_static_ips_spec.rb +++ b/src/bosh-director/spec/unit/deployment_plan/placement_planner/networks_to_static_ips_spec.rb @@ -107,7 +107,7 @@ module Bosh::Director::DeploymentPlan ['192.168.1.10', '192.168.1.11', '192.168.1.12', '192.168.1.13', '192.168.1.14']) ] end - let(:deployment_network) { ManualNetwork.new('network_A', deployment_subnets, nil) } + let(:deployment_network) { ManualNetwork.new('network_A', deployment_subnets, '32', nil) } let(:job_networks) do [FactoryBot.build(:deployment_plan_job_network, name: 'network_A', static_ips: job_static_ips, deployment_network: deployment_network)] end diff --git a/src/bosh-director/spec/unit/ip_util_spec.rb b/src/bosh-director/spec/unit/ip_util_spec.rb index 6a917afca27..ebfb9599aba 100644 --- a/src/bosh-director/spec/unit/ip_util_spec.rb +++ b/src/bosh-director/spec/unit/ip_util_spec.rb @@ -67,7 +67,7 @@ describe 'format_ip' do it 'converts integer to CIDR IP' do - expect(ip_util_includer.format_ip(168427582)).to eq('10.10.0.62') + expect(ip_util_includer.format_ip(168427582)).to eq('10.10.0.62/32') end end diff --git a/src/bosh-director/spec/unit/jobs/vm_state_spec.rb b/src/bosh-director/spec/unit/jobs/vm_state_spec.rb index 304d9fcd4e8..a7082bf43a2 100644 --- a/src/bosh-director/spec/unit/jobs/vm_state_spec.rb +++ b/src/bosh-director/spec/unit/jobs/vm_state_spec.rb @@ -60,7 +60,7 @@ def stub_agent_get_state_to_return_state_with_vitals ) expect(agent).to receive(:get_state).with('full').and_return( 'vm_cid' => 'fake-vm-cid', - 'networks' => { 'test' => { 'ip' => '1.1.1.1/32' } }, + 'networks' => { 'test' => { 'ip' => '1.1.1.1' } }, 'agent_id' => 'fake-agent-id', 'job_state' => 'running', ) @@ -68,7 +68,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1/32']) + expect(status['ips']).to eq(['1.1.1.1']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -98,7 +98,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1/32', '2.2.2.2/32']) + expect(status['ips']).to eq(['1.1.1.1', '2.2.2.2']) end end @@ -116,7 +116,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['3.3.3.3/32', '4.4.4.4/32']) + expect(status['ips']).to eq(['3.3.3.3', '4.4.4.4']) end end @@ -158,8 +158,8 @@ def stub_agent_get_state_to_return_state_with_vitals status = JSON.parse(Models::Task.first(id: task.id).result_output) expect(status['ips']).to contain_exactly( - '1.1.1.1/32', '2.2.2.2/32', # Static - '3.3.3.3/32', '4.4.4.4/32', # Dynamic + '1.1.1.1', '2.2.2.2', # Static + '3.3.3.3', '4.4.4.4', # Dynamic ) end end @@ -176,7 +176,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1/32']) + expect(status['ips']).to eq(['1.1.1.1']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -380,7 +380,7 @@ def stub_agent_get_state_to_return_state_with_vitals job.perform status = JSON.parse(Models::Task.first(id: task.id).result_output) - expect(status['ips']).to eq(['1.1.1.1/32']) + expect(status['ips']).to eq(['1.1.1.1']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -465,7 +465,7 @@ def stub_agent_get_state_to_return_state_with_vitals results.sort_by! { |r| r['ips'][0] } status = results[0] - expect(status['ips']).to eq(['1.1.1.1/32']) + expect(status['ips']).to eq(['1.1.1.1']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') @@ -475,7 +475,7 @@ def stub_agent_get_state_to_return_state_with_vitals { 'name' => 'fake-process-2', 'state' => 'failing' }]) status = results[1] - expect(status['ips']).to eq(['1.1.1.2/32']) + expect(status['ips']).to eq(['1.1.1.2']) expect(status['vm_cid']).to eq('fake-vm-cid-2') expect(status['active']).to eq(false) expect(status['agent_id']).to eq('other_agent_id') @@ -495,7 +495,7 @@ def stub_agent_get_state_to_return_state_with_vitals expect(results.length).to eq(1) status = JSON.parse(results[0]) - expect(status['ips']).to eq(['1.1.1.1/32']) + expect(status['ips']).to eq(['1.1.1.1']) expect(status['vm_cid']).to eq('fake-vm-cid') expect(status['active']).to eq(true) expect(status['agent_id']).to eq('fake-agent-id') diff --git a/src/bosh-director/spec/unit/models/vm_spec.rb b/src/bosh-director/spec/unit/models/vm_spec.rb index e3301d545bd..a3b59acf849 100644 --- a/src/bosh-director/spec/unit/models/vm_spec.rb +++ b/src/bosh-director/spec/unit/models/vm_spec.rb @@ -47,7 +47,7 @@ module Bosh::Director::Models end it 'returns all ips for the vm' do - expect(vm.ips).to match_array(['1.1.1.1/32', '1.1.1.2/32', '1.1.1.3/32']) + expect(vm.ips).to match_array(['1.1.1.1', '1.1.1.2', '1.1.1.3']) end end end From c081592d57c46830800cb15e1b091c5c4f5ef662 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Fri, 9 May 2025 13:30:58 +0200 Subject: [PATCH 17/17] fix prefix for ipv6 --- src/bosh-director/lib/bosh/director/models/ip_address.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bosh-director/lib/bosh/director/models/ip_address.rb b/src/bosh-director/lib/bosh/director/models/ip_address.rb index 0a8855f2776..b4d6be4b487 100644 --- a/src/bosh-director/lib/bosh/director/models/ip_address.rb +++ b/src/bosh-director/lib/bosh/director/models/ip_address.rb @@ -55,7 +55,7 @@ def address else address_str.match?(/\A\d+\z/) ip = Bosh::Director::IpAddrOrCidr.new(address_str.to_i) if ip.ipv6? - prefix = 132 + prefix = 128 else prefix = 32 end