Support multiple Application Gateways (#644)

Note: This increases/restores the symmetry between the LB and AGW functionality (and configuration), by enhancing AGW support with new (for AGWs) functionality which had already been previously supported for LBs (by #541 for #111 and #638 for #328). Restoring this level of symmetry partially resolves #644.

These changes include:

- refactoring of existing code/implementation (and tests). E.g. Renamed some vars and methods to more accurately reflect their current values/behavior.
- significant configuration API enhancements, to enable the (previously LB-only) functionality of #111 and #328 to be used with both LB and/or AGW configs. E.g. Allow the `application_gateway` config to be configured as a Hash; allow the `application_gateway` config to be configured as an Array (new in this PR for both LBs and AGWs); added support for `application_gateway/resource_roup_name` config.

---

SQUASHED COMMIT HISTORY:

issue-644: multi-AGW: Refactored the `AzureClient.get_application_gateway_by_name` method to make the `resource_group_name` param a positional param (instead of an optional keyword param).
Note: This change was made so that the `AzureClient.get_application_gateway_by_name` method's signature is consistent with the signatures of the other `AzureClient.get_*` methods in requiring the `resource_group_name` param to be passed as the first positional param.

issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to allow the `application_gateway` config to be configured as an Array.
Note: When specifying an Array of AGWs, each element of the Array should be a String and/or Hash of the same format which was previously (and still is) valid when configuring a single AGW (as a non-Array).

issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to accept either String or Hash config (instead of String only).

issue-644: multi-AGW: Fixed the `AzureClient.parse_network_interface` method to correctly return all of the NIC's application_gateways' info (instead of only the first LB's info).
NOTE: All unit test specs are now passing again.

issue-644: multi-AGW: Update `VMManager._get_application_gateways` method to support multi-AGW (and also to use the new `ApplicationGatewayConfig.resource_group_name` property/data).

issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to init `ApplicationGatewayConfig.resource_group_name` as `nil`, unless a value is explicitly configured. (See code comment for details.)

issue-644: multi-AGW: Update `AzureClient.parse_network_interface` method to use the new `resource_group_name` keyword param of the `AzureClient.get_application_gateway_by_name` method.

issue-644: multi-AGW: Added a new, optional `resource_group_name` keyword param to the `AzureClient.get_application_gateway_by_name` method.

issue-644: multi-AGW: Updated `AzureClient.create_network_interface` method to support multi-AGW.

Renamed a spec example in the Integration test suite, to make the meaning clearer: application_gateway_spec.rb.

COMMENTS: Added/updated comments and doc comments: AzureClient, VMManager.

issue-644: multi-AGW: Renamed a key within the `nic_params` Hash (from "application_gateway" to "application_gateways").

issue-644: multi-AGW: Rename and update `VMManager._get_application_gateways` method

issue-644: multi-AGW: Renamed various vars/keys (from `application_gateway` to `application_gateways`)

issue-644: multi-AGW: Added additional specs for the `VMCloudProps._parse_application_gateway_config` method.

issue-644: multi-AGW: Add new `VMCloudProps._parse_application_gateway_config` method.
NOTE: This change is part of a multi-commit set of incremental changes. Some of the existing unit test specs are now failing. They will be fixed/updated by the time the incremental changes are completed.

src/bosh_azure_cpi/lib/cloud/azure/models/config.rb

@@ -13,6 +13,20 @@ def to_s
     end
   end
 
+  class ApplicationGatewayConfig
+    # TODO: issue-644: multi-BEPool-AGW: Add support for explicitly specified BEPool name.
+    attr_reader :name, :resource_group_name
+
+    def initialize(resource_group_name, name)
+      @resource_group_name = resource_group_name
+      @name = name
+    end
+
+    def to_s
+      "name: #{@name}, resource_group_name: #{@resource_group_name}"
+    end
+  end
+
   class AvailabilitySetConfig
     attr_reader :name
     attr_reader :platform_update_domain_count, :platform_fault_domain_count

src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb

@@ -10,7 +10,7 @@ class VMCloudProps
     attr_reader :availability_zone
     attr_reader :availability_set
     attr_reader :load_balancers
-    attr_reader :application_gateway
+    attr_reader :application_gateways
     attr_reader :managed_identity
     attr_reader :security_group
     attr_reader :application_security_groups
@@ -26,6 +26,7 @@ class VMCloudProps
 
     AVAILABILITY_SET_KEY = 'availability_set'
     LOAD_BALANCER_KEY = 'load_balancer'
+    APPLICATION_GATEWAY_KEY = 'application_gateway'
     RESOURCE_GROUP_NAME_KEY = 'resource_group_name'
     NAME_KEY = 'name'
 
@@ -57,7 +58,8 @@ def initialize(vm_properties, global_azure_config)
       cloud_error("Only one of 'availability_zone' and 'availability_set' is allowed to be configured for the VM but you have configured both.") if !@availability_zone.nil? && !@availability_set.name.nil?
 
       @load_balancers = _parse_load_balancer_config(vm_properties, global_azure_config)
-      @application_gateway = vm_properties['application_gateway']
+
+      @application_gateways = _parse_application_gateway_config(vm_properties)
 
       @managed_identity = global_azure_config.default_managed_identity
       managed_identity_hash = vm_properties.fetch('managed_identity', nil)
@@ -126,6 +128,35 @@ def _parse_load_balancer_config(vm_properties, global_azure_config)
       load_balancers.compact
     end
 
+    # @return [Array<Bosh::AzureCloud::ApplicationGatewayConfig>,nil]
+    def _parse_application_gateway_config(vm_properties)
+      application_gateway_config = vm_properties[APPLICATION_GATEWAY_KEY]
+
+      return nil unless application_gateway_config
+
+      cloud_error("Property '#{APPLICATION_GATEWAY_KEY}' must be a String, Hash, or Array.") unless application_gateway_config.is_a?(String) || application_gateway_config.is_a?(Hash) || application_gateway_config.is_a?(Array)
+
+      application_gateway_configs = application_gateway_config.is_a?(Array) ? application_gateway_config : [application_gateway_config]
+      application_gateways = Array(application_gateway_configs).flat_map do |agwc|
+        if agwc.is_a?(Hash)
+          application_gateway_names = agwc[NAME_KEY]
+          resource_group_name = agwc[RESOURCE_GROUP_NAME_KEY]
+        else
+          application_gateway_names = agwc
+          resource_group_name = nil
+        end
+        String(application_gateway_names).split(',').map do |application_gateway_name|
+          Bosh::AzureCloud::ApplicationGatewayConfig.new(
+            # NOTE: It is OK for the resource_group_name to be `nil` here. The nil will be defaulted elsewhere (if needed). And leaving it nil makes the specs simpler.
+            # resource_group_name || global_azure_config.resource_group_name,
+            resource_group_name,
+            application_gateway_name
+          )
+        end
+      end
+      application_gateways.compact
+    end
+
     # @return [Bosh::AzureCloud::AvailabilitySetConfig]
     def _parse_availability_set_config(vm_properties, global_azure_config)
       if vm_properties[AVAILABILITY_SET_KEY].is_a?(Hash)

src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb

@@ -1335,13 +1335,13 @@ def delete_public_ip(resource_group_name, name)
     # Network/Load Balancer
 
     # Get a load balancer's information
+    # @param [String,nil] resource_group_name - The load balancer's resource group name.
     # @param [String] name - Name of load balancer.
     #
     # @return [Hash]
     #
     # @See https://docs.microsoft.com/en-us/rest/api/load-balancer/loadbalancers/get
     #
-
     def get_load_balancer_by_name(resource_group_name, name)
       url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_LOAD_BALANCERS, resource_group_name: resource_group_name, name: name)
       _get_load_balancer(url)
@@ -1356,6 +1356,7 @@ def get_load_balancer_by_name(resource_group_name, name)
     #
     def _get_load_balancer(url)
       load_balancer = nil
+      # see: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/get#loadbalancer
       result = get_resource_by_id(url)
       unless result.nil?
         load_balancer = {}
@@ -1380,6 +1381,7 @@ def _get_load_balancer(url)
           load_balancer[:frontend_ip_configurations].push(ip)
         end
 
+        # see: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/get#backendaddresspool
         backend = properties['backendAddressPools']
         load_balancer[:backend_address_pools] = []
         backend.each do |backend_ip|
@@ -1418,8 +1420,8 @@ def _get_load_balancer(url)
     # * +:dns_servers                   - Array. DNS servers.
     # * +:network_security_group        - Hash. The network security group which the network interface is bound to.
     # * +:application_security_groups   - Array. The application security groups which the network interface is bound to.
-    # * +:load_balancers                - Array<Hash>. The load balancers which the network interface is bound to.
-    # * +:application_gateway           - Hash. The application gateway which the network interface is bound to.
+    # * +:load_balancers                - Array<Hash>. The load balancers which the network interface is bound to. (see: Bosh::AzureCloud::VMManager._get_load_balancers)
+    # * +:application_gateways          - Array<Hash>. The application gateways which the network interface is bound to. (see: Bosh::AzureCloud::VMManager._get_application_gateways)
     #
     # @return [Boolean]
     #
@@ -1462,6 +1464,7 @@ def create_network_interface(resource_group_name, nic_params)
       end
       interface['properties']['ipConfigurations'][0]['properties']['applicationSecurityGroups'] = application_security_groups unless application_security_groups.empty?
 
+      # see: Bosh::AzureCloud::VMManager._get_load_balancers
       load_balancers = nic_params[:load_balancers]
       unless load_balancers.nil?
         backend_pools = load_balancers.map { |load_balancer| {:id => load_balancer[:backend_address_pools][0][:id]} }
@@ -1475,13 +1478,12 @@ def create_network_interface(resource_group_name, nic_params)
         interface['properties']['ipConfigurations'][0]['properties']['loadBalancerInboundNatRules'] = inbound_nat_rules
       end
 
-      application_gateway = nic_params[:application_gateway]
-      unless application_gateway.nil?
-        interface['properties']['ipConfigurations'][0]['properties']['applicationGatewayBackendAddressPools'] = [
-          {
-            'id' => application_gateway[:backend_address_pools][0][:id]
-          }
-        ]
+      # see: Bosh::AzureCloud::VMManager._get_application_gateways
+      application_gateways = nic_params[:application_gateways]
+      unless application_gateways.nil?
+        # NOTE: backend_address_pools[0] should always be used. (When `application_gateway/backend_pool_name` is specified, the named pool will always be first here.)
+        backend_pools = application_gateways.map { |application_gateway| {:id => application_gateway[:backend_address_pools][0][:id]} }
+        interface['properties']['ipConfigurations'][0]['properties']['applicationGatewayBackendAddressPools'] = backend_pools
       end
 
       http_put(url, interface)
@@ -1620,14 +1622,15 @@ def get_network_subnet(url)
     # Network/Application Gateway
 
     # Get an application gateway's information
+    # @param [String,nil] resource_group_name - The application gateway's resource group name.
     # @param [String] name - Name of application gateway.
     #
     # @return [Hash]
     #
     # @See https://docs.microsoft.com/en-us/rest/api/application-gateway/applicationgateways/get
     #
-    def get_application_gateway_by_name(name)
-      url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_APPLICATION_GATEWAYS, name: name)
+    def get_application_gateway_by_name(resource_group_name, name)
+      url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_APPLICATION_GATEWAYS, resource_group_name: resource_group_name, name: name)
       get_application_gateway(url)
     end
 
