Remove vip usage (#3761)

Author: moleske

app/models/runtime/route.rb

@@ -8,8 +8,6 @@ module VCAP::CloudController
   class Route < Sequel::Model
     class InvalidOrganizationRelation < CloudController::Errors::InvalidRelation; end
 
-    class OutOfVIPException < CloudController::Errors::InvalidRelation; end
-
     many_to_one :domain
     many_to_one :space, after_set: :validate_changed_space
     one_through_one :organization, join_table: Space.table_name, left_key: :id, left_primary_key: :space_id, right_primary_key: :id, right_key: :organization_id
@@ -126,7 +124,6 @@ def validate
       validate_total_routes
       validate_ports
       validate_total_reserved_route_ports if port && port > 0
-      errors.add(:name, :vip_offset) if vip_offset_exceeds_range?
 
       RouteValidator.new(self).validate
     rescue RoutingApi::UaaUnavailable
@@ -212,59 +209,17 @@ def internal?
       domain.internal
     end
 
-    def vip
-      vip_offset && internal_route_vip_range.nth(vip_offset).to_s
-    end
-
     def wildcard_host?
       host == '*'
     end
 
     private
 
-    def vip_offset_exceeds_range?
-      return false if vip_offset.nil?
-      return true if vip_offset <= 0
-
-      vip_offset > internal_route_vip_range_len
-    end
-
     def before_destroy
       destroy_route_bindings
       super
     end
 
-    def find_next_vip_offset
-      # This code courtesy of Jeremy Evans as part of discussion on
-      # https://groups.google.com/d/msg/sequel-talk/3GJ8_mOgJ9U/roWJ2sWHAwAJ
-      # See SQL self-joins for the reasoning behind this
-
-      n = Route.exclude(vip_offset: 1).
-          exclude { vip_offset - 1 =~ Route.select(:vip_offset) }.order(:vip_offset).get { vip_offset - 1 } ||
-          (return (Route.max(:vip_offset) || 0) + 1)
-      Route.where { vip_offset < n }.reverse(:vip_offset).get { vip_offset + 1 } || 1
-    end
-
-    def before_save
-      return unless internal? && vip_offset.nil?
-
-      len = internal_route_vip_range_len
-      raise OutOfVIPException.new('out of vip_offset slots') if self.class.exclude(vip_offset: nil).count >= len
-
-      self.vip_offset = find_next_vip_offset
-    end
-
-    def internal_route_vip_range_len
-      internal_route_vip_range.len - 2
-    end
-
-    def internal_route_vip_range
-      @internal_route_vip_range ||= begin
-        internal_route_vip_range = Config.config.get(:internal_route_vip_range)
-        NetAddr::IPv4Net.parse(internal_route_vip_range)
-      end
-    end
-
     def destroy_route_bindings
       errors = RouteBindingDelete.new.delete(route_binding_dataset)
 

config/cloud_controller.yml

@@ -379,8 +379,6 @@ credhub_api:
 credential_references:
   interpolate_service_bindings: true
 
-internal_route_vip_range: '127.128.0.0/9'
-
 locket:
   host: 'locket.service.cf.internal'
   port: 8891

lib/cloud_controller/config_schemas/base/api_schema.rb

@@ -380,8 +380,6 @@ class ApiSchema < VCAP::Config
             max_labels_per_resource: Integer,
             max_annotations_per_resource: Integer,
 
-            internal_route_vip_range: String,
-
             default_app_lifecycle: enum('buildpack', 'cnb'),
             custom_metric_tag_prefix_list: Array,
 

lib/cloud_controller/config_schemas/base/worker_schema.rb

@@ -192,7 +192,6 @@ class WorkerSchema < VCAP::Config
 
             max_labels_per_resource: Integer,
             max_annotations_per_resource: Integer,
-            internal_route_vip_range: String,
             custom_metric_tag_prefix_list: Array,
             default_app_lifecycle: enum('buildpack', 'cnb')
           }

