Skip to content

Commit

Permalink
Support named (non-default) Backend Address Pools for Load Balancers (c…
Browse files Browse the repository at this point in the history
…loudfoundry#644)

Note: The new, optional `load_balancer/backend_pool_name` sub-property is an optional key of each LB Hash config (whether specifying an Array of LBs or a single LB).

Note: This maintains/restores functional symmetry between the LB and AGW configs (although the symmetric LB functionality was not an explicitly requested part of the feature enhancements for cloudfoundry#644, adding the feature only for AGWs would break functional symmetry).

---

SQUASHED COMMIT HISTORY:

issue-644: named-pool-LB: Added a new, optional `load_balancer/backend_pool_name` sub-property for the `load_balancer` config.
Note: The new sub-property is an optional key of each LB Hash config, whether specifying an Array of LBs or a single LB.
  • Loading branch information
Justin-W committed Jan 12, 2022
1 parent ff3d78e commit 0ed96f4
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 9 deletions.
8 changes: 5 additions & 3 deletions src/bosh_azure_cpi/lib/cloud/azure/models/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

module Bosh::AzureCloud
class LoadBalancerConfig
attr_reader :name, :resource_group_name
def initialize(resource_group_name, name)
attr_reader :name, :resource_group_name, :backend_pool_name

def initialize(resource_group_name, name, backend_pool_name = nil)
@resource_group_name = resource_group_name
@name = name
@backend_pool_name = backend_pool_name
end

def to_s
"name: #{@name}, resource_group_name: #{@resource_group_name}"
"name: #{@name}, resource_group_name: #{@resource_group_name}, backend_pool_name: #{@backend_pool_name}"
end
end

Expand Down
5 changes: 4 additions & 1 deletion src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,17 @@ def _parse_load_balancer_config(vm_properties, global_azure_config)
if lbc.is_a?(Hash)
load_balancer_names = lbc[NAME_KEY]
resource_group_name = lbc[RESOURCE_GROUP_NAME_KEY]
backend_pool_name = lbc[BACKEND_POOL_NAME_KEY]
else
load_balancer_names = lbc
resource_group_name = nil
backend_pool_name = nil
end
String(load_balancer_names).split(',').map do |load_balancer_name|
Bosh::AzureCloud::LoadBalancerConfig.new(
resource_group_name || global_azure_config.resource_group_name,
load_balancer_name
load_balancer_name,
backend_pool_name
)
end
end
Expand Down
15 changes: 15 additions & 0 deletions src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,24 @@ def _get_load_balancers(vm_props)
load_balancers = nil
load_balancer_configs = vm_props.load_balancers
unless load_balancer_configs.nil?
# NOTE: The following block gets the Azure API info for each Bosh AGW config (from the vm_type config).
load_balancers = load_balancer_configs.map do |load_balancer_config|
load_balancer = @azure_client.get_load_balancer_by_name(load_balancer_config.resource_group_name, load_balancer_config.name)
cloud_error("Cannot find the load balancer '#{load_balancer_config.name}'") if load_balancer.nil?

if load_balancer_config.backend_pool_name
# NOTE: This is the only place where we simultaneously have both the Bosh LB config (from the vm_type) AND the Azure LB info (from the Azure API).
# Since the `AzureClient.create_network_interface` method only uses the first backend pool,
# and since there is not a 1-to-1 mapping between the vm_type LB configs and LBs (despite the config property name, each vm_type LB config actually represents a single LB Backend Pool, not a whole LB),
# we can therefore remove all pools EXCEPT the specified pool.
# To handle multiple pools of a single LB, you should use 1 vm_type LB Hash (with LB name + backend_pool_name) per pool.
pools = load_balancer[:backend_address_pools]
pools = [pools] unless pools.is_a?(Array)
pool = pools.find { |p| Hash(p)[:name] == load_balancer_config.backend_pool_name }
cloud_error("'#{load_balancer_config.name}' does not have a backend_pool named '#{load_balancer_config.backend_pool_name}': #{load_balancer}") if pool.nil?

load_balancer[:backend_address_pools] = [pool]
end
load_balancer
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,36 @@
azure_client.create_network_interface(resource_group, nic_params)
end.not_to raise_error
end

context 'with multiple backend pools' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for multi-pool LBs
it 'should create a network interface without error'

context 'when backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'should use the specified backend_pool'
end

context 'when an invalid backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'should raise an error'
end
end
end

context 'with multiple load balancers' do # rubocop:disable RSpec/RepeatedExampleGroupBody
# TODO: issue-644: multi-LB: add unit tests for multi-LBs
it 'should create a network interface without error'

context 'with multiple backend pools' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for multi-pool LBs
it 'should create a network interface without error'

context 'when backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'should use the specified backend_pool'
end
end
end

context 'with application security groups' do
Expand Down Expand Up @@ -556,7 +586,7 @@
end
end

context 'with multiple application gateways' do
context 'with multiple application gateways' do # rubocop:disable RSpec/RepeatedExampleGroupBody
# TODO: issue-644: multi-AGW: add unit tests for multi-AGWs
it 'should create a network interface without error'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
context 'when called' do
let(:name) { 'fake_name' }
let(:resource_group_name) { 'fake_rg' }
let(:lb_string) { "name: #{name}, resource_group_name: #{resource_group_name}" }
let(:load_balancer_config) { Bosh::AzureCloud::LoadBalancerConfig.new(resource_group_name, name) }
let(:backend_pool_name) { 'fake_pool' }
let(:expected_string) { "name: #{name}, resource_group_name: #{resource_group_name}, backend_pool_name: #{backend_pool_name}" }
let(:load_balancer_config) { Bosh::AzureCloud::LoadBalancerConfig.new(resource_group_name, name, backend_pool_name) }

it 'should return the correct string' do
expect(load_balancer_config.to_s).to eq(lb_string)
expect(load_balancer_config.to_s).to eq(expected_string)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,33 @@
expect(vm_params[:name]).to eq(vm_name)
end

context 'with single load_balancer' do
context 'when load_balancer has multiple backend pools' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for multi-pool LBs
it 'adds the public IP to the default pool'

context 'when backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'adds the public IP to the specified pool'
end

context 'when an invalid backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'should raise an error'
end
end
end

context 'with multiple load_balancers' do
# TODO: issue-644: multi-LB: add unit tests for multi-LBs
it 'adds the public IP to each load_balancer'

context 'when backend_pool_name is specified' do
# TODO: issue-644: multi-BEPool-LB: add unit tests for named-pool LBs
it 'adds the public IP to the specified pool of each load_balancer'
end
end

context 'with single application_gateway' do
context 'when application_gateway has multiple backend pools' do
# TODO: issue-644: multi-BEPool-AGW: add unit tests for multi-pool AGWs
Expand Down
10 changes: 9 additions & 1 deletion src/bosh_azure_cpi/spec/unit/vm_manager/create/shared_stuff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,15 @@
end
let(:load_balancer) do
{
name: 'fake-lb-name'
name: 'fake-lb-name',
backend_address_pools: [
{
name: 'fake-pool-name',
id: 'fake-pool-id',
provisioning_state: 'fake-pool-state',
backend_ip_configurations: []
}
]
}
end
let(:application_gateway) do
Expand Down

0 comments on commit 0ed96f4

Please sign in to comment.