@@ -1640,6 +1643,7 @@ def get_application_gateway_by_name(name)
     #
     def get_application_gateway(url)
       application_gateway = nil
+      # see: https://docs.microsoft.com/en-us/rest/api/application-gateway/application-gateways/get#applicationgateway
       result = get_resource_by_id(url)
       unless result.nil?
         application_gateway = {}
@@ -1649,6 +1653,8 @@ def get_application_gateway(url)
         application_gateway[:tags] = result['tags']
 
         properties = result['properties']
+        # TODO: issue-644: multi-BEPool-AGW: Review: Already supports multiple ApplicationGateway Backend Address Pools?
+        # see: https://docs.microsoft.com/en-us/rest/api/application-gateway/application-gateways/get#applicationgatewaybackendaddresspool
         backend = properties['backendAddressPools']
         application_gateway[:backend_address_pools] = []
         backend.each do |backend_ip|
@@ -2065,13 +2071,18 @@ def parse_network_interface(result, recursive: true)
           end
           interface[:load_balancers] = load_balancers
         end
-        unless ip_configuration_properties['applicationGatewayBackendAddressPools'].nil?
-          if recursive
-            names = _parse_name_from_id(ip_configuration_properties['applicationGatewayBackendAddressPools'][0]['id'])
-            interface[:application_gateway] = get_application_gateway_by_name(names[:resource_name])
-          else
-            interface[:application_gateway] = { id: ip_configuration_properties['applicationGatewayBackendAddressPools'][0]['id'] }
+        application_gateway_backend_pools = ip_configuration_properties['applicationGatewayBackendAddressPools']
+        unless application_gateway_backend_pools.nil?
+          application_gateways = application_gateway_backend_pools.map do |agw_backend_pool|
+            if recursive
+              names = _parse_name_from_id(agw_backend_pool['id'])
+              application_gateway = get_application_gateway_by_name(names[:resource_group_name], names[:resource_name])
+            else
+              application_gateway = { id: agw_backend_pool['id'] }
+            end
+            application_gateway
           end
