Skip to content

Commit

Permalink
[AKS] BREAKING CHANGE: aks create: Change the default value of `--e…
Browse files Browse the repository at this point in the history
…nable-msi-auth-for-monitoring` to true and add check for airgap clouds (#26356)
  • Loading branch information
wanlonghenry authored May 18, 2023
1 parent 20a4fc9 commit ed51b0a
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 27 deletions.
31 changes: 11 additions & 20 deletions src/azure-cli/azure/cli/command_modules/acs/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
from azure.cli.command_modules.acs._validators import extract_comma_separated_string
from azure.cli.command_modules.acs.addonconfiguration import (
add_ingress_appgw_addon_role_assignment,
add_monitoring_role_assignment,
add_virtual_node_role_assignment,
ensure_container_insights_for_monitoring,
ensure_default_log_analytics_workspace_for_monitoring,
Expand Down Expand Up @@ -452,7 +451,7 @@ def aks_create(
# addons
enable_addons=None,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None,
aci_subnet_name=None,
Expand Down Expand Up @@ -869,7 +868,7 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,
enable_secret_rotation=False,
rotation_poll_interval=None,
no_wait=False,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None,):
instance = client.get(resource_group_name, name)
Expand Down Expand Up @@ -940,21 +939,6 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,
result = LongRunningOperation(cmd.cli_ctx)(
client.begin_create_or_update(resource_group_name, name, instance))

# For monitoring addon, Metrics role assignement doesnt require in case of MSI auth
if enable_monitoring and not enable_msi_auth_for_monitoring:
cloud_name = cmd.cli_ctx.cloud.name
# mdm metrics supported only in Azure Public cloud so add the role assignment only in this cloud
if cloud_name.lower() == 'azurecloud':
from msrestazure.tools import resource_id
cluster_resource_id = resource_id(
subscription=subscription_id,
resource_group=resource_group_name,
namespace='Microsoft.ContainerService', type='managedClusters',
name=name
)
add_monitoring_role_assignment(
result, cluster_resource_id, cmd)

if ingress_appgw_addon_enabled:
add_ingress_appgw_addon_role_assignment(result, cmd)

Expand All @@ -975,7 +959,7 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,

def _update_addons(cmd, instance, subscription_id, resource_group_name, name, addons, enable,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
subnet_name=None,
appgw_name=None,
appgw_subnet_cidr=None,
Expand Down Expand Up @@ -1033,9 +1017,16 @@ def _update_addons(cmd, instance, subscription_id, resource_group_name, name, ad
workspace_resource_id = '/' + workspace_resource_id
if workspace_resource_id.endswith('/'):
workspace_resource_id = workspace_resource_id.rstrip('/')

cloud_name = cmd.cli_ctx.cloud.name
if enable_msi_auth_for_monitoring and (cloud_name.lower() == 'ussec' or cloud_name.lower() == 'usnat'):
if instance.identity is not None and instance.identity.type is not None and instance.identity.type == "userassigned":
logger.warning("--enable_msi_auth_for_monitoring is not supported in %s cloud and continuing monitoring enablement without this flag.", cloud_name)
enable_msi_auth_for_monitoring = False

addon_profile.config = {
CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id}
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = enable_msi_auth_for_monitoring
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = "true" if enable_msi_auth_for_monitoring else "false"
elif addon == (CONST_VIRTUAL_NODE_ADDON_NAME + os_type):
if addon_profile.enabled:
raise CLIError('The virtual-node addon is already enabled for this managed cluster.\n'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1309,4 +1309,4 @@ interactions:
status:
code: 202
message: Accepted
version: 1
version: 1
Original file line number Diff line number Diff line change
Expand Up @@ -7323,7 +7323,6 @@ def create_new_cluster_with_monitoring_aad_auth(self, resource_group, resource_g
create_cmd = f'aks create --resource-group={resource_group} --name={aks_name} --location={resource_group_location} ' \
'--enable-managed-identity ' \
'--enable-addons monitoring ' \
'--enable-msi-auth-for-monitoring ' \
'--node-count 1 ' \
'--ssh-key-value={ssh_key_value} '
create_cmd += f'--assign-identity {identity_id} ' if user_assigned_identity else ''
Expand All @@ -7332,7 +7331,7 @@ def create_new_cluster_with_monitoring_aad_auth(self, resource_group, resource_g

response = self.cmd(create_cmd, checks=[
self.check('addonProfiles.omsagent.enabled', True),
self.check('addonProfiles.omsagent.config.useAADAuth', 'True')
self.check('addonProfiles.omsagent.config.useAADAuth', 'true')
]).get_output_in_json()

cluster_resource_id = response["id"]
Expand Down Expand Up @@ -7433,14 +7432,13 @@ def enable_monitoring_existing_cluster_aad_auth(self, resource_group, resource_g
create_cmd += f'--assign-identity {identity_id}' if user_assigned_identity else ''
self.cmd(create_cmd)

enable_monitoring_cmd = f'aks enable-addons -a monitoring --resource-group={resource_group} --name={aks_name} ' \
'--enable-msi-auth-for-monitoring '
enable_monitoring_cmd = f'aks enable-addons -a monitoring --resource-group={resource_group} --name={aks_name} '
if syslog_enabled:
enable_monitoring_cmd += f'--enable-syslog '

response = self.cmd(enable_monitoring_cmd, checks=[
self.check('addonProfiles.omsagent.enabled', True),
self.check('addonProfiles.omsagent.config.useAADAuth', 'True')
self.check('addonProfiles.omsagent.config.useAADAuth', 'true')
]).get_output_in_json()

cluster_resource_id = response["id"]
Expand Down Expand Up @@ -7497,7 +7495,7 @@ def test_aks_create_with_monitoring_legacy_auth(self, resource_group, resource_g
response = self.cmd(create_cmd, checks=[
self.check('addonProfiles.omsagent.enabled', True),
self.exists('addonProfiles.omsagent.config.logAnalyticsWorkspaceResourceID'),
self.check('addonProfiles.omsagent.config.useAADAuth', 'False')
self.check('addonProfiles.omsagent.config.useAADAuth', 'false')
]).get_output_in_json()

# make sure a DCR was not created
Expand Down

0 comments on commit ed51b0a

Please sign in to comment.