Fix azure_rm_acs due to Azure breaking API change (#33221)

* Fix azure_rm_acs due to Azure breaking API change

* resolve code review feedbacks

* resolve test failures

* better name

* ignore case for vm size

* reformat code

* resolve code review feedbacks

lib/ansible/module_utils/azure_rm_common.py

@@ -780,12 +780,14 @@ class AzureRMModuleBase(object):
     def web_client(self):
         self.log('Getting web client')
         if not self._web_client:
-            self._web_client = self.get_mgmt_svc_client(WebSiteManagementClient, base_url=self.base_url)
+            self._web_client = self.get_mgmt_svc_client(WebSiteManagementClient,
+                                                        base_url=self._cloud_environment.endpoints.resource_manager)
         return self._web_client
 
     @property
     def containerservice_client(self):
         self.log('Getting container service client')
         if not self._containerservice_client:
-            self._containerservice_client = self.get_mgmt_svc_client(ContainerServiceClient)
+            self._containerservice_client = self.get_mgmt_svc_client(ContainerServiceClient,
+                                                                     base_url=self._cloud_environment.endpoints.resource_manager)
         return self._containerservice_client

lib/ansible/modules/cloud/azure/azure_rm_acs.py

@@ -5,14 +5,13 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
-__metaclass__ = type
 
+__metaclass__ = type
 
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
                     'supported_by': 'community'}
 
-
 DOCUMENTATION = '''
 ---
 module: azure_rm_acs
@@ -30,7 +29,6 @@ options:
         description:
             - Name of the Container Services instance.
         required: true
-        default: null
     state:
         description:
             - Assert the state of the ACS. Use 'present' to create or update an ACS and 'absent' to delete it.
@@ -52,7 +50,6 @@ options:
         description:
             - Master profile suboptions.
         required: true
-        default: null
         suboptions:
             count:
                 description:
@@ -62,6 +59,11 @@ options:
                   - 1
                   - 3
                   - 5
+            vm_size:
+                description:
+                    - The VM Size of each of the Agent Pool VM's (e.g. Standard_F1 / Standard_D2v2).
+                required: true
+                version_added: 2.5
             dns_prefix:
                 description:
                   - The DNS Prefix to use for the Container Service master nodes.
@@ -70,13 +72,11 @@ options:
         description:
             - The linux profile suboptions.
         required: true
-        default: null
         suboptions:
             admin_username:
                 description:
                   - The Admin Username for the Cluster.
                 required: true
-                default: azureuser
             ssh_key:
                 description:
                     - The Public SSH Key used to access the cluster.
@@ -85,7 +85,6 @@ options:
         description:
             - The agent pool profile suboptions.
         required: true
-        default: null
         suboptions:
             name:
                 description:
@@ -95,7 +94,6 @@ options:
                 description:
                     - Number of agents (VMs) to host docker containers. Allowed values must be in the range of 1 to 100 (inclusive).
                 required: true
-                default: 1
             dns_prefix:
                 description:
                     - The DNS Prefix given to Agents in this Agent Pool.
@@ -104,7 +102,6 @@ options:
                 description:
                     - The VM Size of each of the Agent Pool VM's (e.g. Standard_F1 / Standard_D2v2).
                 required: true
-                default: Standard_D2v2
     service_principal:
         description:
             - The service principal suboptions.
@@ -123,7 +120,6 @@ options:
         description:
             - Should VM Diagnostics be enabled for the Container Service VM's.
         required: true
-        default: false
 
 extends_documentation_fragment:
     - azure
@@ -144,6 +140,7 @@ EXAMPLES = '''
         master_profile:
             - count: 3
               dns_prefix: acsk8smasterdns
+              vm_size: Standard_D2_v2
         linux_profile:
             - admin_username: azureuser
               ssh_key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAA...
@@ -168,6 +165,7 @@ EXAMPLES = '''
         master_profile:
             - count: 3
               dns_prefix: acsdcosmasterdns
+              vm_size: Standard_D2_v2
         linux_profile:
             - admin_username: azureuser
               ssh_key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAA...
@@ -189,6 +187,7 @@ EXAMPLES = '''
         master_profile:
             - count: 3
               dns_prefix: acsswarmmasterdns
+              vm_size: Standard_D2_v2
         linux_profile:
             - admin_username: azureuser
               ssh_key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAA...
@@ -214,13 +213,11 @@ EXAMPLES = '''
         orchestration_platform: Swarm
         master_profile:
             - count: 1
+              vm_size: Standard_A0
               dns_prefix: acstestingmasterdns5
         linux_profile:
             - admin_username: azureuser
               ssh_key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAA...
-        service_principal:
-            - client_id: 7fb4173c-3ca3-4d5b-87f8-1daac941207a
-              client_secret: MPNSuM1auUuITefiLGBrpZZnLMDKBLw2
         agent_pool_profiles:
             - name: default
               count: 4
@@ -320,12 +317,17 @@ def create_ssh_configuration_instance(sshconf):
 def create_master_profile_instance(masterprofile):
     '''
     Helper method to serialize a dict to a ContainerServiceMasterProfile