+          interface[:application_gateways] = application_gateways
         end
         unless ip_configuration_properties['applicationSecurityGroups'].nil?
           asgs_properties = ip_configuration_properties['applicationSecurityGroups']

src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb

@@ -89,6 +89,7 @@ def _get_public_ip(vip_network)
     end
 
     # @return [Array<Hash>]
+    # @see Bosh::AzureCloud::AzureClient.create_network_interface
     def _get_load_balancers(vm_props)
       load_balancers = nil
       load_balancer_configs = vm_props.load_balancers
@@ -102,14 +103,19 @@ def _get_load_balancers(vm_props)
       load_balancers
     end
 
-    def _get_application_gateway(vm_props)
-      application_gateway = nil
-      unless vm_props.application_gateway.nil?
-        application_gateway_name = vm_props.application_gateway
-        application_gateway = @azure_client.get_application_gateway_by_name(application_gateway_name)
-        cloud_error("Cannot find the application gateway '#{application_gateway_name}'") if application_gateway.nil?
+    # @return [Array<Hash>]
+    # @see Bosh::AzureCloud::AzureClient.create_network_interface
+    def _get_application_gateways(vm_props)
+      application_gateways = nil
+      application_gateway_configs = vm_props.application_gateways
+      unless application_gateway_configs.nil?
+        application_gateways = application_gateway_configs.map do |application_gateway_config|
+          application_gateway = @azure_client.get_application_gateway_by_name(application_gateway_config.resource_group_name, application_gateway_config.name)
+          cloud_error("Cannot find the application gateway '#{application_gateway_config.name}'") if application_gateway.nil?
+          application_gateway
+        end
       end