lib/cloud_controller/config_schemas/vms/route_syncer_schema.rb

@@ -35,10 +35,7 @@ class RouteSyncerSchema < VCAP::Config
               keys: Hash,
               current_key_label: String,
               optional(:pbkdf2_hmac_iterations) => Integer
-            },
-
-            internal_route_vip_range: String
-
+            }
           }
         end
 

spec/unit/models/runtime/route_spec.rb

@@ -1407,167 +1407,6 @@ def assert_invalid_path(path)
       end
     end
 
-    describe 'vip_offset' do
-      before do
-        TestConfig.override(
-          internal_route_vip_range: '127.128.99.0/29',
-          kubernetes: {}
-        )
-      end
-
-      context 'auto-assign vip_offset' do
-        let(:internal_domain) { SharedDomain.make(name: 'apps.internal', internal: true) }
-        let!(:internal_route_1) { Route.make(host: 'meow', domain: internal_domain) }
-        let!(:internal_route_2) { Route.make(host: 'woof', domain: internal_domain) }
-        let!(:internal_route_3) { Route.make(host: 'quack', domain: internal_domain) }
-        let(:external_private_route) { Route.make }
-
-        context 'when the Kubernetes API is not configured' do
-          before do
-            TestConfig.override( # 8 theoretical available ips, 6 actual
-              internal_route_vip_range: '127.128.99.0/29',
-              kubernetes: {}
-            )
-          end
-
-          it 'auto-assigns vip_offset to internal routes only' do
-            expect(internal_route_1.vip_offset).not_to be_nil
-            expect(external_private_route.vip_offset).to be_nil
-          end
-
-          it 'assigns multiple vips in ascending order without duplicates' do
-            expect(internal_route_1.vip_offset).to eq(1)
-            expect(internal_route_2.vip_offset).to eq(2)
-          end
-
-          it 'never assigns the same vip_offset to multiple internal routes' do
-            expect do
-              Route.make(host: 'ants', vip_offset: 1)
-            end.to raise_error(Sequel::UniqueConstraintViolation, /duplicate.*routes_vip_offset_index/i)
-          end
-
-          it 'finds an available offset' do
-            Route.make(host: 'gulp', domain: internal_domain)
-            expect(Route.select_map(:vip_offset)).to match_array((1..4).to_a)
-          end
-
-          context 'when the taken offset is not the first' do
-            before do
-              TestConfig.override(
-                kubernetes: {}
-              )
-            end
-
-            it 'finds the first offset' do
-              internal_route_1.destroy
-              expect(Route.make(host: 'gulp', domain: internal_domain).vip_offset).to eq(1)
-            end
-          end
-
-          context 'when the taken offsets include first and not second' do
-            it 'finds an available offset' do
-              internal_route_2.destroy
-              expect(Route.make(host: 'gulp', domain: internal_domain).vip_offset).to eq(2)
-            end
-          end
-
-          context 'when filling the vip range' do
-            it 'can make 3 more new routes only' do
-              expect { Route.make(host: 'route4', domain: internal_domain) }.not_to raise_error
-              expect { Route.make(host: 'route5', domain: internal_domain) }.not_to raise_error
-              expect { Route.make(host: 'route6', domain: internal_domain) }.not_to raise_error
-              expect { Route.make(host: 'route7', domain: internal_domain) }.to raise_error(Route::OutOfVIPException)
-            end
-
-            it 'can reclaim lost vips' do
-              expect { Route.make(host: 'route4', domain: internal_domain) }.not_to raise_error
-              expect { Route.make(host: 'route5', domain: internal_domain) }.not_to raise_error
-              expect { Route.make(host: 'route6', domain: internal_domain) }.not_to raise_error
-              Route.last.destroy
-              internal_route_2.destroy
-              expect(Route.make(host: 'new2', domain: internal_domain).vip_offset).to eq(2)
-              expect(Route.make(host: 'new6', domain: internal_domain).vip_offset).to eq(6)
-            end
-          end
-        end
-      end
-
-      context 'when we assign vip_offsets explicitly' do
-        let(:internal_domain) { SharedDomain.make(name: 'apps.internal', internal: true) }
-
-        it 'does not assign vip_offsets that exceed the CIDR range' do
-          expect do
-            Route.make(host: 'ants0', domain: internal_domain, vip_offset: 0)
-          end.to raise_error(Sequel::ValidationFailed, 'name vip_offset')
-          expect do
-            Route.make(host: 'ants1', domain: internal_domain, vip_offset: 1)
-          end.not_to raise_error
-          expect do
-            Route.make(host: 'ants6', domain: internal_domain, vip_offset: 6)
-          end.not_to raise_error
-          expect do
-            Route.make(host: 'ants7', domain: internal_domain, vip_offset: 7)
-          end.to raise_error(Sequel::ValidationFailed, 'name vip_offset')
-          expect do
-            Route.make(host: 'ants8', domain: internal_domain, vip_offset: 8)
-          end.to raise_error(Sequel::ValidationFailed, 'name vip_offset')
-        end
-      end
-
-      context 'when there are routes on internal domains' do
-        let(:internal_domain) { SharedDomain.make(name: 'apps.internal', internal: true) }
-        let!(:internal_route_1) { Route.make(host: 'meow', domain: internal_domain, vip_offset: nil) }
-        let!(:internal_route_2) { Route.make(host: 'woof', domain: internal_domain, vip_offset: 2) }
-        let!(:internal_route_3) { Route.make(host: 'quack', domain: internal_domain, vip_offset: 4) }
-        let(:external_private_route) { Route.make }
-
-        it 'can have different vip_offsets in range' do
-          expect(internal_route_1).to be_valid
-          expect(internal_route_1.vip_offset).to eq(1)
-          expect(internal_route_2).to be_valid
-          expect(internal_route_3).to be_valid
-        end
-
-        it 'assigns lowest-possible vip_offsets' do
-          internal_route_4 = Route.make(host: 'bray', domain: internal_domain)
-          expect(internal_route_4.vip_offset).to eq(3)
-          internal_route_5 = Route.make(host: 'lemons', domain: internal_domain)
-          expect(internal_route_5.vip_offset).to eq(5)
-        end
-
-        it 'reuses vip_offsets' do
-          expected_vip_offset = internal_route_2.vip_offset
-          internal_route_2.delete
-          internal_route_6 = Route.make(host: 'route6', domain: internal_domain)
-          expect(internal_route_6.vip_offset).to eq(expected_vip_offset)
-        end
-      end
-    end
-
-    describe 'vip' do
-      before do
-        TestConfig.override(
-          kubernetes: {}
-        )
-      end
-
-      let(:internal_domain) { SharedDomain.make(name: 'apps.internal', internal: true) }
-      let!(:internal_route_1) { Route.make(host: 'meow', domain: internal_domain, vip_offset: 1) }
-      let!(:internal_route_2) { Route.make(host: 'woof', domain: internal_domain, vip_offset: 2) }
-      let!(:internal_route_3) { Route.make(host: 'quack', domain: internal_domain, vip_offset: 4) }
-      let(:external_private_route) { Route.make }
-
-      it 'returns a ipv4 ip address offset from the beginning of the internal route vip range' do
-        expect(internal_route_1.vip).to eq('127.128.0.1')
-        internal_route_2.vip_offset = 16
-        expect(internal_route_2.vip).to eq('127.128.0.16')
-      end
-
-      it 'returns nil when asked for the ip addr for a nil offset' do
-        expect(external_private_route.vip).to be_nil
-      end
-    end
-
     describe '#wildcard_host?' do
       let!(:route) { Route.make(host:) }