Skip to content

Commit

Permalink
Remove vip usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@moleske
moleske committed Jul 30, 2024
1 parent a0c1455 commit ba9b2d0
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 215 deletions.
45 changes: 0 additions & 45 deletions app/models/runtime/route.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -109,7 +107,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
Expand Down Expand Up @@ -191,59 +188,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)
raise errors.first unless errors.empty?
Expand Down
2 changes: 0 additions & 2 deletions config/cloud_controller.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,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
Expand Down
2 changes: 0 additions & 2 deletions lib/cloud_controller/config_schemas/base/api_schema.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,6 @@ class ApiSchema < VCAP::Config
max_labels_per_resource: Integer,
max_annotations_per_resource: Integer,

internal_route_vip_range: String,

default_app_lifecycle: String,
custom_metric_tag_prefix_list: Array,

Expand Down
1 change: 0 additions & 1 deletion lib/cloud_controller/config_schemas/base/worker_schema.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,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
}
end
Expand Down
5 changes: 1 addition & 4 deletions lib/cloud_controller/config_schemas/vms/route_syncer_schema.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
161 changes: 0 additions & 161 deletions spec/unit/models/runtime/route_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1390,167 +1390,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:) }

Expand Down

0 comments on commit ba9b2d0

Please sign in to comment.