diff --git a/src/azure-cli/azure/cli/command_modules/acs/_help.py b/src/azure-cli/azure/cli/command_modules/acs/_help.py index 7a89268ed9f..e19e5c64936 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_help.py @@ -345,6 +345,30 @@ - name: --apiserver-subnet-id type: string short-summary: The ID of a subnet in an existing VNet into which to assign control plane apiserver pods(requires --enable-apiserver-vnet-integration) + - name: --system-node-subnet-id + type: string + short-summary: (Automatic SKU) The ID of a subnet in an existing VNet to be used by the Managed System Pool in an Automatic cluster. + long-summary: | + Bring-your-own VNet for an Automatic cluster requires three subnets supplied together: + `--system-node-subnet-id` (this flag, for the Managed System Pool), `--node-subnet-id` + (for user node pools), and `--apiserver-subnet-id` (for the control plane API server). + All three subnets must belong to the same VNet and can only be used with `--sku automatic`. + - name: --node-subnet-id + type: string + short-summary: (Automatic SKU) The ID of a subnet in an existing VNet to be used by user node pools in an Automatic cluster. + long-summary: | + Bring-your-own VNet for an Automatic cluster requires three subnets supplied together: + `--system-node-subnet-id` (for the Managed System Pool), `--node-subnet-id` (this flag, + for user node pools), and `--apiserver-subnet-id` (for the control plane API server). + All three subnets must belong to the same VNet and can only be used with `--sku automatic`. + - name: --enable-hosted-system + type: bool + short-summary: (Automatic SKU) Explicitly opt in to a Managed System Pool for the Automatic cluster. + long-summary: | + Only valid with `--sku automatic`. Use this flag when you want to deterministically + request a Managed System Pool regardless of region defaults. It is also implied when + you supply the bring-your-own VNet subnet trio (`--system-node-subnet-id`, + `--node-subnet-id`, `--apiserver-subnet-id`). - name: --enable-private-cluster type: string short-summary: Enable private cluster. diff --git a/src/azure-cli/azure/cli/command_modules/acs/_params.py b/src/azure-cli/azure/cli/command_modules/acs/_params.py index 49e2d17f585..17ec6e91a24 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_params.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_params.py @@ -133,6 +133,7 @@ validate_disable_windows_outbound_nat, validate_asm_egress_name, validate_crg_id, validate_apiserver_subnet_id, + validate_system_node_subnet_id, validate_node_subnet_id, validate_azure_service_mesh_revision, validate_message_of_the_day, validate_custom_ca_trust_certificates, @@ -443,6 +444,9 @@ def load_arguments(self, _): c.argument('enable_private_cluster', action='store_true') c.argument('enable_apiserver_vnet_integration', action='store_true') c.argument('apiserver_subnet_id', validator=validate_apiserver_subnet_id) + c.argument('system_node_subnet_id', validator=validate_system_node_subnet_id) + c.argument('node_subnet_id', validator=validate_node_subnet_id) + c.argument('enable_hosted_system', action='store_true') c.argument('private_dns_zone') c.argument('disable_public_fqdn', action='store_true') c.argument('service_principal') diff --git a/src/azure-cli/azure/cli/command_modules/acs/_validators.py b/src/azure-cli/azure/cli/command_modules/acs/_validators.py index 45ab88989d9..1a45d9ff49d 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_validators.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_validators.py @@ -455,6 +455,14 @@ def validate_apiserver_subnet_id(namespace): _validate_subnet_id(namespace.apiserver_subnet_id, "--apiserver-subnet-id") +def validate_system_node_subnet_id(namespace): + _validate_subnet_id(namespace.system_node_subnet_id, "--system-node-subnet-id") + + +def validate_node_subnet_id(namespace): + _validate_subnet_id(namespace.node_subnet_id, "--node-subnet-id") + + def _validate_subnet_id(subnet_id, name): if subnet_id is None or subnet_id == '': return 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 b0d7ece1360..2051e589ea4 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -1032,6 +1032,10 @@ def aks_create( # apiserver vnet integration enable_apiserver_vnet_integration=False, apiserver_subnet_id=None, + # BYO VNet for Managed System Pool (Automatic SKU) + system_node_subnet_id=None, + node_subnet_id=None, + enable_hosted_system=False, # node provisioning node_provisioning_mode=None, node_provisioning_default_pools=None, @@ -1273,7 +1277,7 @@ def aks_upgrade(cmd, instance = client.get(resource_group_name, name) vmas_cluster = False - for agent_profile in instance.agent_pool_profiles: + for agent_profile in (instance.agent_pool_profiles or []): if agent_profile.type.lower() == "availabilityset": vmas_cluster = True break @@ -1290,7 +1294,7 @@ def aks_upgrade(cmd, # This only provide convenience for customer at client side so they can run az aks upgrade to upgrade all # nodepools of a cluster. The SDK only support upgrade single nodepool at a time. - for agent_pool_profile in instance.agent_pool_profiles: + for agent_pool_profile in (instance.agent_pool_profiles or []): if vmas_cluster: raise CLIError('This cluster is using AvailabilitySet. Node image upgrade only operation ' 'can only be applied on VirtualMachineScaleSets or VirtualMachines cluster.') @@ -1354,7 +1358,7 @@ def aks_upgrade(cmd, return None if upgrade_all: - for agent_profile in instance.agent_pool_profiles: + for agent_profile in (instance.agent_pool_profiles or []): agent_profile.orchestrator_version = kubernetes_version agent_profile.creation_data = None @@ -1438,12 +1442,17 @@ def _upgrade_single_nodepool_image_version(no_wait, client, resource_group_name, def aks_scale(cmd, client, resource_group_name, name, node_count, nodepool_name="", no_wait=False): instance = client.get(resource_group_name, name) - if len(instance.agent_pool_profiles) > 1 and nodepool_name == "": + agent_pool_profiles = instance.agent_pool_profiles or [] + if not agent_pool_profiles: + raise CLIError('The cluster has no scalable node pools (this may be a Managed System Pool for ' + 'Automatic cluster). Use az aks nodepool add/scale against a user node pool instead.') + + if len(agent_pool_profiles) > 1 and nodepool_name == "": raise CLIError('There are more than one node pool in the cluster. ' 'Please specify nodepool name or use az aks nodepool command to scale node pool') - for agent_profile in instance.agent_pool_profiles: - if agent_profile.name == nodepool_name or (nodepool_name == "" and len(instance.agent_pool_profiles) == 1): + for agent_profile in agent_pool_profiles: + if agent_profile.name == nodepool_name or (nodepool_name == "" and len(agent_pool_profiles) == 1): if agent_profile.enable_auto_scaling: raise CLIError( "Cannot scale cluster autoscaler enabled node pool.") diff --git a/src/azure-cli/azure/cli/command_modules/acs/linter_exclusions.yml b/src/azure-cli/azure/cli/command_modules/acs/linter_exclusions.yml index 39692b2f608..2e991452849 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/linter_exclusions.yml +++ b/src/azure-cli/azure/cli/command_modules/acs/linter_exclusions.yml @@ -1,6 +1,15 @@ --- aks create: parameters: + system_node_subnet_id: + rule_exclusions: + - missing_parameter_test_coverage + node_subnet_id: + rule_exclusions: + - missing_parameter_test_coverage + enable_hosted_system: + rule_exclusions: + - missing_parameter_test_coverage appgw_watch_namespace: rule_exclusions: - option_length_too_long diff --git a/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py index b24a7262f56..cb16ef969df 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py @@ -2362,6 +2362,30 @@ def get_sku_name(self) -> str: skuName = CONST_MANAGED_CLUSTER_SKU_NAME_BASE return skuName + @staticmethod + def _raise_missing_vnet_subnet_for_outbound_type(outbound_type: str, sku_name: str) -> None: + if outbound_type == CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING: + subnet_requirement = "a route table with egress rules" + else: + subnet_requirement = "a NAT gateway with outbound ips" + + if sku_name == CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC: + raise RequiredArgumentMissingError( + "--vnet-subnet-id must be specified for {outbound_type}. For an Automatic cluster " + "using Managed System Pool BYO VNet, specify --system-node-subnet-id, --node-subnet-id " + "and --apiserver-subnet-id instead. The subnet must be pre-configured with {requirement}".format( + outbound_type=outbound_type, + requirement=subnet_requirement, + ) + ) + raise RequiredArgumentMissingError( + "--vnet-subnet-id must be specified for {outbound_type} and it must " + "be pre-configured with {requirement}".format( + outbound_type=outbound_type, + requirement=subnet_requirement, + ) + ) + def _get_outbound_type( self, enable_validation: bool = False, @@ -2407,7 +2431,20 @@ def _get_outbound_type( skuName = self.get_sku_name() isVnetSubnetIdEmpty = self.get_vnet_subnet_id() in ["", None] - if skuName is not None and skuName == CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC and isVnetSubnetIdEmpty: + # For BYO VNet Managed System Pool (Automatic SKU with system-node/node subnet trio), + # the user's subnet IDs replace --vnet-subnet-id; don't force ManagedNATGateway in that case. + byo_subnets_set = bool( + self.raw_param.get("system_node_subnet_id") or + self.raw_param.get("node_subnet_id") + ) + use_automatic_managed_nat_gateway = ( + skuName is not None and + skuName == CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC and + isVnetSubnetIdEmpty and + not read_from_mc and + not byo_subnets_set + ) + if use_automatic_managed_nat_gateway and outbound_type == CONST_OUTBOUND_TYPE_LOAD_BALANCER: # outbound_type of Automatic SKU should be ManagedNATGateway if no subnet id provided. outbound_type = CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY @@ -2434,22 +2471,17 @@ def _get_outbound_type( ) return outbound_type # basic sku lb doesn't support outbound type - if outbound_type == CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING: - if self.get_vnet_subnet_id() in ["", None]: - raise RequiredArgumentMissingError( - "--vnet-subnet-id must be specified for userDefinedRouting and it must " - "be pre-configured with a route table with egress rules" - ) - if outbound_type == CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY: - if self.get_vnet_subnet_id() in ["", None]: - raise RequiredArgumentMissingError( - "--vnet-subnet-id must be specified for userAssignedNATGateway and it must " - "be pre-configured with a NAT gateway with outbound ips" - ) + if outbound_type in [ + CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING, + CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY, + ]: + if self.get_vnet_subnet_id() in ["", None] and not byo_subnets_set: + self._raise_missing_vnet_subnet_for_outbound_type(outbound_type, skuName) if outbound_type == CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY: - if self.get_vnet_subnet_id() not in ["", None]: + if self.get_vnet_subnet_id() not in ["", None] or byo_subnets_set: raise InvalidArgumentValueError( - "--vnet-subnet-id cannot be specified for managedNATGateway" + "--vnet-subnet-id, --system-node-subnet-id and --node-subnet-id cannot be " + "specified for managedNATGateway" ) if outbound_type != CONST_OUTBOUND_TYPE_LOAD_BALANCER: if ( @@ -4211,7 +4243,15 @@ def _get_apiserver_subnet_id(self, enable_validation: bool = False) -> Union[str if enable_validation: if self.decorator_mode == DecoratorMode.CREATE: vnet_subnet_id = self.get_vnet_subnet_id() - if apiserver_subnet_id and vnet_subnet_id is None: + # For BYO VNet Managed System Pool (--sku automatic with subnet trio), + # --vnet-subnet-id is not used: system-node / node subnets replace it. Only + # require --vnet-subnet-id when neither --system-node-subnet-id nor + # --node-subnet-id is provided. + byo_subnets_set = ( + self.raw_param.get("system_node_subnet_id") or + self.raw_param.get("node_subnet_id") + ) + if apiserver_subnet_id and vnet_subnet_id is None and not byo_subnets_set: raise RequiredArgumentMissingError( '"--apiserver-subnet-id" requires "--vnet-subnet-id".') @@ -4240,6 +4280,103 @@ def get_apiserver_subnet_id(self) -> Union[str, None]: """ return self._get_apiserver_subnet_id(enable_validation=True) + def get_system_node_subnet_id(self) -> Union[str, None]: + """Obtain the value of system_node_subnet_id (BYO VNet for Automatic cluster). + + :return: str or None + """ + system_node_subnet_id = self.raw_param.get("system_node_subnet_id") + if self.decorator_mode == DecoratorMode.CREATE: + if ( + self.mc and + self.mc.hosted_system_profile and + getattr(self.mc.hosted_system_profile, "system_node_subnet_id", None) is not None + ): + system_node_subnet_id = self.mc.hosted_system_profile.system_node_subnet_id + return system_node_subnet_id + + def get_node_subnet_id(self) -> Union[str, None]: + """Obtain the value of node_subnet_id (BYO VNet for Automatic cluster). + + :return: str or None + """ + node_subnet_id = self.raw_param.get("node_subnet_id") + if self.decorator_mode == DecoratorMode.CREATE: + if ( + self.mc and + self.mc.hosted_system_profile and + getattr(self.mc.hosted_system_profile, "node_subnet_id", None) is not None + ): + node_subnet_id = self.mc.hosted_system_profile.node_subnet_id + return node_subnet_id + + def get_enable_hosted_system(self) -> bool: + """Obtain the value of enable_hosted_system. + + Returns True when the user explicitly opts in via --enable-hosted-system, + or implicitly via the BYO VNet subnet trio for Managed System Pool. + + :return: bool + """ + if self.decorator_mode != DecoratorMode.CREATE: + return False + explicit = bool(self.raw_param.get("enable_hosted_system")) + implicit = all( + [ + self.raw_param.get("system_node_subnet_id"), + self.raw_param.get("node_subnet_id"), + self.raw_param.get("apiserver_subnet_id"), + ] + ) + return explicit or implicit + + def validate_byo_hosted_system_subnets(self) -> None: + """Validate the BYO VNet subnet trio and the --enable-hosted-system flag. + + BYO VNet for a Managed System Pool is triggered by --system-node-subnet-id / + --node-subnet-id. --apiserver-subnet-id is intentionally NOT part of the trigger + because it keeps its existing general-purpose meaning for + --enable-apiserver-vnet-integration flows on non-Automatic clusters. + + - If either --system-node-subnet-id or --node-subnet-id is set, the full trio + (--system-node-subnet-id, --node-subnet-id, --apiserver-subnet-id) must be + provided and --sku must be automatic. + - --enable-hosted-system is only valid with --sku automatic. + """ + if self.decorator_mode != DecoratorMode.CREATE: + return + system_node_subnet_id = self.raw_param.get("system_node_subnet_id") + node_subnet_id = self.raw_param.get("node_subnet_id") + apiserver_subnet_id = self.raw_param.get("apiserver_subnet_id") + enable_hosted_system = bool(self.raw_param.get("enable_hosted_system")) + + byo_specific_set = bool(system_node_subnet_id or node_subnet_id) + + # --enable-hosted-system requires --sku automatic. + if enable_hosted_system and self.get_sku_name() != CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC: + raise RequiredArgumentMissingError( + '"--enable-hosted-system" requires "--sku automatic".' + ) + + # Partial trio: if any BYO subnet is set, require the full trio. + if byo_specific_set: + missing = [] + if not system_node_subnet_id: + missing.append("--system-node-subnet-id") + if not node_subnet_id: + missing.append("--node-subnet-id") + if not apiserver_subnet_id: + missing.append("--apiserver-subnet-id") + if missing: + raise RequiredArgumentMissingError( + "BYO VNet for a Managed System Pool (Automatic cluster) requires all three " + "subnets. Missing: " + ", ".join(missing) + "." + ) + if self.get_sku_name() != CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC: + raise RequiredArgumentMissingError( + '"--system-node-subnet-id" / "--node-subnet-id" require "--sku automatic".' + ) + def _get_enable_private_cluster(self, enable_validation: bool = False) -> bool: """Internal function to obtain the value of enable_private_cluster. @@ -6218,6 +6355,9 @@ def set_up_agentpool_profile(self, mc: ManagedCluster) -> ManagedCluster: """ self._ensure_mc(mc) + if self.context.get_enable_hosted_system(): + return mc + agentpool_profile = self.agentpool_decorator.construct_agentpool_profile_default() mc.agent_pool_profiles = [agentpool_profile] return mc @@ -6341,6 +6481,19 @@ def set_up_service_principal_profile(self, mc: ManagedCluster) -> ManagedCluster mc.service_principal_profile = service_principal_profile return mc + def _get_byo_hosted_system_subnet_ids(self) -> List[str]: + if not self.context.get_enable_hosted_system(): + return [] + + subnet_ids = [] + seen = set() + for raw_key in ("system_node_subnet_id", "node_subnet_id", "apiserver_subnet_id"): + subnet_id = self.context.raw_param.get(raw_key) + if subnet_id and subnet_id not in seen: + subnet_ids.append(subnet_id) + seen.add(subnet_id) + return subnet_ids + def process_add_role_assignment_for_vnet_subnet(self, mc: ManagedCluster) -> None: """Add role assignment for vent subnet. @@ -6357,6 +6510,10 @@ def process_add_role_assignment_for_vnet_subnet(self, mc: ManagedCluster) -> Non """ self._ensure_mc(mc) + # Validate before granting roles so a malformed BYO trio does not leave + # partial Network Contributor assignments behind. + self.context.validate_byo_hosted_system_subnets() + need_post_creation_vnet_permission_granting = False vnet_subnet_id = self.context.get_vnet_subnet_id() skip_subnet_role_assignment = ( @@ -6402,6 +6559,50 @@ def process_add_role_assignment_for_vnet_subnet(self, mc: ManagedCluster) -> Non logger.warning( "Could not create a role assignment for subnet. Are you an Owner on this subscription?" ) + byo_hosted_system_subnet_ids = self._get_byo_hosted_system_subnet_ids() + if byo_hosted_system_subnet_ids and not skip_subnet_role_assignment: + service_principal_profile = mc.service_principal_profile + assign_identity = self.context.get_assign_identity() + pending_post_creation_subnets = [] + if service_principal_profile is None and not assign_identity: + for subnet_id in byo_hosted_system_subnet_ids: + if not self.context.external_functions.subnet_role_assignment_exists(self.cmd, subnet_id): + pending_post_creation_subnets.append(subnet_id) + if pending_post_creation_subnets: + need_post_creation_vnet_permission_granting = True + self.context.set_intermediate( + "byo_hosted_system_subnets_pending_grant", + pending_post_creation_subnets, + overwrite_exists=True, + ) + else: + identity_object_id = None + if assign_identity: + identity_object_id = self.context.get_user_assigned_identity_object_id() + for subnet_id in byo_hosted_system_subnet_ids: + if self.context.external_functions.subnet_role_assignment_exists(self.cmd, subnet_id): + continue + if assign_identity: + added = self.context.external_functions.add_role_assignment( + self.cmd, + "Network Contributor", + identity_object_id, + is_service_principal=False, + scope=subnet_id, + ) + else: + added = self.context.external_functions.add_role_assignment( + self.cmd, + "Network Contributor", + service_principal_profile.client_id, + scope=subnet_id, + ) + if not added: + logger.warning( + "Could not create a role assignment for subnet %s. " + "Are you an Owner on this subscription?", + subnet_id, + ) # store need_post_creation_vnet_permission_granting as an intermediate self.context.set_intermediate( "need_post_creation_vnet_permission_granting", @@ -6978,6 +7179,10 @@ def set_up_api_server_access_profile(self, mc: ManagedCluster) -> ManagedCluster """ self._ensure_mc(mc) + # Run BYO VNet trio validation first so clearer errors surface before the + # generic --apiserver-subnet-id checks inside _get_apiserver_subnet_id. + self.context.validate_byo_hosted_system_subnets() + api_server_access_profile = None api_server_authorized_ip_ranges = self.context.get_api_server_authorized_ip_ranges() enable_private_cluster = self.context.get_enable_private_cluster() @@ -6998,12 +7203,55 @@ def set_up_api_server_access_profile(self, mc: ManagedCluster) -> ManagedCluster if api_server_access_profile is None: api_server_access_profile = self.models.ManagedClusterAPIServerAccessProfile() api_server_access_profile.subnet_id = self.context.get_apiserver_subnet_id() + # BYO VNet for Managed System Pool (Automatic SKU) requires apiserver VNet + # integration. When the BYO subnet trio is provided, auto-enable vnet + # integration so users are not forced to pass --enable-apiserver-vnet-integration + # alongside the subnet IDs. + if ( + self.context.get_system_node_subnet_id() or + self.context.get_node_subnet_id() + ): + api_server_access_profile.enable_vnet_integration = True mc.api_server_access_profile = api_server_access_profile fqdn_subdomain = self.context.get_fqdn_subdomain() mc.fqdn_subdomain = fqdn_subdomain return mc + def set_up_hosted_system_profile(self, mc: ManagedCluster) -> ManagedCluster: + """Set up hosted_system_profile on the ManagedCluster for Automatic SKU clusters. + + Triggered when the user explicitly opts in via `--enable-hosted-system`, or + implicitly by supplying the BYO VNet subnet trio (`--system-node-subnet-id` / + `--node-subnet-id` / `--apiserver-subnet-id`). In either case: + - `mc.hosted_system_profile.enabled` is set to True so the RP treats this + as a Managed System Pool request. + - `system_node_subnet_id` / `node_subnet_id` are populated when supplied. + - `set_up_agentpool_profile` does not synthesize the default agent pool, + because the system pool is provisioned server-side from `hosted_system_profile`. + + :return: the ManagedCluster object + """ + self._ensure_mc(mc) + + # Run cross-flag validation (--enable-hosted-system SKU gate + BYO trio completeness) + self.context.validate_byo_hosted_system_subnets() + + system_node_subnet_id = self.context.get_system_node_subnet_id() + node_subnet_id = self.context.get_node_subnet_id() + enable_hosted_system = self.context.get_enable_hosted_system() + + if enable_hosted_system: + if mc.hosted_system_profile is None: + mc.hosted_system_profile = self.models.ManagedClusterHostedSystemProfile() + # Explicit enablement so the RP treats this as a Managed System Pool cluster. + mc.hosted_system_profile.enabled = True + if system_node_subnet_id: + mc.hosted_system_profile.system_node_subnet_id = system_node_subnet_id + if node_subnet_id: + mc.hosted_system_profile.node_subnet_id = node_subnet_id + return mc + def set_up_identity(self, mc: ManagedCluster) -> ManagedCluster: """Set up identity for the ManagedCluster object. @@ -7484,6 +7732,8 @@ def construct_mc_profile_default(self, bypass_restore_defaults: bool = False) -> mc = self.set_up_oidc_issuer_profile(mc) # set up api server access profile and fqdn subdomain mc = self.set_up_api_server_access_profile(mc) + # set up hosted system profile (BYO VNet for Managed System Pool) + mc = self.set_up_hosted_system_profile(mc) # set up identity mc = self.set_up_identity(mc) # set up identity profile @@ -7596,16 +7846,27 @@ def immediate_processing_after_request(self, mc: ManagedCluster) -> None: # Grant vnet permission to system assigned identity RIGHT AFTER the cluster is put, this operation can # reduce latency for the role assignment take effect instant_cluster = self.client.get(self.context.get_resource_group_name(), self.context.get_name()) - if not self.context.external_functions.add_role_assignment( - self.cmd, - "Network Contributor", - instant_cluster.identity.principal_id, - scope=self.context.get_vnet_subnet_id(), - is_service_principal=False, - ): - logger.warning( - "Could not create a role assignment for subnet. Are you an Owner on this subscription?" - ) + scopes = [] + vnet_subnet_id = self.context.get_vnet_subnet_id() + if vnet_subnet_id: + scopes.append(vnet_subnet_id) + byo_hosted_system_subnet_ids = self.context.get_intermediate( + "byo_hosted_system_subnets_pending_grant", default_value=[] + ) + for subnet_id in byo_hosted_system_subnet_ids or []: + if subnet_id and subnet_id not in scopes: + scopes.append(subnet_id) + for scope in scopes: + if not self.context.external_functions.add_role_assignment( + self.cmd, + "Network Contributor", + instant_cluster.identity.principal_id, + scope=scope, + is_service_principal=False, + ): + logger.warning( + "Could not create a role assignment for subnet. Are you an Owner on this subscription?" + ) # pylint: disable=too-many-locals def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: @@ -7984,6 +8245,12 @@ def update_agentpool_profile(self, mc: ManagedCluster) -> ManagedCluster: """ self._ensure_mc(mc) + # Automatic clusters with a Managed System Pool manage node pools on the + # server side and surface `agent_pool_profiles` as None. Skip the + # default agent pool update in that case. + if mc.hosted_system_profile and mc.hosted_system_profile.enabled: + return mc + if not mc.agent_pool_profiles: raise UnknownError( "Encounter an unexpected error while getting agent pool profiles from the cluster in the process of " diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py index 4d73ef0e470..b34f3279ce9 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py @@ -26,6 +26,7 @@ CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, CONST_OPEN_SERVICE_MESH_ADDON_NAME, CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING, + CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY, CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY, CONST_OUTBOUND_TYPE_LOAD_BALANCER, CONST_PRIVATE_DNS_ZONE_NONE, @@ -2122,6 +2123,96 @@ def test_get_outbound_type(self): expect_outbound_type_13 = CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY self.assertEqual(outbound_type_13,expect_outbound_type_13) + network_profile_14 = self.models.ContainerServiceNetworkProfile( + outbound_type=CONST_OUTBOUND_TYPE_LOAD_BALANCER + ) + mc_14 = self.models.ManagedCluster( + location="test_location", + network_profile=network_profile_14, + sku=self.models.ManagedClusterSKU(name="Automatic"), + ) + ctx_14 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({}), + self.models, + DecoratorMode.UPDATE, + ) + ctx_14.agentpool_context = mock.MagicMock() + ctx_14.agentpool_context.get_vnet_subnet_id.return_value = None + ctx_14.attach_mc(mc_14) + self.assertEqual(ctx_14.get_outbound_type(), CONST_OUTBOUND_TYPE_LOAD_BALANCER) + + ctx_14_1 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "automatic", + "outbound_type": CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx_14_1.agentpool_context = mock.MagicMock() + ctx_14_1.agentpool_context.get_vnet_subnet_id.return_value = None + with self.assertRaisesRegex( + RequiredArgumentMissingError, + "--system-node-subnet-id, --node-subnet-id and --apiserver-subnet-id", + ): + ctx_14_1.get_outbound_type() + + ctx_14_2 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "automatic", + "outbound_type": CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx_14_2.agentpool_context = mock.MagicMock() + ctx_14_2.agentpool_context.get_vnet_subnet_id.return_value = None + with self.assertRaisesRegex( + RequiredArgumentMissingError, + "--system-node-subnet-id, --node-subnet-id and --apiserver-subnet-id", + ): + ctx_14_2.get_outbound_type() + + byo_params = { + "sku": "automatic", + "system_node_subnet_id": "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/sys", + "node_subnet_id": "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/node", + "apiserver_subnet_id": "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/api", + } + ctx_15 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({**byo_params, "outbound_type": CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY}), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx_15.agentpool_context = mock.MagicMock() + ctx_15.agentpool_context.get_vnet_subnet_id.return_value = None + self.assertEqual(ctx_15.get_outbound_type(), CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY) + + ctx_16 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({**byo_params, "outbound_type": CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING}), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx_16.agentpool_context = mock.MagicMock() + ctx_16.agentpool_context.get_vnet_subnet_id.return_value = None + self.assertEqual(ctx_16.get_outbound_type(), CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING) + + ctx_17 = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({**byo_params, "outbound_type": CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY}), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx_17.agentpool_context = mock.MagicMock() + ctx_17.agentpool_context.get_vnet_subnet_id.return_value = None + with self.assertRaises(InvalidArgumentValueError): + ctx_17.get_outbound_type() + def test_get_network_plugin_mode(self): # default ctx_1 = AKSManagedClusterContext( @@ -4426,6 +4517,82 @@ def test_get_apiserver_subnet_id(self): with self.assertRaises(RequiredArgumentMissingError): ctx_6.get_apiserver_subnet_id() + def test_byo_hosted_system_subnets_validation(self): + system_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/sys" + node_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/nod" + api_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/api" + + # partial trio -> RequiredArgumentMissingError + ctx = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "automatic", + "system_node_subnet_id": system_subnet, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + with self.assertRaises(RequiredArgumentMissingError): + ctx.validate_byo_hosted_system_subnets() + + # trio without --sku automatic -> RequiredArgumentMissingError + ctx = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "base", + "system_node_subnet_id": system_subnet, + "node_subnet_id": node_subnet, + "apiserver_subnet_id": api_subnet, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + with self.assertRaises(RequiredArgumentMissingError): + ctx.validate_byo_hosted_system_subnets() + + # happy path: full trio + automatic + ctx = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "automatic", + "system_node_subnet_id": system_subnet, + "node_subnet_id": node_subnet, + "apiserver_subnet_id": api_subnet, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx.validate_byo_hosted_system_subnets() + self.assertEqual(ctx.get_system_node_subnet_id(), system_subnet) + self.assertEqual(ctx.get_node_subnet_id(), node_subnet) + self.assertTrue(ctx.get_enable_hosted_system()) # BYO trio implies enable_hosted_system + + # --enable-hosted-system without --sku automatic -> RequiredArgumentMissingError + ctx = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "base", + "enable_hosted_system": True, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + with self.assertRaises(RequiredArgumentMissingError): + ctx.validate_byo_hosted_system_subnets() + + # happy path: --enable-hosted-system alone on automatic + ctx = AKSManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "sku": "automatic", + "enable_hosted_system": True, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + ctx.validate_byo_hosted_system_subnets() + self.assertTrue(ctx.get_enable_hosted_system()) + def test_get_private_dns_zone(self): # default ctx_1 = AKSManagedClusterContext( @@ -6408,6 +6575,22 @@ def test_set_up_agentpool_profile(self): ground_truth_mc_1.agent_pool_profiles = [ground_truth_agentpool_profile_1] self.assertEqual(dec_mc_1, ground_truth_mc_1) + # Managed System Pool clusters get their system pool from hosted_system_profile, + # so the CLI should not synthesize a default agent pool up front. + dec_2 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "sku": "automatic", + "enable_hosted_system": True, + }, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_2 = self.models.ManagedCluster(location="test_location") + dec_2.context.attach_mc(mc_2) + dec_mc_2 = dec_2.set_up_agentpool_profile(mc_2) + self.assertIsNone(dec_mc_2.agent_pool_profiles) + def test_set_up_mc_properties(self): dec_1 = AKSManagedClusterCreateDecorator( self.cmd, @@ -6809,6 +6992,101 @@ def test_process_add_role_assignment_for_vnet_subnet(self): False, ) + # BYO VNet for Managed System Pool with user-assigned identity grants all BYO subnets. + system_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/system" + node_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/node" + api_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/api" + identity_obj = Mock( + principal_id="test_object_id", + ) + with patch( + "azure.cli.command_modules.acs.managed_cluster_decorator.AKSManagedClusterContext.get_identity_by_msi_client", + return_value=identity_obj, + ): + dec_6 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "enable_managed_identity": True, + "sku": "automatic", + "system_node_subnet_id": system_subnet, + "node_subnet_id": node_subnet, + "apiserver_subnet_id": api_subnet, + "skip_subnet_role_assignment": False, + "assign_identity": "test_assign_identity", + }, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_6 = self.models.ManagedCluster(location="test_location") + dec_6.context.attach_mc(mc_6) + with patch( + "azure.cli.command_modules.acs.managed_cluster_decorator.subnet_role_assignment_exists", + return_value=False, + ), patch( + "azure.cli.command_modules.acs.managed_cluster_decorator.add_role_assignment", + return_value=True, + ) as add_role_assignment: + dec_6.process_add_role_assignment_for_vnet_subnet(mc_6) + add_role_assignment.assert_has_calls([ + call( + self.cmd, + "Network Contributor", + "test_object_id", + is_service_principal=False, + scope=system_subnet, + ), + call( + self.cmd, + "Network Contributor", + "test_object_id", + is_service_principal=False, + scope=node_subnet, + ), + call( + self.cmd, + "Network Contributor", + "test_object_id", + is_service_principal=False, + scope=api_subnet, + ), + ]) + self.assertEqual(add_role_assignment.call_count, 3) + self.assertEqual( + dec_6.context.get_intermediate("need_post_creation_vnet_permission_granting"), + False, + ) + + # BYO VNet for Managed System Pool with system-assigned identity defers all BYO subnets. + dec_7 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "enable_managed_identity": True, + "sku": "automatic", + "system_node_subnet_id": system_subnet, + "node_subnet_id": node_subnet, + "apiserver_subnet_id": api_subnet, + "skip_subnet_role_assignment": False, + "assign_identity": None, + }, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_7 = self.models.ManagedCluster(location="test_location") + dec_7.context.attach_mc(mc_7) + with patch( + "azure.cli.command_modules.acs.managed_cluster_decorator.subnet_role_assignment_exists", + return_value=False, + ): + dec_7.process_add_role_assignment_for_vnet_subnet(mc_7) + self.assertEqual( + dec_7.context.get_intermediate("need_post_creation_vnet_permission_granting"), + True, + ) + self.assertEqual( + dec_7.context.get_intermediate("byo_hosted_system_subnets_pending_grant"), + [system_subnet, node_subnet, api_subnet], + ) + def test_process_attach_acr(self): # default value in `aks_create` dec_1 = AKSManagedClusterCreateDecorator( @@ -6874,6 +7152,51 @@ def test_process_attach_acr(self): dec_3.process_attach_acr(mc_3) ensure_assignment.assert_called_once_with(self.cmd, "test_service_principal", "test_registry_id", False, True, None) + def test_set_up_hosted_system_profile(self): + system_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/system" + node_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/node" + api_subnet = "/subscriptions/s/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/v/subnets/api" + + dec_1 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "sku": "automatic", + "system_node_subnet_id": system_subnet, + "node_subnet_id": node_subnet, + "apiserver_subnet_id": api_subnet, + }, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_1 = self.models.ManagedCluster(location="test_location") + dec_1.context.attach_mc(mc_1) + + dec_1.set_up_hosted_system_profile(mc_1) + + self.assertTrue(mc_1.hosted_system_profile.enabled) + self.assertEqual(mc_1.hosted_system_profile.system_node_subnet_id, system_subnet) + self.assertEqual(mc_1.hosted_system_profile.node_subnet_id, node_subnet) + self.assertIsNone(mc_1.agent_pool_profiles) + + dec_2 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "sku": "automatic", + "enable_hosted_system": True, + }, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_2 = self.models.ManagedCluster(location="test_location") + dec_2.context.attach_mc(mc_2) + user_pool = self.models.ManagedClusterAgentPoolProfile(name="userpool") + mc_2.agent_pool_profiles = [user_pool] + + dec_2.set_up_hosted_system_profile(mc_2) + + self.assertTrue(mc_2.hosted_system_profile.enabled) + self.assertEqual(mc_2.agent_pool_profiles, [user_pool]) + def test_set_up_network_profile(self): # default value in `aks_create` dec_1 = AKSManagedClusterCreateDecorator( @@ -8541,6 +8864,49 @@ def test_immediate_processing_after_request(self): is_service_principal=False, ) + dec_2 = AKSManagedClusterCreateDecorator( + self.cmd, + self.client, + {}, + ResourceType.MGMT_CONTAINERSERVICE, + ) + mc_2 = self.models.ManagedCluster(location="test_location") + dec_2.context.attach_mc(mc_2) + dec_2.context.set_intermediate("need_post_creation_vnet_permission_granting", True) + dec_2.context.set_intermediate( + "byo_hosted_system_subnets_pending_grant", + ["test_system_subnet_id", "test_node_subnet_id", "test_api_subnet_id"], + ) + self.client.get = Mock(return_value=Mock(identity=Mock(principal_id="test_principal_id"))) + with patch( + "azure.cli.command_modules.acs.managed_cluster_decorator.add_role_assignment", return_value=True + ) as mock_add: + dec_2.immediate_processing_after_request(mc_2) + mock_add.assert_has_calls([ + call( + self.cmd, + "Network Contributor", + "test_principal_id", + scope="test_system_subnet_id", + is_service_principal=False, + ), + call( + self.cmd, + "Network Contributor", + "test_principal_id", + scope="test_node_subnet_id", + is_service_principal=False, + ), + call( + self.cmd, + "Network Contributor", + "test_principal_id", + scope="test_api_subnet_id", + is_service_principal=False, + ), + ]) + self.assertEqual(mock_add.call_count, 3) + def test_postprocessing_after_mc_created(self): dec_1 = AKSManagedClusterCreateDecorator( self.cmd,