From 8506b7bfdc8409ed13c366491a263adbe4b5f2ae Mon Sep 17 00:00:00 2001 From: ZhijunZhao Date: Tue, 5 Dec 2017 07:41:34 +0800 Subject: [PATCH] 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 | 6 +- .../modules/cloud/azure/azure_rm_acs.py | 116 +++++++++++------- packaging/requirements/requirements-azure.txt | 2 +- .../requirements/integration.cloud.azure.txt | 2 +- 4 files changed, 79 insertions(+), 47 deletions(-) diff --git a/lib/ansible/module_utils/azure_rm_common.py b/lib/ansible/module_utils/azure_rm_common.py index 515c585d4a2..8e6c94d3d20 100644 --- a/lib/ansible/module_utils/azure_rm_common.py +++ b/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 diff --git a/lib/ansible/modules/cloud/azure/azure_rm_acs.py b/lib/ansible/modules/cloud/azure/azure_rm_acs.py index 79eadcc2929..cdd7027e1db 100644 --- a/lib/ansible/modules/cloud/azure/azure_rm_acs.py +++ b/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 = [] - for profile in agentpoolprofiles: - result = dict( - count=profile.count, - vm_size=profile.vm_size, - name=profile.name, - dns_prefix=profile.dns_prefix, - fqdn=profile.fqdn - ) - results.append(result) - return results + return [dict( + count=profile.count, + vm_size=profile.vm_size, + name=profile.name, + dns_prefix=profile.dns_prefix, + fqdn=profile.fqdn + ) for profile in agentpoolprofiles] 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'], - profile_result['vm_size'], profile_self['vm_size']))) + .format(profile_result['count'], profile_self['count'], + 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() diff --git a/packaging/requirements/requirements-azure.txt b/packaging/requirements/requirements-azure.txt index cc2ce7e006e..adb575510a3 100644 --- a/packaging/requirements/requirements-azure.txt +++ b/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 \ No newline at end of file +azure-mgmt-containerservice>=2.0.0,<3.0.0 \ No newline at end of file diff --git a/test/runner/requirements/integration.cloud.azure.txt b/test/runner/requirements/integration.cloud.azure.txt index cc2ce7e006e..adb575510a3 100644 --- a/test/runner/requirements/integration.cloud.azure.txt +++ b/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 \ No newline at end of file +azure-mgmt-containerservice>=2.0.0,<3.0.0 \ No newline at end of file