export ips_cidr in different field

Author: fmoehler

src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb

@@ -618,18 +618,15 @@ 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,
           'job' => instance.job,
           '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

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,

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

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
 

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)

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

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,

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
 

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
                                    )