+    Note: first_consecutive_static_ip is specifically set to None, for Azure server doesn't accept
+    request body with this property. This should be an inconsistency bug before Azure client SDK
+    and Azure server.
     :param: masterprofile: dict with the parameters to setup the ContainerServiceMasterProfile
     :return: ContainerServiceMasterProfile
     '''
     return ContainerServiceMasterProfile(
         count=masterprofile[0]['count'],
-        dns_prefix=masterprofile[0]['dns_prefix']
+        dns_prefix=masterprofile[0]['dns_prefix'],
+        vm_size=masterprofile[0]['vm_size'],
+        first_consecutive_static_ip=None
     )
 
 
@@ -357,7 +359,11 @@ def create_acs_dict(acs):
     :param: acs: ContainerService or AzureOperationPoller with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = dict(
+    service_principal_profile_dict = None
+    if acs.orchestrator_profile.orchestrator_type == 'Kubernetes':
+        service_principal_profile_dict = create_service_principal_profile_dict(acs.service_principal_profile)
+
+    return dict(
         id=acs.id,
         name=acs.name,
         location=acs.location,
@@ -365,13 +371,12 @@ def create_acs_dict(acs):
         orchestrator_profile=create_orchestrator_profile_dict(acs.orchestrator_profile),
         master_profile=create_master_profile_dict(acs.master_profile),
         linux_profile=create_linux_profile_dict(acs.linux_profile),
-        service_principal_profile=acs.service_principal_profile,
+        service_principal_profile=service_principal_profile_dict,
         diagnostics_profile=create_diagnotstics_profile_dict(acs.diagnostics_profile),
         provisioning_state=acs.provisioning_state,
         agent_pool_profiles=create_agent_pool_profiles_dict(acs.agent_pool_profiles),
         type=acs.type
     )
-    return results
 
 
 def create_linux_profile_dict(linuxprofile):
@@ -380,11 +385,10 @@ def create_linux_profile_dict(linuxprofile):
     :param: linuxprofile: ContainerServiceLinuxProfile with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = dict(
+    return dict(
         ssh_key=linuxprofile.ssh.public_keys[0].key_data,
         admin_username=linuxprofile.admin_username
     )
-    return results
 
 
 def create_master_profile_dict(masterprofile):
@@ -393,12 +397,24 @@ def create_master_profile_dict(masterprofile):
     :param: masterprofile: ContainerServiceMasterProfile with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = dict(
+    return dict(
         count=masterprofile.count,
         fqdn=masterprofile.fqdn,
+        vm_size=masterprofile.vm_size,
         dns_prefix=masterprofile.dns_prefix
     )
-    return results
+
+
+def create_service_principal_profile_dict(serviceprincipalprofile):
+    '''
+    Helper method to deserialize a ContainerServiceServicePrincipalProfile to a dict
+    Note: For security reason, the service principal secret is skipped on purpose.
+    :param: serviceprincipalprofile: ContainerServiceServicePrincipalProfile with the Azure callback object
+    :return: dict with the state on Azure
+    '''
+    return dict(
+        client_id=serviceprincipalprofile.client_id
+    )
 
 
 def create_diagnotstics_profile_dict(diagnosticsprofile):
@@ -407,10 +423,9 @@ def create_diagnotstics_profile_dict(diagnosticsprofile):
     :param: diagnosticsprofile: ContainerServiceVMDiagnostics with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = dict(
+    return dict(
         vm_diagnostics=diagnosticsprofile.vm_diagnostics.enabled
     )
-    return results
 
 
 def create_orchestrator_profile_dict(orchestratorprofile):
@@ -419,10 +434,9 @@ def create_orchestrator_profile_dict(orchestratorprofile):
     :param: orchestratorprofile: ContainerServiceOrchestratorProfile with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = dict(
+    return dict(
         orchestrator_type=str(orchestratorprofile.orchestrator_type)
     )
-    return results
 
 
 def create_agent_pool_profiles_dict(agentpoolprofiles):
@@ -431,17 +445,13 @@ def create_agent_pool_profiles_dict(agentpoolprofiles):
     :param: agentpoolprofiles: ContainerServiceAgentPoolProfile with the Azure callback object
     :return: dict with the state on Azure
     '''
-    results = []
+    return [dict(
-    for profile in agentpoolprofiles:
+        count=profile.count,
-        result = dict(
+        vm_size=profile.vm_size,
-            count=profile.count,
+        name=profile.name,
-            vm_size=profile.vm_size,
+        dns_prefix=profile.dns_prefix,
-            name=profile.name,
+        fqdn=profile.fqdn
-            dns_prefix=profile.dns_prefix,
+    ) for profile in agentpoolprofiles]
-            fqdn=profile.fqdn
-        )
-        results.append(result)
-    return results
 
 
 class AzureRMContainerService(AzureRMModuleBase):
@@ -560,16 +570,33 @@ class AzureRMContainerService(AzureRMModuleBase):
                     if update_tags:
                         to_be_updated = True
 
