diff --git a/src/azure-cli/azure/cli/command_modules/acs/addonconfiguration.py b/src/azure-cli/azure/cli/command_modules/acs/addonconfiguration.py index eb3a771d80b..7ab979c21f9 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/addonconfiguration.py +++ b/src/azure-cli/azure/cli/command_modules/acs/addonconfiguration.py @@ -206,12 +206,8 @@ def ensure_default_log_analytics_workspace_for_monitoring( if ex.status_code != 404: raise ex else: - ResourceGroup = cmd.get_models( - "ResourceGroup", resource_type=ResourceType.MGMT_RESOURCE_RESOURCES - ) - resource_group = ResourceGroup(location=workspace_region) resource_groups.create_or_update( - default_workspace_resource_group, resource_group + default_workspace_resource_group, {"location": workspace_region} ) GenericResource = cmd.get_models( @@ -314,7 +310,7 @@ def ensure_container_insights_for_monitoring( f"/subscriptions/{cluster_subscription}/resourceGroups/{cluster_resource_group_name}/" f"providers/Microsoft.ContainerService/managedClusters/{cluster_name}" ) - dataCollectionRuleName = f"DCR-{workspace_name}" + dataCollectionRuleName = f"MSCI-{workspace_name}" dcr_resource_id = ( f"/subscriptions/{subscription_id}/resourceGroups/{resource_group}/" f"providers/Microsoft.Insights/dataCollectionRules/{dataCollectionRuleName}" diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 918c628e1f1..2bddc84f59f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -114,6 +114,14 @@ from ._consts import CONST_PRIVATE_DNS_ZONE_NONE from ._consts import CONST_MANAGED_IDENTITY_OPERATOR_ROLE, CONST_MANAGED_IDENTITY_OPERATOR_ROLE_ID from ._consts import DecoratorEarlyExitException +from .addonconfiguration import ( + add_monitoring_role_assignment, + add_ingress_appgw_addon_role_assignment, + add_virtual_node_role_assignment, + ensure_default_log_analytics_workspace_for_monitoring, + ensure_container_insights_for_monitoring, +) +from ._resourcegroup import get_rg_location logger = get_logger(__name__) @@ -937,7 +945,7 @@ def acs_create(cmd, client, resource_group_name, deployment_name, name, ssh_key_ dns_name_prefix = _get_default_dns_prefix( name, resource_group_name, subscription_id) - rg_location = _get_rg_location(cmd.cli_ctx, resource_group_name) + rg_location = get_rg_location(cmd.cli_ctx, resource_group_name) if location is None: location = rg_location @@ -1883,136 +1891,6 @@ def _validate_ssh_key(no_ssh_key, ssh_key_value): 'Provided ssh key ({}) is invalid or non-existent'.format(shortened_key)) -def _add_monitoring_role_assignment(result, cluster_resource_id, cmd): - service_principal_msi_id = None - # Check if service principal exists, if it does, assign permissions to service principal - # Else, provide permissions to MSI - if ( - hasattr(result, 'service_principal_profile') and - hasattr(result.service_principal_profile, 'client_id') and - result.service_principal_profile.client_id.lower() != 'msi' - ): - logger.info('valid service principal exists, using it') - service_principal_msi_id = result.service_principal_profile.client_id - is_service_principal = True - elif ( - (hasattr(result, 'addon_profiles')) and - (CONST_MONITORING_ADDON_NAME in result.addon_profiles) and - (hasattr(result.addon_profiles[CONST_MONITORING_ADDON_NAME], 'identity')) and - (hasattr( - result.addon_profiles[CONST_MONITORING_ADDON_NAME].identity, 'object_id')) - ): - logger.info('omsagent MSI exists, using it') - service_principal_msi_id = result.addon_profiles[CONST_MONITORING_ADDON_NAME].identity.object_id - is_service_principal = False - - if service_principal_msi_id is not None: - if not _add_role_assignment(cmd, 'Monitoring Metrics Publisher', - service_principal_msi_id, is_service_principal, scope=cluster_resource_id): - logger.warning('Could not create a role assignment for Monitoring addon. ' - 'Are you an Owner on this subscription?') - else: - logger.warning('Could not find service principal or user assigned MSI for role' - 'assignment') - - -def _add_ingress_appgw_addon_role_assignment(result, cmd): - service_principal_msi_id = None - # Check if service principal exists, if it does, assign permissions to service principal - # Else, provide permissions to MSI - if ( - hasattr(result, 'service_principal_profile') and - hasattr(result.service_principal_profile, 'client_id') and - result.service_principal_profile.client_id != 'msi' - ): - service_principal_msi_id = result.service_principal_profile.client_id - is_service_principal = True - elif ( - (hasattr(result, 'addon_profiles')) and - (CONST_INGRESS_APPGW_ADDON_NAME in result.addon_profiles) and - (hasattr(result.addon_profiles[CONST_INGRESS_APPGW_ADDON_NAME], 'identity')) and - (hasattr( - result.addon_profiles[CONST_INGRESS_APPGW_ADDON_NAME].identity, 'object_id')) - ): - service_principal_msi_id = result.addon_profiles[ - CONST_INGRESS_APPGW_ADDON_NAME].identity.object_id - is_service_principal = False - - if service_principal_msi_id is not None: - config = result.addon_profiles[CONST_INGRESS_APPGW_ADDON_NAME].config - from msrestazure.tools import parse_resource_id, resource_id - if CONST_INGRESS_APPGW_APPLICATION_GATEWAY_ID in config: - appgw_id = config[CONST_INGRESS_APPGW_APPLICATION_GATEWAY_ID] - parsed_appgw_id = parse_resource_id(appgw_id) - appgw_group_id = resource_id(subscription=parsed_appgw_id["subscription"], - resource_group=parsed_appgw_id["resource_group"]) - if not _add_role_assignment(cmd, 'Contributor', - service_principal_msi_id, is_service_principal, scope=appgw_group_id): - logger.warning('Could not create a role assignment for application gateway: %s ' - 'specified in %s addon. ' - 'Are you an Owner on this subscription?', appgw_id, CONST_INGRESS_APPGW_ADDON_NAME) - if CONST_INGRESS_APPGW_SUBNET_ID in config: - subnet_id = config[CONST_INGRESS_APPGW_SUBNET_ID] - if not _add_role_assignment(cmd, 'Network Contributor', - service_principal_msi_id, is_service_principal, scope=subnet_id): - logger.warning('Could not create a role assignment for subnet: %s ' - 'specified in %s addon. ' - 'Are you an Owner on this subscription?', subnet_id, CONST_INGRESS_APPGW_ADDON_NAME) - if CONST_INGRESS_APPGW_SUBNET_CIDR in config: - if result.agent_pool_profiles[0].vnet_subnet_id is not None: - parsed_subnet_vnet_id = parse_resource_id( - result.agent_pool_profiles[0].vnet_subnet_id) - vnet_id = resource_id(subscription=parsed_subnet_vnet_id["subscription"], - resource_group=parsed_subnet_vnet_id["resource_group"], - namespace="Microsoft.Network", - type="virtualNetworks", - name=parsed_subnet_vnet_id["name"]) - if not _add_role_assignment(cmd, 'Contributor', - service_principal_msi_id, is_service_principal, scope=vnet_id): - logger.warning('Could not create a role assignment for virtual network: %s ' - 'specified in %s addon. ' - 'Are you an Owner on this subscription?', vnet_id, CONST_INGRESS_APPGW_ADDON_NAME) - - -def _add_virtual_node_role_assignment(cmd, result, vnet_subnet_id): - # Remove trailing "/subnets/" to get the vnet id - vnet_id = vnet_subnet_id.rpartition('/')[0] - vnet_id = vnet_id.rpartition('/')[0] - - service_principal_msi_id = None - is_service_principal = False - os_type = 'Linux' - addon_name = CONST_VIRTUAL_NODE_ADDON_NAME + os_type - # Check if service principal exists, if it does, assign permissions to service principal - # Else, provide permissions to MSI - if ( - hasattr(result, 'service_principal_profile') and - hasattr(result.service_principal_profile, 'client_id') and - result.service_principal_profile.client_id.lower() != 'msi' - ): - logger.info('valid service principal exists, using it') - service_principal_msi_id = result.service_principal_profile.client_id - is_service_principal = True - elif ( - (hasattr(result, 'addon_profiles')) and - (addon_name in result.addon_profiles) and - (hasattr(result.addon_profiles[addon_name], 'identity')) and - (hasattr(result.addon_profiles[addon_name].identity, 'object_id')) - ): - logger.info('virtual node MSI exists, using it') - service_principal_msi_id = result.addon_profiles[addon_name].identity.object_id - is_service_principal = False - - if service_principal_msi_id is not None: - if not _add_role_assignment(cmd, 'Contributor', - service_principal_msi_id, is_service_principal, scope=vnet_id): - logger.warning('Could not create a role assignment for virtual node addon. ' - 'Are you an Owner on this subscription?') - else: - logger.warning('Could not find service principal or user assigned MSI for role' - 'assignment') - - # pylint: disable=too-many-statements,too-many-branches def aks_create(cmd, client, resource_group_name, name, ssh_key_value, # pylint: disable=too-many-locals dns_name_prefix=None, @@ -2180,8 +2058,15 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons, if need_pull_for_result: if enable_monitoring: - _ensure_container_insights_for_monitoring( - cmd, instance.addon_profiles[CONST_MONITORING_ADDON_NAME]) + ensure_container_insights_for_monitoring( + cmd, + instance.addon_profiles[CONST_MONITORING_ADDON_NAME], + subscription_id, + resource_group_name, + name, + instance.location, + aad_route=False, + ) # adding a wait here since we rely on the result for role assignment result = LongRunningOperation(cmd.cli_ctx)( @@ -2198,18 +2083,18 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons, namespace='Microsoft.ContainerService', type='managedClusters', name=name ) - _add_monitoring_role_assignment( + add_monitoring_role_assignment( result, cluster_resource_id, cmd) if ingress_appgw_addon_enabled: - _add_ingress_appgw_addon_role_assignment(result, cmd) + add_ingress_appgw_addon_role_assignment(result, cmd) if enable_virtual_node: # All agent pool will reside in the same vnet, we will grant vnet level Contributor role # in later function, so using a random agent pool here is OK random_agent_pool = result.agent_pool_profiles[0] if random_agent_pool.vnet_subnet_id != "": - _add_virtual_node_role_assignment( + add_virtual_node_role_assignment( cmd, result, random_agent_pool.vnet_subnet_id) # Else, the cluster is not using custom VNet, the permission is already granted in AKS RP, # we don't need to handle it in client side in this case. @@ -3082,7 +2967,7 @@ def _update_addons(cmd, instance, subscription_id, resource_group_name, name, ad 'To change monitoring configuration, run "az aks disable-addons -a monitoring"' 'before enabling it again.') if not workspace_resource_id: - workspace_resource_id = _ensure_default_log_analytics_workspace_for_monitoring( + workspace_resource_id = ensure_default_log_analytics_workspace_for_monitoring( cmd, subscription_id, resource_group_name) @@ -3221,7 +3106,7 @@ def _handle_addons_args(cmd, addons_str, subscription_id, resource_group_name, a if 'monitoring' in addons: if not workspace_resource_id: # use default workspace if exists else create default workspace - workspace_resource_id = _ensure_default_log_analytics_workspace_for_monitoring( + workspace_resource_id = ensure_default_log_analytics_workspace_for_monitoring( cmd, subscription_id, resource_group_name) workspace_resource_id = workspace_resource_id.strip() @@ -3333,197 +3218,6 @@ def _get_or_add_extension(cmd, extension_name, extension_module, update=False): return True -def _ensure_default_log_analytics_workspace_for_monitoring(cmd, subscription_id, resource_group_name): - # mapping for azure public cloud - # log analytics workspaces cannot be created in WCUS region due to capacity limits - # so mapped to EUS per discussion with log analytics team - AzureCloudLocationToOmsRegionCodeMap = { - "australiasoutheast": "ASE", - "australiaeast": "EAU", - "australiacentral": "CAU", - "canadacentral": "CCA", - "centralindia": "CIN", - "centralus": "CUS", - "eastasia": "EA", - "eastus": "EUS", - "eastus2": "EUS2", - "eastus2euap": "EAP", - "francecentral": "PAR", - "japaneast": "EJP", - "koreacentral": "SE", - "northeurope": "NEU", - "southcentralus": "SCUS", - "southeastasia": "SEA", - "uksouth": "SUK", - "usgovvirginia": "USGV", - "westcentralus": "EUS", - "westeurope": "WEU", - "westus": "WUS", - "westus2": "WUS2", - "brazilsouth": "CQ", - "brazilsoutheast": "BRSE", - "norwayeast": "NOE", - "southafricanorth": "JNB", - "northcentralus": "NCUS", - "uaenorth": "DXB", - "germanywestcentral": "DEWC", - "ukwest": "WUK", - "switzerlandnorth": "CHN", - "switzerlandwest": "CHW", - "uaecentral": "AUH" - } - AzureCloudRegionToOmsRegionMap = { - "australiacentral": "australiacentral", - "australiacentral2": "australiacentral", - "australiaeast": "australiaeast", - "australiasoutheast": "australiasoutheast", - "brazilsouth": "brazilsouth", - "canadacentral": "canadacentral", - "canadaeast": "canadacentral", - "centralus": "centralus", - "centralindia": "centralindia", - "eastasia": "eastasia", - "eastus": "eastus", - "eastus2": "eastus2", - "francecentral": "francecentral", - "francesouth": "francecentral", - "japaneast": "japaneast", - "japanwest": "japaneast", - "koreacentral": "koreacentral", - "koreasouth": "koreacentral", - "northcentralus": "northcentralus", - "northeurope": "northeurope", - "southafricanorth": "southafricanorth", - "southafricawest": "southafricanorth", - "southcentralus": "southcentralus", - "southeastasia": "southeastasia", - "southindia": "centralindia", - "uksouth": "uksouth", - "ukwest": "ukwest", - "westcentralus": "eastus", - "westeurope": "westeurope", - "westindia": "centralindia", - "westus": "westus", - "westus2": "westus2", - "norwayeast": "norwayeast", - "norwaywest": "norwayeast", - "switzerlandnorth": "switzerlandnorth", - "switzerlandwest": "switzerlandwest", - "uaenorth": "uaenorth", - "germanywestcentral": "germanywestcentral", - "germanynorth": "germanywestcentral", - "uaecentral": "uaecentral", - "eastus2euap": "eastus2euap", - "brazilsoutheast": "brazilsoutheast" - } - - # mapping for azure china cloud - # currently log analytics supported only China East 2 region - AzureChinaLocationToOmsRegionCodeMap = { - "chinaeast": "EAST2", - "chinaeast2": "EAST2", - "chinanorth": "EAST2", - "chinanorth2": "EAST2" - } - AzureChinaRegionToOmsRegionMap = { - "chinaeast": "chinaeast2", - "chinaeast2": "chinaeast2", - "chinanorth": "chinaeast2", - "chinanorth2": "chinaeast2" - } - - # mapping for azure us governmner cloud - AzureFairfaxLocationToOmsRegionCodeMap = { - "usgovvirginia": "USGV", - "usgovarizona": "PHX" - } - AzureFairfaxRegionToOmsRegionMap = { - "usgovvirginia": "usgovvirginia", - "usgovtexas": "usgovvirginia", - "usgovarizona": "usgovarizona" - } - - rg_location = _get_rg_location(cmd.cli_ctx, resource_group_name) - cloud_name = cmd.cli_ctx.cloud.name - - workspace_region = "eastus" - workspace_region_code = "EUS" - - # sanity check that locations and clouds match. - if ((cloud_name.lower() == 'azurecloud' and AzureChinaRegionToOmsRegionMap.get(rg_location, False)) or - (cloud_name.lower() == 'azurecloud' and AzureFairfaxRegionToOmsRegionMap.get(rg_location, False))): - raise CLIError('Wrong cloud (azurecloud) setting for region {}, please use "az cloud set ..."' - .format(rg_location)) - - if ((cloud_name.lower() == 'azurechinacloud' and AzureCloudRegionToOmsRegionMap.get(rg_location, False)) or - (cloud_name.lower() == 'azurechinacloud' and AzureFairfaxRegionToOmsRegionMap.get(rg_location, False))): - raise CLIError('Wrong cloud (azurechinacloud) setting for region {}, please use "az cloud set ..."' - .format(rg_location)) - - if ((cloud_name.lower() == 'azureusgovernment' and AzureCloudRegionToOmsRegionMap.get(rg_location, False)) or - (cloud_name.lower() == 'azureusgovernment' and AzureChinaRegionToOmsRegionMap.get(rg_location, False))): - raise CLIError('Wrong cloud (azureusgovernment) setting for region {}, please use "az cloud set ..."' - .format(rg_location)) - - if cloud_name.lower() == 'azurecloud': - workspace_region = AzureCloudRegionToOmsRegionMap.get( - rg_location, "eastus") - workspace_region_code = AzureCloudLocationToOmsRegionCodeMap.get( - workspace_region, "EUS") - elif cloud_name.lower() == 'azurechinacloud': - workspace_region = AzureChinaRegionToOmsRegionMap.get( - rg_location, "chinaeast2") - workspace_region_code = AzureChinaLocationToOmsRegionCodeMap.get( - workspace_region, "EAST2") - elif cloud_name.lower() == 'azureusgovernment': - workspace_region = AzureFairfaxRegionToOmsRegionMap.get( - rg_location, "usgovvirginia") - workspace_region_code = AzureFairfaxLocationToOmsRegionCodeMap.get( - workspace_region, "USGV") - else: - workspace_region = rg_location - workspace_region_code = rg_location.upper() - - default_workspace_resource_group = 'DefaultResourceGroup-' + workspace_region_code - default_workspace_name = 'DefaultWorkspace-{0}-{1}'.format( - subscription_id, workspace_region_code) - default_workspace_resource_id = '/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.OperationalInsights' \ - '/workspaces/{2}'.format(subscription_id, - default_workspace_resource_group, default_workspace_name) - resource_groups = cf_resource_groups(cmd.cli_ctx, subscription_id) - resources = cf_resources(cmd.cli_ctx, subscription_id) - - # check if default RG exists - if resource_groups.check_existence(default_workspace_resource_group): - try: - resource = resources.get_by_id( - default_workspace_resource_id, '2015-11-01-preview') - return resource.id - except CloudError as ex: - if ex.status_code != 404: - raise ex - else: - # TODO: track2/replace create_or_update with begin_create_or_update, depends on 'azure.mgmt.resource.resources' - resource_groups.create_or_update(default_workspace_resource_group, { - 'location': workspace_region}) - GenericResource = cmd.get_models( - 'GenericResource', resource_type=ResourceType.MGMT_RESOURCE_RESOURCES) - generic_resource = GenericResource(location=workspace_region, properties={ - 'sku': {'name': 'standalone'}}) - - async_poller = resources.begin_create_or_update_by_id(default_workspace_resource_id, '2015-11-01-preview', - generic_resource) - - ws_resource_id = '' - while True: - result = async_poller.result(15) - if async_poller.done(): - ws_resource_id = result.id - break - - return ws_resource_id - - def _ensure_container_insights_for_monitoring(cmd, addon): # Workaround for this addon key which has been seen lowercased in the wild. for key in list(addon.config): @@ -4372,7 +4066,7 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= logger.warning('Support for the creation of ARO 3.11 clusters ends 30 Nov 2020. Please see aka.ms/aro/4 for information on switching to ARO 4.') # pylint: disable=line-too-long if location is None: - location = _get_rg_location(cmd.cli_ctx, resource_group_name) + location = get_rg_location(cmd.cli_ctx, resource_group_name) agent_pool_profiles = [] agent_node_pool_profile = OpenShiftManagedClusterAgentPoolProfile( name='compute', # Must be 12 chars or less before ACS RP adds to it @@ -4585,11 +4279,11 @@ def _put_managed_cluster_ensuring_permission( namespace='Microsoft.ContainerService', type='managedClusters', name=name ) - _add_monitoring_role_assignment(cluster, cluster_resource_id, cmd) + add_monitoring_role_assignment(cluster, cluster_resource_id, cmd) if ingress_appgw_addon_enabled: - _add_ingress_appgw_addon_role_assignment(cluster, cmd) + add_ingress_appgw_addon_role_assignment(cluster, cmd) if virtual_node_addon_enabled: - _add_virtual_node_role_assignment(cmd, cluster, vnet_subnet_id) + add_virtual_node_role_assignment(cmd, cluster, vnet_subnet_id) if need_grant_vnet_permission_to_cluster_identity: if not _create_role_assignment(cmd, 'Network Contributor', cluster.identity.principal_id, scope=vnet_subnet_id, diff --git a/src/azure-cli/azure/cli/command_modules/acs/decorator.py b/src/azure-cli/azure/cli/command_modules/acs/decorator.py index da6c977e0c5..8bfd38d3a12 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/decorator.py @@ -23,17 +23,21 @@ from azure.cli.command_modules.acs._loadbalancer import ( update_load_balancer_profile as _update_load_balancer_profile, ) +from azure.cli.command_modules.acs._resourcegroup import ( + get_rg_location, +) from azure.cli.command_modules.acs._validators import ( extract_comma_separated_string, ) +from azure.cli.command_modules.acs.addonconfiguration import ( + ensure_default_log_analytics_workspace_for_monitoring, + ensure_container_insights_for_monitoring, +) from azure.cli.command_modules.acs.custom import ( _add_role_assignment, _ensure_aks_acr, _ensure_aks_service_principal, _ensure_cluster_identity_permission_on_kubelet_identity, - _ensure_container_insights_for_monitoring, - _ensure_default_log_analytics_workspace_for_monitoring, - _get_rg_location, _get_user_assigned_identity, _put_managed_cluster_ensuring_permission, subnet_role_assignment_exists, @@ -558,7 +562,7 @@ def get_name(self) -> str: def _get_location(self, read_only: bool = False, **kwargs) -> Union[str, None]: """Internal function to dynamically obtain the value of location according to the context. - When location is not assigned, dynamic completion will be triggerd. Function "_get_rg_location" will be called + When location is not assigned, dynamic completion will be triggerd. Function "get_rg_location" will be called to get the location of the provided resource group, which internally used ResourceManagementClient to send the request. @@ -580,7 +584,7 @@ def _get_location(self, read_only: bool = False, **kwargs) -> Union[str, None]: # dynamic completion if not read_from_mc and location is None: - location = _get_rg_location( + location = get_rg_location( self.cmd.cli_ctx, self.get_resource_group_name() ) @@ -590,7 +594,7 @@ def _get_location(self, read_only: bool = False, **kwargs) -> Union[str, None]: def get_location(self) -> Union[str, None]: """Dynamically obtain the value of location according to the context. - When location is not assigned, dynamic completion will be triggerd. Function "_get_rg_location" will be called + When location is not assigned, dynamic completion will be triggerd. Function "get_rg_location" will be called to get the location of the provided resource group, which internally used ResourceManagementClient to send the request. @@ -2691,7 +2695,7 @@ def _get_workspace_resource_id( """Internal function to dynamically obtain the value of workspace_resource_id according to the context. When workspace_resource_id is not assigned, dynamic completion will be triggerd. Function - "_ensure_default_log_analytics_workspace_for_monitoring" will be called to create a workspace with + "ensure_default_log_analytics_workspace_for_monitoring" will be called to create a workspace with subscription_id and resource_group_name, which internally used ResourceManagementClient to send the request. This function supports the option of enable_validation. When enabled, it will check if workspace_resource_id is @@ -2733,7 +2737,7 @@ def _get_workspace_resource_id( if workspace_resource_id is None: # use default workspace if exists else create default workspace workspace_resource_id = ( - _ensure_default_log_analytics_workspace_for_monitoring( + ensure_default_log_analytics_workspace_for_monitoring( self.cmd, self.get_subscription_id(), self.get_resource_group_name(), @@ -2756,7 +2760,7 @@ def get_workspace_resource_id(self) -> Union[str, None]: """Dynamically obtain the value of workspace_resource_id according to the context. When workspace_resource_id is not assigned, dynamic completion will be triggerd. Function - "_ensure_default_log_analytics_workspace_for_monitoring" will be called to create a workspace with + "ensure_default_log_analytics_workspace_for_monitoring" will be called to create a workspace with subscription_id and resource_group_name, which internally used ResourceManagementClient to send the request. :return: string or None @@ -4410,7 +4414,7 @@ def __init__( def init_mc(self) -> ManagedCluster: """Initialize a ManagedCluster object with several parameters and attach it to internal context. - When location is not assigned, function "_get_rg_location" will be called to get the location of the provided + When location is not assigned, function "get_rg_location" will be called to get the location of the provided resource group, which internally used ResourceManagementClient to send the request. :return: the ManagedCluster object @@ -4781,9 +4785,9 @@ def build_kube_dashboard_addon_profile(self) -> ManagedClusterAddonProfile: def build_monitoring_addon_profile(self) -> ManagedClusterAddonProfile: """Build monitoring addon profile. - The function "_ensure_container_insights_for_monitoring" will be called to create a deployment which publishes + The function "ensure_container_insights_for_monitoring" will be called to create a deployment which publishes the Container Insights solution to the Log Analytics workspace. - When workspace_resource_id is not assigned, function "_ensure_default_log_analytics_workspace_for_monitoring" + When workspace_resource_id is not assigned, function "ensure_default_log_analytics_workspace_for_monitoring" will be called to create a workspace, which internally used ResourceManagementClient to send the request. :return: a ManagedClusterAddonProfile object @@ -4802,8 +4806,13 @@ def build_monitoring_addon_profile(self) -> ManagedClusterAddonProfile: }, ) # post-process, create a deployment - _ensure_container_insights_for_monitoring( - self.cmd, monitoring_addon_profile + ensure_container_insights_for_monitoring( + self.cmd, monitoring_addon_profile, + self.context.get_subscription_id(), + self.context.get_resource_group_name(), + self.context.get_name(), + self.context.get_location(), + aad_route=False, ) # set intermediate self.context.set_intermediate("monitoring", True, overwrite_exists=True) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_custom.py index ab38a5b5897..73e114e9a68 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_custom.py @@ -26,8 +26,8 @@ from azure.cli.command_modules.acs.custom import (merge_kubernetes_configurations, list_acs_locations, _acs_browse_internal, _add_role_assignment, _get_default_dns_prefix, create_application, _update_addons, - _ensure_container_insights_for_monitoring, k8s_install_kubectl, k8s_install_kubelogin) +from azure.cli.command_modules.acs.addonconfiguration import ensure_container_insights_for_monitoring from azure.mgmt.containerservice.models import (ContainerServiceOrchestratorTypes, ContainerService, ContainerServiceOrchestratorProfile) @@ -618,7 +618,7 @@ def _test_deserializer(resp_type, response): self.assertTrue( 'https://docs.microsoft.com/azure/azure-resource-manager/resource-group-create-service-principal-portal' in str(context.exception)) - @mock.patch('azure.cli.command_modules.acs.custom._get_rg_location', return_value='eastus') + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.custom.cf_resource_groups', autospec=True) @mock.patch('azure.cli.command_modules.acs.custom.cf_resources', autospec=True) def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): @@ -639,12 +639,19 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): self.assertFalse(addon_profile.enabled) # monitoring added - instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', - 'clitest000001', 'clitest000001', 'monitoring', enable=True) - monitoring_addon_profile = instance.addon_profiles[CONST_MONITORING_ADDON_NAME] - self.assertTrue(monitoring_addon_profile.enabled) - routing_addon_profile = instance.addon_profiles[CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME] - self.assertFalse(routing_addon_profile.enabled) + with mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups", + autospec=True, + ), mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resources", + autospec=True, + ): + instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', + 'clitest000001', 'clitest000001', 'monitoring', enable=True) + monitoring_addon_profile = instance.addon_profiles[CONST_MONITORING_ADDON_NAME] + self.assertTrue(monitoring_addon_profile.enabled) + routing_addon_profile = instance.addon_profiles[CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME] + self.assertFalse(routing_addon_profile.enabled) # monitoring disabled, routing enabled instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', @@ -714,11 +721,18 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): # monitoring enabled and then enabled again should error instance = mock.Mock() instance.addon_profiles = None - instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', - 'clitest000001', 'clitest000001', 'monitoring', enable=True) - with self.assertRaises(CLIError): + with mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups", + autospec=True, + ), mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resources", + autospec=True, + ): instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', 'clitest000001', 'clitest000001', 'monitoring', enable=True) + with self.assertRaises(CLIError): + instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', + 'clitest000001', 'clitest000001', 'monitoring', enable=True) # virtual-node enabled instance = mock.MagicMock() @@ -752,16 +766,20 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): addon_profile = instance.addon_profiles['ingressApplicationGateway'] self.assertFalse(addon_profile.enabled) - @mock.patch('azure.cli.command_modules.acs.custom.cf_resources', autospec=True) - @mock.patch('azure.cli.command_modules.acs.custom._invoke_deployment') + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resources', autospec=True) + @mock.patch('azure.cli.command_modules.acs.addonconfiguration._invoke_deployment') def test_ensure_container_insights_for_monitoring(self, invoke_def, cf_resources): cmd = mock.Mock() addon = mock.Mock() wsID = "/subscriptions/1234abcd-cad5-417b-1234-aec62ffa6fe7/resourcegroups/mbdev/providers/microsoft.operationalinsights/workspaces/mbdev" + subscription_id = "test_subscription_id" + rg_name = "test_rg_name" + cluster_name = "test_cluster_name" + location = "test_location" addon.config = { CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: wsID } - self.assertTrue(_ensure_container_insights_for_monitoring(cmd, addon)) + self.assertTrue(ensure_container_insights_for_monitoring(cmd, addon, subscription_id, rg_name, cluster_name, location)) args, kwargs = invoke_def.call_args self.assertEqual(args[3]['resources'][0]['type'], "Microsoft.Resources/deployments") self.assertEqual(args[4]['workspaceResourceId']['value'], wsID) @@ -770,7 +788,7 @@ def test_ensure_container_insights_for_monitoring(self, invoke_def, cf_resources addon.config = { CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: wsID } - self.assertTrue(_ensure_container_insights_for_monitoring(cmd, addon)) + self.assertTrue(ensure_container_insights_for_monitoring(cmd, addon, subscription_id, rg_name, cluster_name, location)) args, kwargs = invoke_def.call_args self.assertEqual(args[3]['resources'][0]['type'], "Microsoft.Resources/deployments") self.assertEqual(args[4]['workspaceResourceId']['value'], wsID) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 9da14d70ea2..1dafb2d9a2f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -26,8 +26,8 @@ from azure.cli.command_modules.acs.custom import (merge_kubernetes_configurations, list_acs_locations, _acs_browse_internal, _add_role_assignment, _get_default_dns_prefix, create_application, _update_addons, - _ensure_container_insights_for_monitoring, k8s_install_kubectl, k8s_install_kubelogin) +from azure.cli.command_modules.acs.addonconfiguration import ensure_container_insights_for_monitoring from azure.mgmt.containerservice.models import (ContainerServiceOrchestratorTypes, ContainerService, ContainerServiceOrchestratorProfile) @@ -618,7 +618,7 @@ def _test_deserializer(resp_type, response): self.assertTrue( 'https://docs.microsoft.com/azure/azure-resource-manager/resource-group-create-service-principal-portal' in str(context.exception)) - @mock.patch('azure.cli.command_modules.acs.custom._get_rg_location', return_value='eastus') + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.custom.cf_resource_groups', autospec=True) @mock.patch('azure.cli.command_modules.acs.custom.cf_resources', autospec=True) def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): @@ -639,12 +639,19 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): self.assertFalse(addon_profile.enabled) # monitoring added - instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', - 'clitest000001', 'clitest000001', 'monitoring', enable=True) - monitoring_addon_profile = instance.addon_profiles[CONST_MONITORING_ADDON_NAME] - self.assertTrue(monitoring_addon_profile.enabled) - routing_addon_profile = instance.addon_profiles[CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME] - self.assertFalse(routing_addon_profile.enabled) + with mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups", + autospec=True, + ), mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resources", + autospec=True, + ): + instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', + 'clitest000001', 'clitest000001', 'monitoring', enable=True) + monitoring_addon_profile = instance.addon_profiles[CONST_MONITORING_ADDON_NAME] + self.assertTrue(monitoring_addon_profile.enabled) + routing_addon_profile = instance.addon_profiles[CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME] + self.assertFalse(routing_addon_profile.enabled) # monitoring disabled, routing enabled instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', @@ -714,11 +721,18 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): # monitoring enabled and then enabled again should error instance = mock.Mock() instance.addon_profiles = None - instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', - 'clitest000001', 'clitest000001', 'monitoring', enable=True) - with self.assertRaises(CLIError): + with mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups", + autospec=True, + ), mock.patch( + "azure.cli.command_modules.acs.addonconfiguration.cf_resources", + autospec=True, + ): instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', 'clitest000001', 'clitest000001', 'monitoring', enable=True) + with self.assertRaises(CLIError): + instance = _update_addons(MockCmd(self.cli), instance, '00000000-0000-0000-0000-000000000000', + 'clitest000001', 'clitest000001', 'monitoring', enable=True) # virtual-node enabled instance = mock.MagicMock() @@ -752,16 +766,20 @@ def test_update_addons(self, rg_def, cf_resource_groups, cf_resources): addon_profile = instance.addon_profiles['ingressApplicationGateway'] self.assertFalse(addon_profile.enabled) - @mock.patch('azure.cli.command_modules.acs.custom.cf_resources', autospec=True) - @mock.patch('azure.cli.command_modules.acs.custom._invoke_deployment') + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resources', autospec=True) + @mock.patch('azure.cli.command_modules.acs.addonconfiguration._invoke_deployment') def test_ensure_container_insights_for_monitoring(self, invoke_def, cf_resources): cmd = mock.Mock() addon = mock.Mock() wsID = "/subscriptions/1234abcd-cad5-417b-1234-aec62ffa6fe7/resourcegroups/mbdev/providers/microsoft.operationalinsights/workspaces/mbdev" + subscription_id = "test_subscription_id" + rg_name = "test_rg_name" + cluster_name = "test_cluster_name" + location = "test_location" addon.config = { CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: wsID } - self.assertTrue(_ensure_container_insights_for_monitoring(cmd, addon)) + self.assertTrue(ensure_container_insights_for_monitoring(cmd, addon, subscription_id, rg_name, cluster_name, location)) args, kwargs = invoke_def.call_args self.assertEqual(args[3]['resources'][0]['type'], "Microsoft.Resources/deployments") self.assertEqual(args[4]['workspaceResourceId']['value'], wsID) @@ -770,7 +788,7 @@ def test_ensure_container_insights_for_monitoring(self, invoke_def, cf_resources addon.config = { CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: wsID } - self.assertTrue(_ensure_container_insights_for_monitoring(cmd, addon)) + self.assertTrue(ensure_container_insights_for_monitoring(cmd, addon, subscription_id, rg_name, cluster_name, location)) args, kwargs = invoke_def.call_args self.assertEqual(args[3]['resources'][0]['type'], "Microsoft.Resources/deployments") self.assertEqual(args[4]['workspaceResourceId']['value'], wsID) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_decorator.py index 64b0b66d2c1..d4681dc5bf4 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_decorator.py @@ -437,7 +437,7 @@ def test_get_location(self): decorator_mode=DecoratorMode.CREATE, ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ): self.assertEqual(ctx_1._get_location(read_only=True), None) @@ -1342,7 +1342,7 @@ def test_get_service_principal_and_client_secret( "subscription_id", "1234-5678", overwrite_exists=True ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ), patch( "azure.cli.command_modules.acs.custom.get_graph_rbac_management_client", @@ -1370,7 +1370,7 @@ def test_get_service_principal_and_client_secret( "subscription_id", "1234-5678", overwrite_exists=True ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ), patch( "azure.cli.command_modules.acs.custom.get_graph_rbac_management_client", @@ -1417,7 +1417,7 @@ def test_get_service_principal_and_client_secret( "subscription_id", "1234-5678", overwrite_exists=True ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ), patch( "azure.cli.command_modules.acs.custom.get_graph_rbac_management_client", @@ -2489,13 +2489,13 @@ def test_get_workspace_resource_id(self): begin_create_or_update_by_id=Mock(return_value=async_poller) ) with patch( - "azure.cli.command_modules.acs.custom._get_rg_location", + "azure.cli.command_modules.acs.addonconfiguration.get_rg_location", return_value="test_location", ), patch( - "azure.cli.command_modules.acs.custom.cf_resource_groups", + "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups", return_value=cf_resource_groups, ), patch( - "azure.cli.command_modules.acs.custom.cf_resources", + "azure.cli.command_modules.acs.addonconfiguration.cf_resources", return_value=cf_resources, ): self.assertEqual( @@ -4602,7 +4602,7 @@ def test_set_up_service_principal_profile(self): "subscription_id", "1234-5678", overwrite_exists=True ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ), patch( "azure.cli.command_modules.acs.custom.get_graph_rbac_management_client", @@ -5042,14 +5042,21 @@ def test_build_monitoring_addon_profile(self): self.cmd, self.client, { + "location": "test_location", "enable_addons": "monitoring", "workspace_resource_id": "test_workspace_resource_id", }, ResourceType.MGMT_CONTAINERSERVICE, ) + mock_profile = Mock( + get_subscription_id=Mock(return_value="1234-5678-9012") + ) with patch( - "azure.cli.command_modules.acs.decorator._ensure_container_insights_for_monitoring", + "azure.cli.command_modules.acs.decorator.Profile", + return_value=mock_profile, + ), patch( + "azure.cli.command_modules.acs.decorator.ensure_container_insights_for_monitoring", return_value=None, ): self.assertEqual(dec_1.context.get_intermediate("monitoring"), None) @@ -5343,6 +5350,7 @@ def test_set_up_addon_profiles(self): self.cmd, self.client, { + "location": "test_location", "vnet_subnet_id": "test_vnet_subnet_id", "enable_addons": "http_application_routing,monitoring,virtual-node,kube-dashboard,azure-policy,ingress-appgw,confcom,open-service-mesh,azure-keyvault-secrets-provider", "workspace_resource_id": "test_workspace_resource_id", @@ -5359,8 +5367,14 @@ def test_set_up_addon_profiles(self): ResourceType.MGMT_CONTAINERSERVICE, ) mc_2 = self.models.ManagedCluster(location="test_location") + mock_profile = Mock( + get_subscription_id=Mock(return_value="1234-5678-9012") + ) with patch( - "azure.cli.command_modules.acs.decorator._ensure_container_insights_for_monitoring", + "azure.cli.command_modules.acs.decorator.Profile", + return_value=mock_profile, + ), patch( + "azure.cli.command_modules.acs.decorator.ensure_container_insights_for_monitoring", return_value=None, ): dec_mc_2 = dec_2.set_up_addon_profiles(mc_2) @@ -6059,7 +6073,7 @@ def test_construct_default_mc_profile(self): get_subscription_id=Mock(return_value="1234-5678-9012") ) with patch( - "azure.cli.command_modules.acs.decorator._get_rg_location", + "azure.cli.command_modules.acs.decorator.get_rg_location", return_value="test_location", ), patch( "azure.cli.command_modules.acs.decorator.Profile",