-      application_gateway
+      application_gateways
     end
 
     def _get_or_create_public_ip(resource_group_name, vm_name, location, vm_props, network_configurator)
@@ -131,11 +137,12 @@ def _get_or_create_public_ip(resource_group_name, vm_name, location, vm_props, n
       public_ip
     end
 
+    # @return [Array<Hash>] one Hash (returned by Bosh::AzureCloud::AzureClient.get_network_interface_by_name)  per network interface created
     def _create_network_interfaces(resource_group_name, vm_name, location, vm_props, network_configurator, primary_nic_tags = AZURE_TAGS)
       # Tasks to prepare before creating NICs:
       #   * prepare public ip
       #   * prepare load balancer(s)
-      #   * prepare application gateway
+      #   * prepare application gateway(s)
       tasks_preparing = []
 
       tasks_preparing.push(
@@ -149,8 +156,8 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props,
         end
       )
       tasks_preparing.push(
-        task_get_application_gateway = Concurrent::Future.execute do
-          _get_application_gateway(vm_props)
+        task_get_application_gateways = Concurrent::Future.execute do
+          _get_application_gateways(vm_props)
         end
       )
 
@@ -159,7 +166,7 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props,
 
       public_ip = task_get_or_create_public_ip.value!
       load_balancers = task_get_load_balancers.value!
-      application_gateway = task_get_application_gateway.value!
+      application_gateways = task_get_application_gateways.value!
 
       # tasks to create NICs, NICs will be created in different threads
       tasks_creating = []
@@ -187,12 +194,12 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props,
           nic_params[:public_ip] = public_ip
           nic_params[:tags] = primary_nic_tags
           nic_params[:load_balancers] = load_balancers
-          nic_params[:application_gateway] = application_gateway
+          nic_params[:application_gateways] = application_gateways
         else
           nic_params[:public_ip] = nil
           nic_params[:tags] = AZURE_TAGS
           nic_params[:load_balancers] = nil
-          nic_params[:application_gateway] = nil
+          nic_params[:application_gateways] = nil
         end
         tasks_creating.push(
           Concurrent::Future.execute do

src/bosh_azure_cpi/spec/integration/resources/application_gateway_spec.rb

@@ -7,7 +7,7 @@
     @application_gateway_name = ENV.fetch('BOSH_AZURE_APPLICATION_GATEWAY_NAME')
   end
 
-  context 'when application_gateway is specified in resource pool' do
+  context 'when single application_gateway is specified in resource pool' do
     let(:network_spec) do
       {
         'network_a' => {

src/bosh_azure_cpi/spec/unit/azure_client/create_network_interface_spec.rb

@@ -56,7 +56,7 @@
             network_security_group: { id: nsg_id },
             application_security_groups: [],
             load_balancers: nil,
-            application_gateway: nil
+            application_gateways: nil
           }
         end
         let(:request_body) do
@@ -131,7 +131,7 @@
             network_security_group: { id: nsg_id },
             application_security_groups: [],
             load_balancers: nil,
-            application_gateway: nil
+            application_gateways: nil
           }
         end
         let(:request_body) do
@@ -209,7 +209,7 @@
             network_security_group: nil,
             application_security_groups: [],
             load_balancers: nil,
-            application_gateway: nil
+            application_gateways: nil
           }
         end
         let(:request_body) do
@@ -296,7 +296,7 @@
                 }
               ]
             }],
-            application_gateway: nil
+            application_gateways: nil
           }
         end
 
@@ -381,7 +381,7 @@
             network_security_group: { id: nsg_id },
             application_security_groups: [{ id: 'fake-asg-id-1' }, { id: 'fake-asg-id-2' }],
             load_balancers: nil,
-            application_gateway: nil
+            application_gateways: nil
           }
         end
         let(:request_body) do
@@ -467,13 +467,13 @@
             network_security_group: { id: nsg_id },
             application_security_groups: [],
             load_balancers: nil,
-            application_gateway: {
+            application_gateways: [{
               backend_address_pools: [
                 {
                   id: 'fake-id-2'
                 }
               ]
-            }
+            }]
           }
         end
         let(:request_body) do