+                    def is_property_changed(profile, property, ignore_case=False):
+                        base = response[profile].get(property)
+                        new = getattr(self, profile)[0].get(property)
+                        if ignore_case:
+                            return base.lower() != new.lower()
+                        else:
+                            return base != new
+
                     # Cannot Update the master count for now // Uncomment this block in the future to support it
-                    if response['master_profile'].get('count') != self.master_profile[0].get('count'):
+                    if is_property_changed('master_profile', 'count'):
                         # self.log(("Master Profile Count Diff, Was {0} / Now {1}"
                         #           .format(response['master_profile'].count,
                         #           self.master_profile[0].get('count'))))
                         # to_be_updated = True
                         self.module.warn("master_profile.count cannot be updated")
 
+                    # Cannot Update the master vm_size for now. Could be a client SDK bug
+                    # Uncomment this block in the future to support it
+                    if is_property_changed('master_profile', 'vm_size', True):
+                        # self.log(("Master Profile VM Size Diff, Was {0} / Now {1}"
+                        #           .format(response['master_profile'].get('vm_size'),
+                        #                   self.master_profile[0].get('vm_size'))))
+                        # to_be_updated = True
+                        self.module.warn("master_profile.vm_size cannot be updated")
+
                     # Cannot Update the SSH Key for now // Uncomment this block in the future to support it
-                    if response['linux_profile'].get('ssh_key') != self.linux_profile[0].get('ssh_key'):
+                    if is_property_changed('linux_profile', 'ssh_key'):
                         # self.log(("Linux Profile Diff SSH, Was {0} / Now {1}"
                         #          .format(response['linux_profile'].ssh.public_keys[0].key_data,
                         #          self.linux_profile[0].get('ssh_key'))))
@@ -579,7 +606,7 @@ class AzureRMContainerService(AzureRMModuleBase):
                     # self.log("linux_profile response : {0}".format(response['linux_profile'].get('admin_username')))
                     # self.log("linux_profile self : {0}".format(self.linux_profile[0].get('admin_username')))
                     # Cannot Update the Username for now // Uncomment this block in the future to support it
-                    if response['linux_profile'].get('admin_username') != self.linux_profile[0].get('admin_username'):
+                    if is_property_changed('linux_profile', 'admin_username'):
                         # self.log(("Linux Profile Diff User, Was {0} / Now {1}"
                         #          .format(response['linux_profile'].admin_username,
                         #          self.linux_profile[0].get('admin_username'))))
@@ -596,10 +623,11 @@ class AzureRMContainerService(AzureRMModuleBase):
                         for profile_self in self.agent_pool_profiles:
                             if profile_result['name'] == profile_self['name']:
                                 matched = True
-                                if profile_result['count'] != profile_self['count'] or profile_result['vm_size'] != profile_self['vm_size']:
+                                if profile_result['count'] != profile_self['count'] or profile_result['vm_size'] != \
+                                        profile_self['vm_size']:
                                     self.log(("Agent Profile Diff - Count was {0} / Now {1} - Vm_size was {2} / Now {3}"
-                                             .format(profile_result['count'], profile_self['count'],
+                                              .format(profile_result['count'], profile_self['count'],
-                                              profile_result['vm_size'], profile_self['vm_size'])))
+                                                      profile_result['vm_size'], profile_self['vm_size'])))
                                     to_be_updated = True
                         if not matched:
                             self.log("Agent Pool not found")
@@ -663,7 +691,8 @@ class AzureRMContainerService(AzureRMModuleBase):
         # self.log("vm_diagnostics : {0}".format(parameters.diagnostics_profile.vm_diagnostics))
 
         try:
-            poller = self.containerservice_client.container_services.create_or_update(self.resource_group, self.name, parameters)
+            poller = self.containerservice_client.container_services.create_or_update(self.resource_group, self.name,
+                                                                                      parameters)
             response = self.get_poller_result(poller)
         except CloudError as exc:
             self.log('Error attempting to create the ACS instance.')
@@ -714,5 +743,6 @@ def main():
     """Main execution"""
     AzureRMContainerService()
 
+
 if __name__ == '__main__':
     main()

packaging/requirements/requirements-azure.txt

@@ -13,4 +13,4 @@ azure-mgmt-keyvault>=0.40.0,<0.41
 azure-mgmt-batch>=4.1.0,<5
 azure-mgmt-sql>=0.7.1,<0.8
 azure-mgmt-web>=0.32.0,<0.33
-azure-mgmt-containerservice>=1.0.0
+azure-mgmt-containerservice>=2.0.0,<3.0.0

test/runner/requirements/integration.cloud.azure.txt

@@ -13,4 +13,4 @@ azure-mgmt-keyvault>=0.40.0,<0.41
 azure-mgmt-batch>=4.1.0,<5
 azure-mgmt-sql>=0.7.1,<0.8
 azure-mgmt-web>=0.32.0,<0.33
-azure-mgmt-containerservice>=1.0.0
+azure-mgmt-containerservice>=2.0.0,<3.0.0