From a0725c727d937b0df480a47edd83e6044f1c57f7 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Tue, 21 Jul 2026 18:32:02 +0100 Subject: [PATCH 1/9] aks-preview: migrate Container Insights logs to azureMonitorProfile.containerInsights; deprecate monitoring addon --- src/aks-preview/HISTORY.rst | 5 + src/aks-preview/azext_aks_preview/_help.py | 4 +- src/aks-preview/azext_aks_preview/_params.py | 12 +- .../azext_aks_preview/_validators.py | 45 +++- .../managed_cluster_decorator.py | 252 ++++++++++-------- .../latest/test_managed_cluster_decorator.py | 155 ++++++----- src/aks-preview/setup.py | 2 +- 7 files changed, 290 insertions(+), 185 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index a587efce50a..bd8efd40e46 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -13,6 +13,11 @@ Pending +++++++ * `az aks nodepool update`: Avoid applying VirtualMachines autoscaler profile conversions twice with newer Azure CLI versions while preserving compatibility with older CLI versions. +21.0.0b14 +++++++++ +* `az aks create/update`: `--enable-azure-monitor-logs` / `--disable-azure-monitor-logs` now configure the modern ``azureMonitorProfile.containerInsights`` profile instead of the legacy ``addonProfiles.omsagent`` addon. The containerInsights path always uses managed-identity (MSI/AAD) auth. +* `az aks create/update`: Deprecate the ``monitoring`` addon (``--enable-addons monitoring`` / ``--disable-addons monitoring``) for Container Insights in favor of ``--enable-azure-monitor-logs`` / ``--disable-azure-monitor-logs``. + 21.0.0b13 ++++++++ * `az aks maintenanceconfiguration add` and `az aks maintenanceconfiguration update`: Add `--maintenance-window-id` (preview) to link a maintenance configuration to a shared MaintenanceWindow resource. When set, the schedule lives in the referenced MaintenanceWindow and inline schedule arguments cannot be used; omit it for no shared resource. `--maintenance-window-id` cannot be combined with `--config-file` (set the `maintenanceWindowId` property in the JSON instead) and cannot be empty. Requires the `Microsoft.ContainerService/AKSSharedMaintenanceWindowPreview` feature to be registered on the subscription. diff --git a/src/aks-preview/azext_aks_preview/_help.py b/src/aks-preview/azext_aks_preview/_help.py index 6ccb248cba6..78e5ee4ab08 100644 --- a/src/aks-preview/azext_aks_preview/_help.py +++ b/src/aks-preview/azext_aks_preview/_help.py @@ -193,7 +193,7 @@ - name: --enable-azure-monitor-logs type: bool short-summary: Enable Azure Monitor logs for the cluster. - long-summary: This is equivalent to using "--enable-addons monitoring". Turn on Log Analytics monitoring. Uses the Log Analytics Default Workspace if it exists, else creates one. Specify "--workspace-resource-id" to use an existing workspace. If monitoring addon is enabled --no-wait argument will have no effect + long-summary: Enables Container Insights logs for the cluster by configuring the azureMonitorProfile.containerInsights profile (managed-identity/MSI auth). This is the preferred way to onboard Container Insights and supersedes the deprecated "--enable-addons monitoring". Uses the Log Analytics Default Workspace if it exists, else creates one. Specify "--workspace-resource-id" to use an existing workspace. If monitoring is enabled --no-wait argument will have no effect - name: --disable-rbac type: bool short-summary: Disable Kubernetes Role-Based Access Control. @@ -1116,7 +1116,7 @@ - name: --enable-azure-monitor-logs type: bool short-summary: Enable Azure Monitor logs for the cluster. - long-summary: This is equivalent to using "az aks enable-addons -a monitoring". Enables Log Analytics monitoring for the cluster. Uses the Log Analytics Default Workspace if it exists, else creates one. Specify "--workspace-resource-id" to use an existing workspace. If monitoring addon is enabled --no-wait argument will have no effect + long-summary: Enables Container Insights logs for the cluster by configuring the azureMonitorProfile.containerInsights profile (managed-identity/MSI auth). This is the preferred way to onboard Container Insights and supersedes the deprecated "az aks enable-addons -a monitoring". Uses the Log Analytics Default Workspace if it exists, else creates one. Specify "--workspace-resource-id" to use an existing workspace. If monitoring is enabled --no-wait argument will have no effect - name: --disable-azure-monitor-logs type: bool short-summary: Disable Azure Monitor logs for the cluster. diff --git a/src/aks-preview/azext_aks_preview/_params.py b/src/aks-preview/azext_aks_preview/_params.py index a5a81f76665..98988fa1ea8 100644 --- a/src/aks-preview/azext_aks_preview/_params.py +++ b/src/aks-preview/azext_aks_preview/_params.py @@ -828,7 +828,9 @@ def load_arguments(self, _): "enable_azure_monitor_logs", action="store_true", validator=validate_azure_monitor_logs_and_enable_addons, - help="Enable Azure Monitor logs for the cluster. Equivalent to '--enable-addons monitoring'." + help="Enable Azure Monitor logs (Container Insights) for the cluster. Configures the " + "azureMonitorProfile.containerInsights profile (managed-identity/MSI auth). Preferred " + "over the deprecated '--enable-addons monitoring'." ) c.argument("workspace_resource_id") c.argument( @@ -1562,13 +1564,17 @@ def load_arguments(self, _): "enable_azure_monitor_logs", action="store_true", validator=validate_azure_monitor_logs_enable_disable, - help="Enable Azure Monitor logs for the cluster. Equivalent to 'az aks enable-addons -a monitoring'." + help="Enable Azure Monitor logs (Container Insights) for the cluster. Configures the " + "azureMonitorProfile.containerInsights profile (managed-identity/MSI auth). Preferred " + "over the deprecated 'az aks enable-addons -a monitoring'." ) # Monitoring parameters are inherited from base CLI c.argument( "disable_azure_monitor_logs", action="store_true", - help="Disable Azure Monitor logs for the cluster. Equivalent to 'az aks disable-addons -a monitoring'." + help="Disable Azure Monitor logs (Container Insights) for the cluster. Clears the " + "azureMonitorProfile.containerInsights profile. Preferred over the deprecated " + "'az aks disable-addons -a monitoring'." ) c.argument("enable_secret_rotation", action="store_true") c.argument("disable_secret_rotation", action="store_true") diff --git a/src/aks-preview/azext_aks_preview/_validators.py b/src/aks-preview/azext_aks_preview/_validators.py index b84add3f8bb..968c022dd1b 100644 --- a/src/aks-preview/azext_aks_preview/_validators.py +++ b/src/aks-preview/azext_aks_preview/_validators.py @@ -621,6 +621,8 @@ def validate_addons(namespace): addons = namespace.addons addon_args = addons.split(',') _recognize_addons(addon_args) + if 'monitoring' in addon_args: + _warn_monitoring_addon_deprecated() def validate_pod_identity_pod_labels(namespace): @@ -1201,15 +1203,44 @@ def validate_azure_monitor_and_opentelemetry_for_update(namespace): validate_opentelemetry_logs_dependencies_for_update(namespace) +def _warn_monitoring_addon_deprecated(): + """Emit a deprecation warning steering users to --enable-azure-monitor-logs.""" + logger.warning( + "The 'monitoring' addon (--enable-addons monitoring / --disable-addons monitoring) is " + "deprecated for Container Insights. Use '--enable-azure-monitor-logs' / " + "'--disable-azure-monitor-logs' instead, which configure the azureMonitorProfile." + ) + + def validate_azure_monitor_logs_and_enable_addons(namespace): """Validate that enable_azure_monitor_logs and enable_addons don't conflict.""" + enable_addons = getattr(namespace, 'enable_addons', None) if hasattr(namespace, 'enable_azure_monitor_logs') and namespace.enable_azure_monitor_logs: - if hasattr(namespace, 'enable_addons') and namespace.enable_addons: - if 'monitoring' in namespace.enable_addons: - raise ArgumentUsageError( - "Cannot specify both '--enable-azure-monitor-logs' and '--enable-addons monitoring'. " - "Use either '--enable-azure-monitor-logs' or '--enable-addons monitoring'." - ) + if enable_addons and 'monitoring' in enable_addons: + raise ArgumentUsageError( + "Cannot specify both '--enable-azure-monitor-logs' and '--enable-addons monitoring'. " + "Use either '--enable-azure-monitor-logs' or '--enable-addons monitoring'." + ) + # The containerInsights path is always MSI/AAD; legacy (non-MSI) auth cannot be expressed. + _validate_monitor_logs_requires_msi_auth(namespace) + elif enable_addons and 'monitoring' in enable_addons: + _warn_monitoring_addon_deprecated() + + +def _validate_monitor_logs_requires_msi_auth(namespace): + """--enable-azure-monitor-logs uses the containerInsights path, which is always MSI/AAD auth. + + Legacy (non-MSI) auth has no representation on that surface, so reject an explicit request to + disable MSI auth alongside it. + """ + disable_msi_auth = getattr(namespace, 'disable_msi_auth_for_monitoring', None) + enable_msi_auth = getattr(namespace, 'enable_msi_auth_for_monitoring', None) + if disable_msi_auth or enable_msi_auth is False: + raise ArgumentUsageError( + "'--enable-azure-monitor-logs' onboards Container Insights via the azureMonitorProfile, " + "which always uses managed-identity (MSI/AAD) authentication. It cannot be combined with " + "disabling MSI auth for monitoring." + ) def validate_azure_monitor_logs_enable_disable(namespace): @@ -1220,6 +1251,8 @@ def validate_azure_monitor_logs_enable_disable(namespace): "Cannot specify both '--enable-azure-monitor-logs' and '--disable-azure-monitor-logs'. " "Use either '--enable-azure-monitor-logs' or '--disable-azure-monitor-logs'." ) + if hasattr(namespace, 'enable_azure_monitor_logs') and namespace.enable_azure_monitor_logs: + _validate_monitor_logs_requires_msi_auth(namespace) def validate_nat_gateway_managed_outbound_ipv6_count(namespace): diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 3249aa2ef3e..b2104895132 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -4592,11 +4592,16 @@ def set_up_addon_profiles(self, mc: ManagedCluster) -> ManagedCluster: container_network_logs_enabled = self.context.get_container_network_logs(mc) if container_network_logs_enabled is not None: - monitoring_addon_profile = addon_profiles.get(addon_consts.get("CONST_MONITORING_ADDON_NAME")) - if monitoring_addon_profile: - config = monitoring_addon_profile.config or {} - config["enableRetinaNetworkFlags"] = str(container_network_logs_enabled) - monitoring_addon_profile.config = config + # Container network logs are configured on the modern azureMonitorProfile.containerInsights + # surface (containerNetworkLogs enum) rather than the legacy omsagent addon config. + self._ensure_azure_monitor_profile(mc) + if mc.azure_monitor_profile.container_insights is None: + mc.azure_monitor_profile.container_insights = ( + self.models.ManagedClusterAzureMonitorProfileContainerInsights() + ) + mc.azure_monitor_profile.container_insights.container_network_logs = ( + "Enabled" if container_network_logs_enabled else "Disabled" + ) # Trigger validation for high log scale mode when container network logs are enabled. # This ensures proper error messages are raised before cluster creation if the user @@ -5023,18 +5028,14 @@ def _setup_azure_monitor_app_monitoring(self, mc: ManagedCluster) -> None: ) def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: - """Set up Azure Monitor logs configuration.""" - - addon_consts = self.context.get_addon_consts() + """Set up Azure Monitor logs (Container Insights) via azureMonitorProfile.containerInsights. - if mc.addon_profiles is None: - mc.addon_profiles = {} - - CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME") - addon_profile = mc.addon_profiles.get( - CONST_MONITORING_ADDON_NAME, - self.models.ManagedClusterAddonProfile(enabled=False)) - addon_profile.enabled = True + Writes the modern ``azureMonitorProfile.containerInsights`` surface instead of the legacy + ``addonProfiles.omsagent`` addon. The AKS RP normalizes containerInsights into omsagent on + the PUT (defaulting ``useAADAuth=true``), so the ama-logs agent is still provisioned and + MSI/AAD auth is implied on this path. See plan.md for the RP contract details. + """ + self._ensure_azure_monitor_profile(mc) # Get or create workspace resource ID workspace_resource_id = self.context.raw_param.get("workspace_resource_id") @@ -5051,28 +5052,16 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: sanitize_func = self.context.external_functions.sanitize_loganalytics_ws_resource_id workspace_resource_id = sanitize_func(workspace_resource_id) - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( - "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - - # Get MSI auth setting using the same logic as update decorator - enable_msi_auth_bool = self.context.get_enable_msi_auth_for_monitoring() - - if enable_msi_auth_bool: - enable_msi_auth = "true" - else: - enable_msi_auth = "false" - - # Create completely new config - addon_profile.config = { - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id, - CONST_MONITORING_USING_AAD_MSI_AUTH: enable_msi_auth - } - mc.addon_profiles[CONST_MONITORING_ADDON_NAME] = addon_profile + container_insights = mc.azure_monitor_profile.container_insights + if container_insights is None: + container_insights = self.models.ManagedClusterAzureMonitorProfileContainerInsights() + container_insights.enabled = True + container_insights.log_analytics_workspace_resource_id = workspace_resource_id + mc.azure_monitor_profile.container_insights = container_insights # DCR and DCRA creation is deferred to postprocessing_after_mc_created # so that all flags are finalized and the cluster exists. - # Only MSI clusters need a DCR. + # Only MSI clusters need a DCR; the containerInsights path is always MSI/AAD. self.context.set_intermediate("monitoring_addon_enabled", True, overwrite_exists=True) def _setup_opentelemetry_metrics(self, mc: ManagedCluster) -> None: @@ -5910,15 +5899,10 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: cluster, cluster_resource_id, self.cmd ) elif self._should_create_dcra(): - addon_consts = self.context.get_addon_consts() - monitoring_addon_key = ( - _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) - if cluster.addon_profiles - else addon_consts.get("CONST_MONITORING_ADDON_NAME") - ) + monitoring_addon_profile = self._get_monitoring_addon_profile_for_dcr(cluster) self.context.external_functions.ensure_container_insights_for_monitoring( self.cmd, - cluster.addon_profiles[monitoring_addon_key], + monitoring_addon_profile, self.context.get_subscription_id(), self.context.get_resource_group_name(), self.context.get_name(), @@ -6175,6 +6159,46 @@ def _is_cnl_or_hlsm_changing(self) -> bool: params.get("enable_high_log_scale_mode") is not None ) + def _get_monitoring_addon_profile_for_dcr(self, cluster: ManagedCluster): + """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes. + + Container Insights is now configured via ``azureMonitorProfile.containerInsights`` and the + AKS RP normalizes it into ``addonProfiles.omsagent`` on the PUT response. When that mirrored + addon is present we use it directly. Otherwise (e.g. mocked responses in unit tests) we build + a transient omsagent-style profile from ``containerInsights`` so the DCR engine still gets the + workspace id and MSI/AAD auth signal. This object is only passed to the engine, never persisted. + """ + addon_consts = self.context.get_addon_consts() + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( + "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") + CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") + + monitoring_addon_key = ( + _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) + if cluster.addon_profiles + else None + ) + if (monitoring_addon_key and cluster.addon_profiles and + monitoring_addon_key in cluster.addon_profiles and + cluster.addon_profiles[monitoring_addon_key].config): + return cluster.addon_profiles[monitoring_addon_key] + + workspace_resource_id = None + container_insights = ( + cluster.azure_monitor_profile.container_insights + if cluster.azure_monitor_profile + else None + ) + if container_insights is not None: + workspace_resource_id = container_insights.log_analytics_workspace_resource_id + return self.models.ManagedClusterAddonProfile( + enabled=True, + config={ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id, + CONST_MONITORING_USING_AAD_MSI_AUTH: "true", + }, + ) + class AKSPreviewManagedClusterUpdateDecorator(AKSManagedClusterUpdateDecorator): def __init__( @@ -6464,14 +6488,15 @@ def update_monitoring_profile_flow_logs(self, mc: ManagedCluster) -> ManagedClus container_network_logs_enabled = self.context.get_container_network_logs(mc) if container_network_logs_enabled is not None: - if mc.addon_profiles: - addon_consts = self.context.get_addon_consts() - monitoring_addon_key = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_addon_profile = mc.addon_profiles.get(monitoring_addon_key) - if monitoring_addon_profile: - config = monitoring_addon_profile.config or {} - config["enableRetinaNetworkFlags"] = str(container_network_logs_enabled) - mc.addon_profiles[monitoring_addon_key].config = config + # Container network logs are configured on the modern containerInsights surface. + self._ensure_azure_monitor_profile(mc) + if mc.azure_monitor_profile.container_insights is None: + mc.azure_monitor_profile.container_insights = ( + self.models.ManagedClusterAzureMonitorProfileContainerInsights() + ) + mc.azure_monitor_profile.container_insights.container_network_logs = ( + "Enabled" if container_network_logs_enabled else "Disabled" + ) # When enabling CNL, the DCR must be updated to add the high-scale stream. # Set the postprocessing intermediate so that the update path calls ensure_container_insights. @@ -8477,16 +8502,15 @@ def _ensure_azure_monitor_profile(self, mc: ManagedCluster) -> None: mc.azure_monitor_profile = self.models.ManagedClusterAzureMonitorProfile() def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: - """Set up Azure Monitor logs configuration.""" + """Set up Azure Monitor logs (Container Insights) via azureMonitorProfile.containerInsights. + Writes the modern ``azureMonitorProfile.containerInsights`` surface instead of the legacy + ``addonProfiles.omsagent`` addon. The AKS RP normalizes containerInsights into omsagent on + the PUT (defaulting ``useAADAuth=true``), so MSI/AAD auth is implied on this path. + """ addon_consts = self.context.get_addon_consts() - if mc.addon_profiles is None: - mc.addon_profiles = {} - - CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME") CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") # Get or create workspace resource ID workspace_resource_id = self.context.raw_param.get("workspace_resource_id") @@ -8503,57 +8527,46 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: sanitize_func = self.context.external_functions.sanitize_loganalytics_ws_resource_id workspace_resource_id = sanitize_func(workspace_resource_id) - # Call get_enable_msi_auth_for_monitoring BEFORE detecting the existing key, - # because the parent's implementation may normalize addon_profiles keys in-place - # (e.g., renaming "omsAgent" to "omsagent"). - enable_msi_auth_bool = self.context.get_enable_msi_auth_for_monitoring() - if enable_msi_auth_bool: - enable_msi_auth = "true" - else: - enable_msi_auth = "false" - - # Detect existing key (could be "omsagent" or "omsAgent" from Azure API) - existing_key = None - if CONST_MONITORING_ADDON_NAME in mc.addon_profiles: - existing_key = CONST_MONITORING_ADDON_NAME - elif CONST_MONITORING_ADDON_NAME_CAMELCASE in mc.addon_profiles: - existing_key = CONST_MONITORING_ADDON_NAME_CAMELCASE - - if existing_key: - addon_profile = mc.addon_profiles[existing_key] - # Detect workspace change: if the workspace is different from the existing one, - # trigger DCR postprocessing so the DCR destination gets updated. - old_config = addon_profile.config or {} - old_workspace = old_config.get(CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, "") - if old_workspace and old_workspace.lower() != workspace_resource_id.lower(): - self.context.set_intermediate( - "monitoring_addon_postprocessing_required", True, overwrite_exists=True) - else: - addon_profile = self.models.ManagedClusterAddonProfile(enabled=False) - existing_key = CONST_MONITORING_ADDON_NAME - - addon_profile.enabled = True + # Detect workspace change from the existing surfaces (containerInsights preferred, omsagent + # addon fallback for brownfield clusters) so DCR postprocessing updates the DCR destination. + old_workspace = "" + if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and + mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id): + old_workspace = mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id + elif mc.addon_profiles: + existing_key = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) + existing_profile = mc.addon_profiles.get(existing_key) if existing_key else None + if existing_profile and existing_profile.config: + old_workspace = existing_profile.config.get( + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, "") or "" + if old_workspace and old_workspace.lower() != workspace_resource_id.lower(): + self.context.set_intermediate( + "monitoring_addon_postprocessing_required", True, overwrite_exists=True) - new_config = { - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id, - CONST_MONITORING_USING_AAD_MSI_AUTH: enable_msi_auth - } + self._ensure_azure_monitor_profile(mc) + container_insights = mc.azure_monitor_profile.container_insights + if container_insights is None: + container_insights = self.models.ManagedClusterAzureMonitorProfileContainerInsights() + container_insights.enabled = True + container_insights.log_analytics_workspace_resource_id = workspace_resource_id - # Also set enableRetinaNetworkFlags if container network logs are being enabled - # in the same command. This must be done here because update_monitoring_profile_flow_logs - # may run before update_addon_profiles when the base class calls it first. + # Container network logs are configured on the containerInsights surface as well. container_network_logs_enabled = self.context.get_container_network_logs(mc) if container_network_logs_enabled is not None: - new_config["enableRetinaNetworkFlags"] = str(container_network_logs_enabled) - - # Replace the entire config, not just individual keys - addon_profile.config = new_config + container_insights.container_network_logs = ( + "Enabled" if container_network_logs_enabled else "Disabled" + ) - mc.addon_profiles[existing_key] = addon_profile + mc.azure_monitor_profile.container_insights = container_insights self.context.set_intermediate("monitoring_addon_enabled", True, overwrite_exists=True) def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: - """Disable Azure Monitor logs configuration.""" + """Disable Azure Monitor logs configuration. + + Clears ``azureMonitorProfile.containerInsights`` (the RP source of truth) as the primary + surface, and also clears the legacy ``addonProfiles.omsagent`` addon when present (brownfield + clusters / RP-mirrored state) so the RP does not re-enable the addon. + """ addon_consts = self.context.get_addon_consts() CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") @@ -8564,12 +8577,21 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: if addon_key not in mc.addon_profiles: addon_key = None - # If the addon profile doesn't exist at all, there's nothing to disable - if not addon_key: + container_insights = ( + mc.azure_monitor_profile.container_insights + if mc.azure_monitor_profile + else None + ) + + # If neither the legacy addon nor the containerInsights surface exists, nothing to disable + if not addon_key and container_insights is None: return - # Check if Azure Monitor logs (monitoring addon) is currently enabled - azure_monitor_logs_enabled = mc.addon_profiles[addon_key].enabled + # Check if Azure Monitor logs is currently enabled on either surface + azure_monitor_logs_enabled = ( + (addon_key is not None and mc.addon_profiles[addon_key].enabled) or + (container_insights is not None and container_insights.enabled) + ) # Check if OpenTelemetry logs are enabled and prompt for confirmation opentelemetry_logs_enabled = ( @@ -8587,11 +8609,16 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: if not prompt_y_n(msg, default="n"): raise CLIError("Operation cancelled.") - # Check if MSI auth is enabled - if so, cleanup DCR/DCRA BEFORE disabling (same as aks_disable_addons) - addon_config = mc.addon_profiles[addon_key].config - has_msi_auth_key = addon_config and CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config - msi_auth_enabled = (addon_config and has_msi_auth_key and - str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true") + # Check if MSI auth is enabled - if so, cleanup DCR/DCRA BEFORE disabling (same as aks_disable_addons). + # The containerInsights path is always MSI/AAD, so treat it as MSI-enabled when the legacy + # addon config is not present. + if addon_key is not None: + addon_config = mc.addon_profiles[addon_key].config + has_msi_auth_key = addon_config and CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config + msi_auth_enabled = (addon_config and has_msi_auth_key and + str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true") + else: + msi_auth_enabled = container_insights is not None # Perform DCR/DCRA cleanup BEFORE disabling (same as aks_disable_addons lines 2796-2822) if azure_monitor_logs_enabled and msi_auth_enabled: @@ -8626,11 +8653,12 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: # Ignore TypeError just like aks_disable_addons does (line 2823) pass - # Now disable the addon and clear configuration - mc.addon_profiles[addon_key].enabled = False - mc.addon_profiles[addon_key].config = None + # Now disable the legacy addon (when present) and clear its configuration + if addon_key is not None: + mc.addon_profiles[addon_key].enabled = False + mc.addon_profiles[addon_key].config = None - # Also disable azureMonitorProfile.containerInsights (the new API surface) + # Disable azureMonitorProfile.containerInsights (the source-of-truth API surface). # The RP uses containerInsights.enabled as the source of truth; if it remains # true while the legacy addon is disabled, the RP re-enables the addon. if (mc.azure_monitor_profile and diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index acd48a0daa8..161374d03a9 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -12435,13 +12435,12 @@ def test_update_enable_azure_monitor_logs(self): ): dec_mc_1 = dec_1.update_addon_profiles(mc_1) - # Verify monitoring addon is enabled - self.assertIn(CONST_MONITORING_ADDON_NAME, dec_mc_1.addon_profiles) - self.assertTrue(dec_mc_1.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) + # Verify Container Insights is enabled on the azureMonitorProfile surface + self.assertIsNotNone(dec_mc_1.azure_monitor_profile) + self.assertIsNotNone(dec_mc_1.azure_monitor_profile.container_insights) + self.assertTrue(dec_mc_1.azure_monitor_profile.container_insights.enabled) self.assertEqual( - dec_mc_1.addon_profiles[CONST_MONITORING_ADDON_NAME].config[ - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID - ], + dec_mc_1.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id, "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/test-workspace", ) @@ -12509,9 +12508,9 @@ def test_update_enable_azure_monitor_logs(self): ): dec_mc_3 = dec_3.update_azure_monitor_profile(dec_mc_3) - # Verify monitoring addon is enabled - self.assertIn(CONST_MONITORING_ADDON_NAME, dec_mc_3.addon_profiles) - self.assertTrue(dec_mc_3.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) + # Verify Container Insights is enabled + self.assertIsNotNone(dec_mc_3.azure_monitor_profile.container_insights) + self.assertTrue(dec_mc_3.azure_monitor_profile.container_insights.enabled) # Verify OpenTelemetry logs are configured if ( @@ -12555,15 +12554,9 @@ def test_update_enable_azure_monitor_logs(self): ): dec_mc_4 = dec_4.update_addon_profiles(mc_4) - # Verify MSI auth is enabled - self.assertIn(CONST_MONITORING_ADDON_NAME, dec_mc_4.addon_profiles) - self.assertTrue(dec_mc_4.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) - self.assertEqual( - dec_mc_4.addon_profiles[CONST_MONITORING_ADDON_NAME].config[ - CONST_MONITORING_USING_AAD_MSI_AUTH - ], - "true", - ) + # Verify Container Insights is enabled; MSI/AAD auth is implied on this path (RP-defaulted) + self.assertIsNotNone(dec_mc_4.azure_monitor_profile.container_insights) + self.assertTrue(dec_mc_4.azure_monitor_profile.container_insights.enabled) def test_update_disable_azure_monitor_logs(self): # Test disabling Azure Monitor logs when currently enabled @@ -13081,18 +13074,18 @@ def test_setup_azure_monitor_logs_with_omsagent_camelcase(self): # Call _setup_azure_monitor_logs dec_1._setup_azure_monitor_logs(mc_1) - # Verify: The existing key is preserved (no duplicate created). - # The implementation keeps the original casing ("omsAgent") found in addon_profiles. - self.assertEqual( - len([k for k in mc_1.addon_profiles if k.lower() == "omsagent"]), 1 - ) # No duplicate - # Find the actual key used (could be normalized or preserved depending on parent behavior) - actual_key = next(k for k in mc_1.addon_profiles if k.lower() == "omsagent") - self.assertTrue(mc_1.addon_profiles[actual_key].enabled) + # Verify: Container Insights is configured on the azureMonitorProfile surface with the new + # workspace. The legacy omsAgent addon is left untouched (brownfield) — the RP reconciles it. + self.assertIsNotNone(mc_1.azure_monitor_profile.container_insights) + self.assertTrue(mc_1.azure_monitor_profile.container_insights.enabled) self.assertEqual( - mc_1.addon_profiles[actual_key].config["logAnalyticsWorkspaceResourceID"], + mc_1.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id, "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/test-workspace", ) + # No duplicate omsagent key was created + self.assertEqual( + len([k for k in mc_1.addon_profiles if k.lower() == "omsagent"]), 1 + ) def test_setup_azure_monitor_logs_with_omsagent_lowercase(self): # Test that _setup_azure_monitor_logs handles existing omsagent (lowercase) correctly @@ -15308,9 +15301,14 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Enabled", + ), + ), ) self.assertEqual(dec_mc_1, ground_truth_mc_1) # Verify HLSM is auto-enabled when CNL is enabled @@ -15359,9 +15357,14 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "False"} + enabled=True, config={"enableRetinaNetworkFlags": "True"} ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Disabled", + ), + ), ) self.assertEqual(dec_mc_2, ground_truth_mc_2) @@ -15443,6 +15446,11 @@ def test_enable_container_network_logs(self): self.assertEqual( dec_mc_4.addon_profiles["omsagent"], ground_truth_mc_4["omsagent"] ) + # Container network logs are also recorded on the containerInsights surface. + self.assertEqual( + dec_mc_4.azure_monitor_profile.container_insights.container_network_logs, + "Enabled", + ) # Case 5: enable_acns and enable_retina_network_flow_logs without monitoring addon dec_5 = AKSPreviewManagedClusterCreateDecorator( @@ -15567,6 +15575,11 @@ def test_enable_container_network_logs(self): enabled=True, config={"enableRetinaNetworkFlags": "True"} ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Enabled", + ), + ), ) self.assertEqual(dec_mc_7, ground_truth_mc_7) # Verify HLSM is auto-enabled when using deprecated flag @@ -15692,19 +15705,17 @@ def test_enable_container_network_logs(self): return_value=None, ): dec_mc_11 = dec_11.set_up_addon_profiles(mc_11) - ground_truth_mc_11 = { - CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( - enabled=True, - config={ - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: "/test_workspace_resource_id", - CONST_MONITORING_USING_AAD_MSI_AUTH: "true", - "enableRetinaNetworkFlags": "True", - }, - ), - } + # Container Insights is configured on the azureMonitorProfile surface (no legacy omsagent), + # including the container network logs setting. + self.assertNotIn("omsagent", dec_mc_11.addon_profiles or {}) + container_insights_11 = dec_mc_11.azure_monitor_profile.container_insights + self.assertIsNotNone(container_insights_11) + self.assertTrue(container_insights_11.enabled) self.assertEqual( - dec_mc_11.addon_profiles["omsagent"], ground_truth_mc_11["omsagent"] + container_insights_11.log_analytics_workspace_resource_id, + "/test_workspace_resource_id", ) + self.assertEqual(container_insights_11.container_network_logs, "Enabled") # Case 12: Verify monitoring_addon_postprocessing_required is set when CNL is enabled (update path) # This test verifies the fix for the bug where DCR is not updated when enabling CNL on update @@ -15756,9 +15767,14 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Enabled", + ), + ), ) self.assertEqual(dec_mc_12, ground_truth_mc_12) @@ -15812,9 +15828,14 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "False"} + enabled=True, config={"enableRetinaNetworkFlags": "True"} ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Disabled", + ), + ), ) self.assertEqual(dec_mc_13, ground_truth_mc_13) @@ -15862,10 +15883,15 @@ def test_enable_container_network_logs(self): ), ), addon_profiles={ - "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "False"} + "omsAgent": self.models.ManagedClusterAddonProfile( + enabled=True, config={"enableRetinaNetworkFlags": "True"} ) }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + container_network_logs="Disabled", + ), + ), ) self.assertEqual(dec_mc_13b, ground_truth_mc_13b) @@ -16269,8 +16295,8 @@ def test_update_enable_cnl_with_azure_monitor_logs_on_cluster(self): dec.context.attach_mc(mc) dec_mc = dec.update_monitoring_profile_flow_logs(mc) self.assertEqual( - dec_mc.addon_profiles["omsagent"].config["enableRetinaNetworkFlags"], - "True", + dec_mc.azure_monitor_profile.container_insights.container_network_logs, + "Enabled", ) self.assertTrue( dec.context.get_intermediate("monitoring_addon_postprocessing_required") @@ -16306,8 +16332,8 @@ def test_update_cnl_explicit_true_hlsm_with_prerequisites(self): dec.context.attach_mc(mc) dec_mc = dec.update_monitoring_profile_flow_logs(mc) self.assertEqual( - dec_mc.addon_profiles["omsagent"].config["enableRetinaNetworkFlags"], - "True", + dec_mc.azure_monitor_profile.container_insights.container_network_logs, + "Enabled", ) self.assertTrue(dec.context.get_enable_high_log_scale_mode()) self.assertTrue( @@ -18114,8 +18140,10 @@ def test_setup_azure_monitor_logs_sets_retina_flags_when_cnl_enabled(self): dec._setup_azure_monitor_logs(mc) addon_profile = mc.addon_profiles.get(CONST_MONITORING_ADDON_NAME) - self.assertIsNotNone(addon_profile) - self.assertEqual(addon_profile.config.get("enableRetinaNetworkFlags"), "True") + self.assertIsNone(addon_profile) + self.assertIsNotNone(mc.azure_monitor_profile.container_insights) + self.assertEqual( + mc.azure_monitor_profile.container_insights.container_network_logs, "Enabled") def test_setup_azure_monitor_logs_no_retina_flags_without_cnl(self): """_setup_azure_monitor_logs does NOT set enableRetinaNetworkFlags when CNL is not specified.""" @@ -18142,8 +18170,9 @@ def test_setup_azure_monitor_logs_no_retina_flags_without_cnl(self): dec._setup_azure_monitor_logs(mc) addon_profile = mc.addon_profiles.get(CONST_MONITORING_ADDON_NAME) - self.assertIsNotNone(addon_profile) - self.assertNotIn("enableRetinaNetworkFlags", addon_profile.config) + self.assertIsNone(addon_profile) + self.assertIsNotNone(mc.azure_monitor_profile.container_insights) + self.assertIsNone(mc.azure_monitor_profile.container_insights.container_network_logs) # ------------------------------------------------------------------ # Tests for _setup_azure_monitor_logs workspace change detection @@ -18188,10 +18217,10 @@ def test_setup_azure_monitor_logs_workspace_change_triggers_postprocessing(self) "monitoring_addon_postprocessing_required", default_value=False ) ) - # Verify workspace was updated - actual_key = next(k for k in mc.addon_profiles if k.lower() == "omsagent") + # Verify workspace was updated on the containerInsights surface + self.assertIsNotNone(mc.azure_monitor_profile.container_insights) self.assertEqual( - mc.addon_profiles[actual_key].config["logAnalyticsWorkspaceResourceID"], + mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id, new_ws, ) @@ -18583,12 +18612,16 @@ def test_reenable_monitoring_after_disable_does_not_carry_cnl(self): dec_enable.context.set_intermediate("subscription_id", "test-subscription-id") dec_enable._setup_azure_monitor_logs(mc) - # Config should only have workspace + MSI auth — no CNL or HLSM keys - addon_config = mc.addon_profiles[CONST_MONITORING_ADDON_NAME].config - self.assertTrue(mc.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) - self.assertIn("logAnalyticsWorkspaceResourceID", addon_config) - self.assertIn(CONST_MONITORING_USING_AAD_MSI_AUTH, addon_config) - self.assertNotIn("enableRetinaNetworkFlags", addon_config) + # Container Insights is re-enabled on the azureMonitorProfile surface with a fresh + # workspace and no CNL carried forward. The legacy omsagent addon is left untouched. + container_insights = mc.azure_monitor_profile.container_insights + self.assertIsNotNone(container_insights) + self.assertTrue(container_insights.enabled) + self.assertEqual( + container_insights.log_analytics_workspace_resource_id, + "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/new-workspace", + ) + self.assertIsNone(container_insights.container_network_logs) if __name__ == "__main__": diff --git a/src/aks-preview/setup.py b/src/aks-preview/setup.py index d197125dfa7..507acbff9e0 100644 --- a/src/aks-preview/setup.py +++ b/src/aks-preview/setup.py @@ -9,7 +9,7 @@ from setuptools import find_packages, setup -VERSION = "21.0.0b13" +VERSION = "21.0.0b14" CLASSIFIERS = [ "Development Status :: 4 - Beta", From 3d895b9dc5f579deea42837bdb33a4d349cbd330 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Wed, 22 Jul 2026 15:49:04 +0100 Subject: [PATCH 2/9] aks-preview: ensure DCR/DCRA on every --enable-azure-monitor-logs update (fix no-data on first-time enable) --- .../managed_cluster_decorator.py | 26 +++++-------------- .../latest/test_managed_cluster_decorator.py | 22 +++++++++------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index b2104895132..5c51e86f045 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -8508,10 +8508,6 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: ``addonProfiles.omsagent`` addon. The AKS RP normalizes containerInsights into omsagent on the PUT (defaulting ``useAADAuth=true``), so MSI/AAD auth is implied on this path. """ - addon_consts = self.context.get_addon_consts() - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( - "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") - # Get or create workspace resource ID workspace_resource_id = self.context.raw_param.get("workspace_resource_id") if not workspace_resource_id: @@ -8527,21 +8523,13 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: sanitize_func = self.context.external_functions.sanitize_loganalytics_ws_resource_id workspace_resource_id = sanitize_func(workspace_resource_id) - # Detect workspace change from the existing surfaces (containerInsights preferred, omsagent - # addon fallback for brownfield clusters) so DCR postprocessing updates the DCR destination. - old_workspace = "" - if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and - mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id): - old_workspace = mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id - elif mc.addon_profiles: - existing_key = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - existing_profile = mc.addon_profiles.get(existing_key) if existing_key else None - if existing_profile and existing_profile.config: - old_workspace = existing_profile.config.get( - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, "") or "" - if old_workspace and old_workspace.lower() != workspace_resource_id.lower(): - self.context.set_intermediate( - "monitoring_addon_postprocessing_required", True, overwrite_exists=True) + # Ensure the DCR/DCRA is (re)created in postprocessing whenever Container Insights is + # enabled on the update path. Previously only a workspace *change* triggered this, so a + # first-time `az aks update --enable-azure-monitor-logs` left the agent in MSI mode with no + # DCR to collect against -> no data flowed. ensure_container_insights_for_monitoring is + # idempotent, so re-ensuring on every enable is safe and self-heals clusters missing a DCR. + self.context.set_intermediate( + "monitoring_addon_postprocessing_required", True, overwrite_exists=True) self._ensure_azure_monitor_profile(mc) container_insights = mc.azure_monitor_profile.container_insights diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index 161374d03a9..a9c14b2159b 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -18224,8 +18224,9 @@ def test_setup_azure_monitor_logs_workspace_change_triggers_postprocessing(self) new_ws, ) - def test_setup_azure_monitor_logs_same_workspace_no_postprocessing(self): - """_setup_azure_monitor_logs does NOT set monitoring_addon_postprocessing_required when workspace is unchanged.""" + def test_setup_azure_monitor_logs_same_workspace_requires_postprocessing(self): + """_setup_azure_monitor_logs always requires DCR postprocessing on enable (idempotent ensure), + even when the workspace is unchanged, so the DCR/DCRA is (re)created.""" ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/same-ws" dec = AKSPreviewManagedClusterUpdateDecorator( self.cmd, @@ -18258,14 +18259,14 @@ def test_setup_azure_monitor_logs_same_workspace_no_postprocessing(self): ): dec._setup_azure_monitor_logs(mc) - self.assertFalse( + self.assertTrue( dec.context.get_intermediate( "monitoring_addon_postprocessing_required", default_value=False ) ) def test_setup_azure_monitor_logs_workspace_change_case_insensitive(self): - """_setup_azure_monitor_logs compares workspaces case-insensitively (no false positives on casing).""" + """_setup_azure_monitor_logs always requires DCR postprocessing on enable (idempotent ensure).""" ws_lower = "/subscriptions/test/resourcegroups/test/providers/microsoft.operationalinsights/workspaces/my-ws" ws_mixed = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/my-ws" dec = AKSPreviewManagedClusterUpdateDecorator( @@ -18299,15 +18300,16 @@ def test_setup_azure_monitor_logs_workspace_change_case_insensitive(self): ): dec._setup_azure_monitor_logs(mc) - # Same workspace (different casing) should NOT trigger postprocessing - self.assertFalse( + # DCR postprocessing is always required on enable so the DCR/DCRA is ensured + self.assertTrue( dec.context.get_intermediate( "monitoring_addon_postprocessing_required", default_value=False ) ) - def test_setup_azure_monitor_logs_new_addon_no_postprocessing(self): - """_setup_azure_monitor_logs does NOT trigger postprocessing when there is no existing addon (fresh enable).""" + def test_setup_azure_monitor_logs_new_addon_requires_postprocessing(self): + """_setup_azure_monitor_logs triggers DCR postprocessing on a first-time enable (no existing + addon), so the DCR/DCRA is created and data flows (previously this was skipped -> no data).""" new_ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/new-ws" dec = AKSPreviewManagedClusterUpdateDecorator( self.cmd, @@ -18332,8 +18334,8 @@ def test_setup_azure_monitor_logs_new_addon_no_postprocessing(self): ): dec._setup_azure_monitor_logs(mc) - # Fresh enable — no old workspace to compare, should NOT trigger postprocessing - self.assertFalse( + # First-time enable must trigger DCR/DCRA creation in postprocessing + self.assertTrue( dec.context.get_intermediate( "monitoring_addon_postprocessing_required", default_value=False ) From aa8795db635bca30414498663f8804ed67944217 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Wed, 22 Jul 2026 16:48:42 +0100 Subject: [PATCH 3/9] aks-preview: address rubber-duck review - clear CNL on disable, HLSM guard reads containerInsights, defensive update DCR postprocessing --- .../managed_cluster_decorator.py | 213 ++++++++++-------- .../latest/test_managed_cluster_decorator.py | 53 +++++ 2 files changed, 173 insertions(+), 93 deletions(-) diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 5c51e86f045..e41f10d4b2b 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -192,6 +192,73 @@ def _get_monitoring_addon_key_from_consts(addon_profiles, addon_consts): ) +def _is_container_network_logs_enabled_on_mc(mc, addon_consts): + """Return True if container network logs are already enabled on the cluster. + + Checks the modern ``azureMonitorProfile.containerInsights.containerNetworkLogs`` surface first + (where the CLI now writes CNL), falling back to the legacy + ``addonProfiles.omsagent.config.enableRetinaNetworkFlags`` for brownfield clusters. + """ + if mc is None: + return False + if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and + mc.azure_monitor_profile.container_insights.container_network_logs is not None): + cnl = mc.azure_monitor_profile.container_insights.container_network_logs + # cnl may be a plain string ("Enabled"/"Disabled") or the ContainerNetworkLogs enum; + # normalize via the enum's .value so str-enum repr does not break the comparison. + cnl = getattr(cnl, "value", cnl) + return str(cnl).lower() == "enabled" + if mc.addon_profiles: + mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) + monitoring_profile = mc.addon_profiles.get(mk) + if monitoring_profile and monitoring_profile.config: + return str( + monitoring_profile.config.get("enableRetinaNetworkFlags", "") + ).lower() == "true" + return False + + +def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): + """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes. + + Container Insights is now configured via ``azureMonitorProfile.containerInsights`` and the AKS RP + normalizes it into ``addonProfiles.omsagent`` on the PUT response. When that mirrored addon is + present (and has config) we use it directly. Otherwise (e.g. mocked responses in unit tests, or a + hypothetical response without the mirror) we build a transient omsagent-style profile from + ``containerInsights`` so the DCR engine still gets the workspace id and MSI/AAD auth signal. The + returned object is only passed to the engine, never persisted on the cluster. + """ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( + "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") + CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") + + monitoring_addon_key = ( + _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) + if cluster.addon_profiles + else None + ) + if (monitoring_addon_key and cluster.addon_profiles and + monitoring_addon_key in cluster.addon_profiles and + cluster.addon_profiles[monitoring_addon_key].config): + return cluster.addon_profiles[monitoring_addon_key] + + workspace_resource_id = None + container_insights = ( + cluster.azure_monitor_profile.container_insights + if cluster.azure_monitor_profile + else None + ) + if container_insights is not None: + workspace_resource_id = container_insights.log_analytics_workspace_resource_id + return models.ManagedClusterAddonProfile( + enabled=True, + config={ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id, + CONST_MONITORING_USING_AAD_MSI_AUTH: "true", + }, + ) + + # pylint: disable=too-few-public-methods class AKSPreviewManagedClusterModels(AKSManagedClusterModels): """Store the models used in aks series of commands. @@ -3255,15 +3322,8 @@ def get_enable_high_log_scale_mode(self) -> Union[bool, None]: # If user explicitly disables HLSM, check if CNL is already enabled on the cluster if enable_high_log_scale_mode is False: - cnl_already_enabled = False - if self.mc and self.mc.addon_profiles: - addon_consts = self.get_addon_consts() - mk = _get_monitoring_addon_key_from_consts(self.mc.addon_profiles, addon_consts) - monitoring_profile = self.mc.addon_profiles.get(mk) - if monitoring_profile and monitoring_profile.config: - cnl_already_enabled = str( - monitoring_profile.config.get("enableRetinaNetworkFlags", "") - ).lower() == "true" + cnl_already_enabled = _is_container_network_logs_enabled_on_mc( + self.mc, self.get_addon_consts()) if cnl_already_enabled: raise MutuallyExclusiveArgumentError( "Cannot explicitly disable --enable-high-log-scale-mode while " @@ -6160,44 +6220,9 @@ def _is_cnl_or_hlsm_changing(self) -> bool: ) def _get_monitoring_addon_profile_for_dcr(self, cluster: ManagedCluster): - """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes. - - Container Insights is now configured via ``azureMonitorProfile.containerInsights`` and the - AKS RP normalizes it into ``addonProfiles.omsagent`` on the PUT response. When that mirrored - addon is present we use it directly. Otherwise (e.g. mocked responses in unit tests) we build - a transient omsagent-style profile from ``containerInsights`` so the DCR engine still gets the - workspace id and MSI/AAD auth signal. This object is only passed to the engine, never persisted. - """ - addon_consts = self.context.get_addon_consts() - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( - "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - - monitoring_addon_key = ( - _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) - if cluster.addon_profiles - else None - ) - if (monitoring_addon_key and cluster.addon_profiles and - monitoring_addon_key in cluster.addon_profiles and - cluster.addon_profiles[monitoring_addon_key].config): - return cluster.addon_profiles[monitoring_addon_key] - - workspace_resource_id = None - container_insights = ( - cluster.azure_monitor_profile.container_insights - if cluster.azure_monitor_profile - else None - ) - if container_insights is not None: - workspace_resource_id = container_insights.log_analytics_workspace_resource_id - return self.models.ManagedClusterAddonProfile( - enabled=True, - config={ - CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_resource_id, - CONST_MONITORING_USING_AAD_MSI_AUTH: "true", - }, - ) + """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes.""" + return _build_monitoring_addon_profile_for_dcr( + self.context.get_addon_consts(), self.models, cluster) class AKSPreviewManagedClusterUpdateDecorator(AKSManagedClusterUpdateDecorator): @@ -6549,15 +6574,8 @@ def update_monitoring_profile_flow_logs(self, mc: ManagedCluster) -> ManagedClus elif enable_high_log_scale_mode is False: # Check if CNL is already enabled on the cluster — cannot disable HLSM while CNL is on - cnl_already_enabled = False - if mc.addon_profiles: - addon_consts = self.context.get_addon_consts() - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_profile = mc.addon_profiles.get(mk) - if monitoring_profile and monitoring_profile.config: - cnl_already_enabled = str( - monitoring_profile.config.get("enableRetinaNetworkFlags", "") - ).lower() == "true" + cnl_already_enabled = _is_container_network_logs_enabled_on_mc( + mc, self.context.get_addon_consts()) if cnl_already_enabled: raise MutuallyExclusiveArgumentError( "Cannot explicitly disable --enable-high-log-scale-mode while " @@ -8652,6 +8670,9 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights): mc.azure_monitor_profile.container_insights.enabled = False + # Clear container network logs so a later re-enable (without a CNL flag) does not + # silently resurrect CNL from stale state. + mc.azure_monitor_profile.container_insights.container_network_logs = None # Also disable OpenTelemetry logs when disabling Azure Monitor logs if opentelemetry_logs_enabled: @@ -8894,49 +8915,55 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: addon_consts = self.context.get_addon_consts() CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - monitoring_addon_key = ( - _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) - if cluster.addon_profiles - else addon_consts.get("CONST_MONITORING_ADDON_NAME") + # Resolve the omsagent-style profile the DCR engine consumes. Prefer the RP-mirrored + # omsagent from the PUT response; fall back to a transient profile synthesized from + # containerInsights so a missing/unmirrored addon does not silently skip DCR creation + # (or crash on a None config). + monitoring_addon_profile = _build_monitoring_addon_profile_for_dcr( + addon_consts, self.models, cluster) + + # Container Insights is considered enabled if either surface reports it. + container_insights_enabled = bool( + cluster.azure_monitor_profile and + cluster.azure_monitor_profile.container_insights and + cluster.azure_monitor_profile.container_insights.enabled ) + monitoring_enabled = bool(monitoring_addon_profile.enabled) or container_insights_enabled - if (cluster.addon_profiles and - monitoring_addon_key in cluster.addon_profiles and - cluster.addon_profiles[monitoring_addon_key].enabled): - - # Check if MSI auth is enabled - if (CONST_MONITORING_USING_AAD_MSI_AUTH in - cluster.addon_profiles[monitoring_addon_key].config and - str(cluster.addon_profiles[monitoring_addon_key].config[ - CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true"): + addon_config = monitoring_addon_profile.config or {} + msi_auth_enabled = ( + CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config and + str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" + ) - # Check parameter sizes to identify what might be causing large headers - data_collection_settings = self.context.get_data_collection_settings() + if monitoring_enabled and msi_auth_enabled: + # Check parameter sizes to identify what might be causing large headers + data_collection_settings = self.context.get_data_collection_settings() - # Try to limit data_collection_settings size to avoid "Request Header Fields Too Large" error + # Try to limit data_collection_settings size to avoid "Request Header Fields Too Large" error + safe_data_collection_settings = None + if data_collection_settings and len(str(data_collection_settings)) > 10000: safe_data_collection_settings = None - if data_collection_settings and len(str(data_collection_settings)) > 10000: - safe_data_collection_settings = None - else: - safe_data_collection_settings = data_collection_settings + else: + safe_data_collection_settings = data_collection_settings - self.context.external_functions.ensure_container_insights_for_monitoring( - self.cmd, - cluster.addon_profiles[monitoring_addon_key], - self.context.get_subscription_id(), - self.context.get_resource_group_name(), - self.context.get_name(), - self.context.get_location(), - remove_monitoring=False, - aad_route=self.context.get_enable_msi_auth_for_monitoring(), - create_dcr=True, - create_dcra=True, - enable_syslog=self.context.get_enable_syslog(), - data_collection_settings=safe_data_collection_settings, - is_private_cluster=self.context.get_enable_private_cluster(), - ampls_resource_id=self.context.get_ampls_resource_id(), - enable_high_log_scale_mode=self.context.get_enable_high_log_scale_mode(), - ) + self.context.external_functions.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(), + remove_monitoring=False, + aad_route=self.context.get_enable_msi_auth_for_monitoring(), + create_dcr=True, + create_dcra=True, + enable_syslog=self.context.get_enable_syslog(), + data_collection_settings=safe_data_collection_settings, + is_private_cluster=self.context.get_enable_private_cluster(), + ampls_resource_id=self.context.get_ampls_resource_id(), + enable_high_log_scale_mode=self.context.get_enable_high_log_scale_mode(), + ) # Monitoring addon disable cleanup is now done upfront in _disable_azure_monitor_logs (not in postprocessing) # This matches the pattern from aks_disable_addons lines 2796-2822 where cleanup happens BEFORE the PUT diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index a9c14b2159b..bdc27e44171 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -18419,6 +18419,59 @@ def test_disable_azure_monitor_logs_no_container_insights_skips(self): dec._disable_azure_monitor_logs(mc) self.assertFalse(mc.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) + def test_disable_azure_monitor_logs_clears_container_network_logs(self): + """Disabling monitoring clears containerInsights.container_network_logs so a later + re-enable (without a CNL flag) does not silently resurrect CNL (rubber-duck finding #2).""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "disable_azure_monitor_logs": True, + "yes": True, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + log_analytics_workspace_resource_id="/subscriptions/test/workspace", + container_network_logs="Enabled", + ), + ), + ) + dec.context.attach_mc(mc) + dec.client = Mock() + dec.client.get = Mock(return_value=mc) + dec._disable_azure_monitor_logs(mc) + self.assertFalse(mc.azure_monitor_profile.container_insights.enabled) + self.assertIsNone(mc.azure_monitor_profile.container_insights.container_network_logs) + + def test_hlsm_disable_blocked_when_cnl_enabled_on_containerinsights(self): + """--enable-high-log-scale-mode false is rejected when CNL is enabled on the modern + containerInsights surface (not just the legacy omsagent field) (rubber-duck finding #3).""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_high_log_scale_mode": False, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + container_network_logs="Enabled", + ), + ), + ) + dec.context.attach_mc(mc) + with self.assertRaises(MutuallyExclusiveArgumentError): + dec.update_monitoring_profile_flow_logs(mc) + # ------------------------------------------------------------------ # Tests for update_monitoring_profile_flow_logs: monitoring_being_enabled bypass # ------------------------------------------------------------------ From 4738f4b777e93b81a43fcbd311be2fb0e42e17ce Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Sat, 25 Jul 2026 00:56:00 +0100 Subject: [PATCH 4/9] aks-preview: honor containerInsights surface in CNL and HLSM enablement checks --- .../managed_cluster_decorator.py | 73 +++++++++++++------ .../latest/test_managed_cluster_decorator.py | 60 +++++++++++++++ 2 files changed, 109 insertions(+), 24 deletions(-) diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index e41f10d4b2b..7fd884f74cf 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -218,6 +218,48 @@ def _is_container_network_logs_enabled_on_mc(mc, addon_consts): return False +def _is_monitoring_enabled_on_mc(mc, addon_consts): + """Return True if Container Insights is enabled on the cluster. + + Checks the modern ``azureMonitorProfile.containerInsights`` surface (the RP source of truth) + first, falling back to the legacy ``addonProfiles.omsagent`` addon for brownfield clusters. + """ + if mc is None: + return False + if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and + mc.azure_monitor_profile.container_insights.enabled): + return True + if mc.addon_profiles: + mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) + monitoring_profile = mc.addon_profiles.get(mk) + return bool(monitoring_profile and monitoring_profile.enabled) + return False + + +def _is_monitoring_msi_auth_on_mc(mc, addon_consts): + """Return True if Container Insights uses MSI/AAD auth on the cluster. + + The modern ``containerInsights`` surface is always MSI/AAD (the RP defaults ``useAADAuth=true`` + when it mirrors the profile), so treat an enabled containerInsights as MSI. Otherwise fall back + to the legacy ``omsagent.config.useAADAuth`` flag. + """ + if mc is None: + return False + if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and + mc.azure_monitor_profile.container_insights.enabled): + return True + CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") + if mc.addon_profiles: + mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) + monitoring_profile = mc.addon_profiles.get(mk) + addon_config = (monitoring_profile.config or {}) if monitoring_profile else {} + return ( + CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config and + str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" + ) + return False + + def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes. @@ -1198,13 +1240,7 @@ def get_container_network_logs(self, mc: ManagedCluster) -> Union[bool, None]: "monitoring" in enable_addons or bool(self.raw_param.get("enable_azure_monitor_logs")) ) - monitoring_already_enabled = False - if mc.addon_profiles: - addon_consts = self.get_addon_consts() - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_already_enabled = bool( - mc.addon_profiles.get(mk) and mc.addon_profiles[mk].enabled - ) + monitoring_already_enabled = _is_monitoring_enabled_on_mc(mc, self.get_addon_consts()) monitoring_enabled = monitoring_being_enabled or monitoring_already_enabled if not acns_enabled or not monitoring_enabled: raise InvalidArgumentValueError( @@ -6542,29 +6578,18 @@ def update_monitoring_profile_flow_logs(self, mc: ManagedCluster) -> ManagedClus ) if not monitoring_being_enabled: - # Only validate existing addon state when not enabling monitoring simultaneously + # Only validate existing cluster state when not enabling monitoring simultaneously. + # Check the modern containerInsights surface first, then the legacy omsagent addon. addon_consts = self.context.get_addon_consts() - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - - # Resolve the addon profile, normalizing non-standard key casing. - monitoring_addon_profile = None - if mc.addon_profiles: - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_addon_profile = mc.addon_profiles.get(mk) - if not monitoring_addon_profile or not monitoring_addon_profile.enabled: + if not _is_monitoring_enabled_on_mc(mc, addon_consts): raise RequiredArgumentMissingError( - "--enable-high-log-scale-mode requires the Azure Monitor logs addon (omsagent) " + "--enable-high-log-scale-mode requires Azure Monitor logs (Container Insights) " "to be enabled on the cluster. Please enable it first with " - "--enable-addons monitoring or --enable-azure-monitor-logs." + "--enable-azure-monitor-logs." ) - addon_config = monitoring_addon_profile.config or {} - msi_auth_enabled = ( - CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config and - str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" - ) - if not msi_auth_enabled: + if not _is_monitoring_msi_auth_on_mc(mc, addon_consts): raise RequiredArgumentMissingError( "--enable-high-log-scale-mode requires MSI authentication to be enabled " "for the monitoring addon. Please enable it with --enable-msi-auth-for-monitoring." diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index bdc27e44171..511d39c02d3 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -5752,6 +5752,37 @@ def test_get_container_network_logs_with_azure_monitor_logs(self): result = ctx.get_container_network_logs(mc) self.assertTrue(result) + def test_get_container_network_logs_with_containerinsights_only_cluster(self): + """CNL enable succeeds when monitoring is already on via azureMonitorProfile.containerInsights + only (no legacy omsagent addon) - the modern source-of-truth surface must be honored.""" + ctx = AKSPreviewManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict( + { + "enable_container_network_logs": True, + "enable_acns": True, + } + ), + self.models, + decorator_mode=DecoratorMode.UPDATE, + ) + mc = self.models.ManagedCluster( + location="test_location", + network_profile=self.models.ContainerServiceNetworkProfile( + advanced_networking=self.models.AdvancedNetworking( + enabled=True, + ), + ), + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + ), + ), + ) + ctx.attach_mc(mc) + result = ctx.get_container_network_logs(mc) + self.assertTrue(result) + def test_get_container_network_logs_legacy_disable_retina_flow_logs(self): """Test get_container_network_logs returns False when legacy disable_retina_flow_logs is specified.""" ctx = AKSPreviewManagedClusterContext( @@ -18448,6 +18479,35 @@ def test_disable_azure_monitor_logs_clears_container_network_logs(self): self.assertFalse(mc.azure_monitor_profile.container_insights.enabled) self.assertIsNone(mc.azure_monitor_profile.container_insights.container_network_logs) + def test_hlsm_standalone_allowed_on_containerinsights_only_cluster(self): + """--enable-high-log-scale-mode is accepted when monitoring is on via containerInsights only + (no legacy omsagent addon). MSI is implied on that surface, so neither the 'monitoring must be + enabled' nor the 'MSI required' guard should reject it.""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_high_log_scale_mode": True, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + log_analytics_workspace_resource_id="/subscriptions/test/workspace", + ), + ), + ) + dec.context.attach_mc(mc) + dec.update_monitoring_profile_flow_logs(mc) + self.assertTrue( + dec.context.get_intermediate( + "monitoring_addon_postprocessing_required", default_value=False + ) + ) + def test_hlsm_disable_blocked_when_cnl_enabled_on_containerinsights(self): """--enable-high-log-scale-mode false is rejected when CNL is enabled on the modern containerInsights surface (not just the legacy omsagent field) (rubber-duck finding #3).""" From c66e19bfe49fa6372ad25704a93e7cc7e3515280 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Mon, 27 Jul 2026 16:03:37 +0100 Subject: [PATCH 5/9] aks-preview: complete containerInsights-only CNL state handling --- .../azext_aks_preview/_validators.py | 8 ++-- .../managed_cluster_decorator.py | 46 ++++++++++--------- .../latest/test_managed_cluster_decorator.py | 38 +++++++++++---- .../tests/latest/test_validators.py | 7 +++ 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/aks-preview/azext_aks_preview/_validators.py b/src/aks-preview/azext_aks_preview/_validators.py index 968c022dd1b..547f404573c 100644 --- a/src/aks-preview/azext_aks_preview/_validators.py +++ b/src/aks-preview/azext_aks_preview/_validators.py @@ -616,10 +616,12 @@ def validate_addon(namespace): def validate_addons(namespace): - if not hasattr(namespace, 'addons'): + addons = getattr(namespace, 'addons', None) + if addons is None: + addons = getattr(namespace, 'enable_addons', None) + if not addons: return - addons = namespace.addons - addon_args = addons.split(',') + addon_args = addons.split(',') if isinstance(addons, str) else list(addons) _recognize_addons(addon_args) if 'monitoring' in addon_args: _warn_monitoring_addon_deprecated() diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 7fd884f74cf..37798a6ec31 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -218,6 +218,23 @@ def _is_container_network_logs_enabled_on_mc(mc, addon_consts): return False +def _clear_legacy_container_network_logs_config(mc, addon_consts): + """Remove the legacy CNL config when the modern containerInsights field is explicitly written.""" + if not mc or not mc.addon_profiles: + return + monitoring_addon_name = addon_consts.get("CONST_MONITORING_ADDON_NAME") + monitoring_key = next( + (key for key in mc.addon_profiles if key.lower() == monitoring_addon_name.lower()), + None, + ) + monitoring_profile = mc.addon_profiles.get(monitoring_key) if monitoring_key else None + if not monitoring_profile or not monitoring_profile.config: + return + for key in list(monitoring_profile.config): + if key.lower() == "enableretinanetworkflags": + monitoring_profile.config.pop(key) + + def _is_monitoring_enabled_on_mc(mc, addon_consts): """Return True if Container Insights is enabled on the cluster. @@ -561,21 +578,9 @@ def get_enable_msi_auth_for_monitoring(self) -> Union[bool, None]: result = True elif enable_msi_auth_for_monitoring is False: # The base class returns False when service_principal_profile.client_id is not None, - # but MSI-based clusters set client_id to "msi". Check if the monitoring addon - # already has useAADAuth=true, which indicates MSI auth is actually in use. - addon_consts = self.get_addon_consts() - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - if self.mc and self.mc.addon_profiles: - monitoring_addon_key = _get_monitoring_addon_key_from_consts( - self.mc.addon_profiles, addon_consts) - monitoring_profile = self.mc.addon_profiles.get(monitoring_addon_key) - result = bool( - monitoring_profile and monitoring_profile.config and - str(monitoring_profile.config.get( - CONST_MONITORING_USING_AAD_MSI_AUTH, "")).lower() == "true" - ) - else: - result = False + # but MSI-based clusters set client_id to "msi". Check the modern containerInsights + # profile first, then the legacy addon useAADAuth flag. + result = _is_monitoring_msi_auth_on_mc(self.mc, self.get_addon_consts()) elif enable_msi_auth_for_monitoring is None and not disable_msi_auth and not enable_msi_auth: result = True else: @@ -3340,12 +3345,8 @@ def get_enable_high_log_scale_mode(self) -> Union[bool, None]: # Check if enabling Azure Monitor logs enable_azure_monitor_logs = self.raw_param.get("enable_azure_monitor_logs") - # Check if monitoring addon is already enabled in the cluster - monitoring_addon_enabled = False - if self.mc and self.mc.addon_profiles: - mk = _get_monitoring_addon_key_from_consts(self.mc.addon_profiles, addon_consts) - if mk in self.mc.addon_profiles: - monitoring_addon_enabled = self.mc.addon_profiles[mk].enabled + # Check the modern containerInsights surface first, with legacy addon fallback. + monitoring_addon_enabled = _is_monitoring_enabled_on_mc(self.mc, addon_consts) if not monitoring_being_enabled and not enable_azure_monitor_logs and not monitoring_addon_enabled: raise RequiredArgumentMissingError( @@ -4698,6 +4699,7 @@ def set_up_addon_profiles(self, mc: ManagedCluster) -> ManagedCluster: mc.azure_monitor_profile.container_insights.container_network_logs = ( "Enabled" if container_network_logs_enabled else "Disabled" ) + _clear_legacy_container_network_logs_config(mc, addon_consts) # Trigger validation for high log scale mode when container network logs are enabled. # This ensures proper error messages are raised before cluster creation if the user @@ -6558,6 +6560,8 @@ def update_monitoring_profile_flow_logs(self, mc: ManagedCluster) -> ManagedClus mc.azure_monitor_profile.container_insights.container_network_logs = ( "Enabled" if container_network_logs_enabled else "Disabled" ) + _clear_legacy_container_network_logs_config( + mc, self.context.get_addon_consts()) # When enabling CNL, the DCR must be updated to add the high-scale stream. # Set the postprocessing intermediate so that the update path calls ensure_container_insights. diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index 511d39c02d3..e5be2836366 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -5989,7 +5989,7 @@ def test_get_enable_msi_auth_for_monitoring_with_msi_service_principal(self): """Test that MSI auth is correctly detected when service_principal_profile.client_id='msi'. The base class returns False when client_id is not None, but MSI-based clusters set - client_id to 'msi'. The preview override should check the addon config for useAADAuth. + client_id to 'msi'. The preview override should check the monitoring profile. """ ctx = AKSPreviewManagedClusterContext( self.cmd, @@ -6015,6 +6015,29 @@ def test_get_enable_msi_auth_for_monitoring_with_msi_service_principal(self): result = ctx.get_enable_msi_auth_for_monitoring() self.assertTrue(result) + def test_get_enable_msi_auth_for_monitoring_with_containerinsights_only(self): + """The modern containerInsights profile implies MSI even without an omsagent mirror.""" + ctx = AKSPreviewManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({}), + self.models, + decorator_mode=DecoratorMode.UPDATE, + ) + mc = self.models.ManagedCluster( + location="test_location", + service_principal_profile=self.models.ManagedClusterServicePrincipalProfile( + client_id="msi", + ), + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + ), + ), + ) + ctx.attach_mc(mc) + result = ctx.get_enable_msi_auth_for_monitoring() + self.assertTrue(result) + def test_get_enable_default_domain(self): # default value ctx_1 = AKSPreviewManagedClusterContext( @@ -15388,7 +15411,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( @@ -15470,7 +15493,6 @@ def test_enable_container_network_logs(self): config={ CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: "/test_workspace_resource_id", CONST_MONITORING_USING_AAD_MSI_AUTH: "true", - "enableRetinaNetworkFlags": "True", }, ), } @@ -15583,7 +15605,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, ) @@ -15603,7 +15625,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( @@ -15833,7 +15855,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, ) @@ -15859,7 +15881,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsagent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( @@ -15915,7 +15937,7 @@ def test_enable_container_network_logs(self): ), addon_profiles={ "omsAgent": self.models.ManagedClusterAddonProfile( - enabled=True, config={"enableRetinaNetworkFlags": "True"} + enabled=True, config={} ) }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py index 5ebffe2db4c..f4ca07a88d6 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py @@ -2413,6 +2413,13 @@ def test_enable_addons_monitoring_without_enable_azure_monitor_logs_succeeds(sel # Should not raise an exception validators.validate_azure_monitor_logs_and_enable_addons(namespace) + def test_validate_addons_warns_for_create_enable_addons_monitoring(self): + namespace = SimpleNamespace(enable_addons=["monitoring"]) + with patch.object(validators.logger, "warning") as warning: + validators.validate_addons(namespace) + warning.assert_called_once() + self.assertIn("deprecated", warning.call_args.args[0]) + class TestValidateAzureMonitorLogsEnableDisable(unittest.TestCase): def test_enable_and_disable_azure_monitor_logs_throws_error(self): From 5a7470085e05b989ae6ab9989b5b99352ce6df0b Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Mon, 27 Jul 2026 16:50:31 +0100 Subject: [PATCH 6/9] aks-preview: add Container Insights AMP controls and auth guidance --- src/aks-preview/HISTORY.rst | 4 +- src/aks-preview/azext_aks_preview/_help.py | 22 +- src/aks-preview/azext_aks_preview/_params.py | 46 +++++ .../azext_aks_preview/_validators.py | 84 ++++++-- src/aks-preview/azext_aks_preview/custom.py | 14 +- .../managed_cluster_decorator.py | 194 +++++++++++++----- .../latest/test_managed_cluster_decorator.py | 165 +++++++++++++++ .../tests/latest/test_validators.py | 69 ++++++- 8 files changed, 531 insertions(+), 67 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index bd8efd40e46..ff3ba58f71e 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -16,7 +16,9 @@ Pending 21.0.0b14 ++++++++ * `az aks create/update`: `--enable-azure-monitor-logs` / `--disable-azure-monitor-logs` now configure the modern ``azureMonitorProfile.containerInsights`` profile instead of the legacy ``addonProfiles.omsagent`` addon. The containerInsights path always uses managed-identity (MSI/AAD) auth. -* `az aks create/update`: Deprecate the ``monitoring`` addon (``--enable-addons monitoring`` / ``--disable-addons monitoring``) for Container Insights in favor of ``--enable-azure-monitor-logs`` / ``--disable-azure-monitor-logs``. +* `az aks create/update`: Add ``--syslog-port`` and ``--enable/--disable-prometheus-metrics-scraping`` controls for the Container Insights Azure Monitor profile. +* `az aks create/update`: Reject the legacy ``--enable-msi-auth-for-monitoring`` flag when used with ``--enable-azure-monitor-logs`` because the Azure Monitor profile is managed-identity only. +* `az aks enable-addons`: Show guidance toward ``--enable-azure-monitor-logs`` for the legacy ``monitoring`` addon and warn when ``--enable-msi-auth-for-monitoring`` is explicitly supplied. 21.0.0b13 ++++++++ diff --git a/src/aks-preview/azext_aks_preview/_help.py b/src/aks-preview/azext_aks_preview/_help.py index 78e5ee4ab08..be99f97ba99 100644 --- a/src/aks-preview/azext_aks_preview/_help.py +++ b/src/aks-preview/azext_aks_preview/_help.py @@ -194,6 +194,16 @@ type: bool short-summary: Enable Azure Monitor logs for the cluster. long-summary: Enables Container Insights logs for the cluster by configuring the azureMonitorProfile.containerInsights profile (managed-identity/MSI auth). This is the preferred way to onboard Container Insights and supersedes the deprecated "--enable-addons monitoring". Uses the Log Analytics Default Workspace if it exists, else creates one. Specify "--workspace-resource-id" to use an existing workspace. If monitoring is enabled --no-wait argument will have no effect + - name: --syslog-port + type: int + short-summary: Set the syslog host port for Azure Monitor Container Insights. + long-summary: Configures containerInsights.syslogPort. This controls the host port only and is distinct from --enable-syslog, which controls syslog collection in the DCR. If omitted, the service default is 28330. + - name: --enable-prometheus-metrics-scraping + type: bool + short-summary: Enable Prometheus metrics scraping for Azure Monitor Container Insights. + - name: --disable-prometheus-metrics-scraping + type: bool + short-summary: Disable Prometheus metrics scraping for Azure Monitor Container Insights. - name: --disable-rbac type: bool short-summary: Disable Kubernetes Role-Based Access Control. @@ -1120,10 +1130,20 @@ - name: --disable-azure-monitor-logs type: bool short-summary: Disable Azure Monitor logs for the cluster. - long-summary: This is equivalent to using "az aks disable-addons -a monitoring". Disables Log Analytics monitoring for the cluster. + long-summary: Disables Container Insights through azureMonitorProfile.containerInsights. The CLI also clears the legacy mirrored addon state when present for brownfield compatibility. - name: --workspace-resource-id type: string short-summary: The resource ID of an existing Log Analytics Workspace to use for storing monitoring data. If not specified, uses the default Log Analytics Workspace if it exists, otherwise creates one. + - name: --syslog-port + type: int + short-summary: Set the syslog host port for Azure Monitor Container Insights. + long-summary: Configures containerInsights.syslogPort. This controls the host port only and is distinct from --enable-syslog, which controls syslog collection in the DCR. If omitted, the service default is 28330. + - name: --enable-prometheus-metrics-scraping + type: bool + short-summary: Enable Prometheus metrics scraping for Azure Monitor Container Insights. + - name: --disable-prometheus-metrics-scraping + type: bool + short-summary: Disable Prometheus metrics scraping for Azure Monitor Container Insights. - name: --enable-msi-auth-for-monitoring type: bool short-summary: Send monitoring data to Log Analytics using the cluster's assigned identity (instead of the Log Analytics Workspace's shared key). diff --git a/src/aks-preview/azext_aks_preview/_params.py b/src/aks-preview/azext_aks_preview/_params.py index 98988fa1ea8..c1e2709e917 100644 --- a/src/aks-preview/azext_aks_preview/_params.py +++ b/src/aks-preview/azext_aks_preview/_params.py @@ -192,6 +192,8 @@ validate_azure_monitor_and_opentelemetry_for_create, validate_azure_monitor_and_opentelemetry_for_update, validate_azure_monitor_logs_and_enable_addons, + validate_azure_monitor_logs_amp_controls_for_create, + validate_azure_monitor_logs_amp_controls_for_update, validate_azure_monitor_logs_enable_disable, validate_azuremonitorworkspaceresourceid, validate_cluster_id, @@ -833,6 +835,28 @@ def load_arguments(self, _): "over the deprecated '--enable-addons monitoring'." ) c.argument("workspace_resource_id") + c.argument( + "syslog_port", + type=int, + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_create, + help="Syslog host port for Azure Monitor Container Insights. This configures the host " + "port only; use --enable-syslog to control syslog collection." + ) + c.argument( + "enable_prometheus_metrics_scraping", + action="store_true", + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_create, + help="Enable Prometheus metrics scraping for Azure Monitor Container Insights." + ) + c.argument( + "disable_prometheus_metrics_scraping", + action="store_true", + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_create, + help="Disable Prometheus metrics scraping for Azure Monitor Container Insights." + ) c.argument( "enable_msi_auth_for_monitoring", arg_type=get_three_state_flag(), @@ -1576,6 +1600,28 @@ def load_arguments(self, _): "azureMonitorProfile.containerInsights profile. Preferred over the deprecated " "'az aks disable-addons -a monitoring'." ) + c.argument( + "syslog_port", + type=int, + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_update, + help="Set the syslog host port for Azure Monitor Container Insights. This configures " + "the host port only; use --enable-syslog to control syslog collection." + ) + c.argument( + "enable_prometheus_metrics_scraping", + action="store_true", + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_update, + help="Enable Prometheus metrics scraping for Azure Monitor Container Insights." + ) + c.argument( + "disable_prometheus_metrics_scraping", + action="store_true", + is_preview=True, + validator=validate_azure_monitor_logs_amp_controls_for_update, + help="Disable Prometheus metrics scraping for Azure Monitor Container Insights." + ) c.argument("enable_secret_rotation", action="store_true") c.argument("disable_secret_rotation", action="store_true") c.argument("rotation_poll_interval") diff --git a/src/aks-preview/azext_aks_preview/_validators.py b/src/aks-preview/azext_aks_preview/_validators.py index 547f404573c..2cc94ec8a58 100644 --- a/src/aks-preview/azext_aks_preview/_validators.py +++ b/src/aks-preview/azext_aks_preview/_validators.py @@ -624,7 +624,9 @@ def validate_addons(namespace): addon_args = addons.split(',') if isinstance(addons, str) else list(addons) _recognize_addons(addon_args) if 'monitoring' in addon_args: - _warn_monitoring_addon_deprecated() + _show_monitoring_addon_legacy_info() + if getattr(namespace, 'enable_msi_auth_for_monitoring', None) is not None: + _warn_legacy_msi_auth_flag_deprecated() def validate_pod_identity_pod_labels(namespace): @@ -1205,12 +1207,20 @@ def validate_azure_monitor_and_opentelemetry_for_update(namespace): validate_opentelemetry_logs_dependencies_for_update(namespace) -def _warn_monitoring_addon_deprecated(): - """Emit a deprecation warning steering users to --enable-azure-monitor-logs.""" +def _show_monitoring_addon_legacy_info(): + """Point legacy monitoring-addon users to the strategic AMP onboarding command.""" logger.warning( - "The 'monitoring' addon (--enable-addons monitoring / --disable-addons monitoring) is " - "deprecated for Container Insights. Use '--enable-azure-monitor-logs' / " - "'--disable-azure-monitor-logs' instead, which configure the azureMonitorProfile." + "The 'monitoring' addon is the legacy Container Insights onboarding path. " + "Use '--enable-azure-monitor-logs' / '--disable-azure-monitor-logs' to configure " + "the Azure Monitor profile." + ) + + +def _warn_legacy_msi_auth_flag_deprecated(): + """Warn when the legacy monitoring addon explicitly selects an authentication mode.""" + logger.warning( + "'--enable-msi-auth-for-monitoring' is deprecated on the legacy monitoring-addon path. " + "Managed-identity authentication through '--enable-azure-monitor-logs' is recommended." ) @@ -1225,23 +1235,21 @@ def validate_azure_monitor_logs_and_enable_addons(namespace): ) # The containerInsights path is always MSI/AAD; legacy (non-MSI) auth cannot be expressed. _validate_monitor_logs_requires_msi_auth(namespace) - elif enable_addons and 'monitoring' in enable_addons: - _warn_monitoring_addon_deprecated() def _validate_monitor_logs_requires_msi_auth(namespace): """--enable-azure-monitor-logs uses the containerInsights path, which is always MSI/AAD auth. - Legacy (non-MSI) auth has no representation on that surface, so reject an explicit request to - disable MSI auth alongside it. + Auth selection has no representation on that surface, so reject any explicit use of the legacy + auth-selection flag alongside it. """ disable_msi_auth = getattr(namespace, 'disable_msi_auth_for_monitoring', None) enable_msi_auth = getattr(namespace, 'enable_msi_auth_for_monitoring', None) - if disable_msi_auth or enable_msi_auth is False: + if disable_msi_auth or enable_msi_auth is not None: raise ArgumentUsageError( "'--enable-azure-monitor-logs' onboards Container Insights via the azureMonitorProfile, " - "which always uses managed-identity (MSI/AAD) authentication. It cannot be combined with " - "disabling MSI auth for monitoring." + "which always uses managed-identity (MSI/AAD) authentication. Do not combine it with " + "'--enable-msi-auth-for-monitoring'." ) @@ -1257,6 +1265,56 @@ def validate_azure_monitor_logs_enable_disable(namespace): _validate_monitor_logs_requires_msi_auth(namespace) +def _validate_azure_monitor_logs_amp_controls(namespace): + """Validate static relationships for AMP-only Container Insights controls.""" + syslog_port = getattr(namespace, 'syslog_port', None) + enable_prometheus_scraping = getattr(namespace, 'enable_prometheus_metrics_scraping', False) + disable_prometheus_scraping = getattr(namespace, 'disable_prometheus_metrics_scraping', False) + + if syslog_port is not None and not 1 <= syslog_port <= 65535: + raise InvalidArgumentValueError("--syslog-port must be an integer between 1 and 65535.") + + if enable_prometheus_scraping and disable_prometheus_scraping: + raise MutuallyExclusiveArgumentError( + "Cannot specify both '--enable-prometheus-metrics-scraping' and " + "'--disable-prometheus-metrics-scraping'." + ) + + amp_controls_requested = ( + syslog_port is not None or + enable_prometheus_scraping or + disable_prometheus_scraping + ) + if amp_controls_requested and getattr(namespace, 'disable_azure_monitor_logs', False): + raise MutuallyExclusiveArgumentError( + "Azure Monitor logs AMP controls cannot be combined with '--disable-azure-monitor-logs'." + ) + + +def validate_azure_monitor_logs_amp_controls_for_create(namespace): + """Validate AMP-only controls on create, where monitoring must be enabled in the same command.""" + _validate_azure_monitor_logs_amp_controls(namespace) + amp_controls_requested = ( + getattr(namespace, 'syslog_port', None) is not None or + getattr(namespace, 'enable_prometheus_metrics_scraping', False) or + getattr(namespace, 'disable_prometheus_metrics_scraping', False) + ) + if amp_controls_requested and not getattr(namespace, 'enable_azure_monitor_logs', False): + raise RequiredArgumentMissingError( + "'--syslog-port' and Prometheus-scraping controls require " + "'--enable-azure-monitor-logs' on cluster create." + ) + + +def validate_azure_monitor_logs_amp_controls_for_update(namespace): + """Validate static AMP-control relationships on update. + + Whether monitoring is already enabled is validated against the fetched cluster in the update + decorator. + """ + _validate_azure_monitor_logs_amp_controls(namespace) + + def validate_nat_gateway_managed_outbound_ipv6_count(namespace): """validate NAT gateway profile managed outbound IPv6 count""" if namespace.nat_gateway_managed_outbound_ipv6_count is not None: diff --git a/src/aks-preview/azext_aks_preview/custom.py b/src/aks-preview/azext_aks_preview/custom.py index 50b5210e40d..af01356a5c1 100644 --- a/src/aks-preview/azext_aks_preview/custom.py +++ b/src/aks-preview/azext_aks_preview/custom.py @@ -1280,7 +1280,10 @@ def aks_create( enable_addons=None, # pylint: disable=redefined-outer-name enable_azure_monitor_logs=False, workspace_resource_id=None, - enable_msi_auth_for_monitoring=True, + syslog_port=None, + enable_prometheus_metrics_scraping=False, + disable_prometheus_metrics_scraping=False, + enable_msi_auth_for_monitoring=None, enable_syslog=None, data_collection_settings=None, ampls_resource_id=None, @@ -1570,6 +1573,9 @@ def aks_update( enable_azure_monitor_logs=False, disable_azure_monitor_logs=False, workspace_resource_id=None, + syslog_port=None, + enable_prometheus_metrics_scraping=False, + disable_prometheus_metrics_scraping=False, enable_msi_auth_for_monitoring=None, enable_syslog=None, data_collection_settings=None, @@ -3589,7 +3595,7 @@ def aks_enable_addons( enable_secret_rotation=False, rotation_poll_interval=None, no_wait=False, - enable_msi_auth_for_monitoring=True, + enable_msi_auth_for_monitoring=None, dns_zone_resource_id=None, dns_zone_resource_ids=None, enable_syslog=None, @@ -3604,6 +3610,10 @@ def aks_enable_addons( msi_auth = False if instance.service_principal_profile.client_id == "msi": msi_auth = True + if enable_msi_auth_for_monitoring is None: + # Preserve the legacy command's existing default while allowing validators to + # distinguish an omitted flag from an explicitly supplied true/false value. + enable_msi_auth_for_monitoring = True else: enable_msi_auth_for_monitoring = False diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 37798a6ec31..e911cf6a121 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -192,6 +192,19 @@ def _get_monitoring_addon_key_from_consts(addon_profiles, addon_consts): ) +def _find_monitoring_addon_profile(addon_profiles, addon_consts): + """Find the monitoring addon without normalizing or mutating addon-key casing.""" + if not addon_profiles: + return None, None + monitoring_addon_name = addon_consts.get("CONST_MONITORING_ADDON_NAME") + key = next( + (candidate for candidate in addon_profiles + if candidate.lower() == monitoring_addon_name.lower()), + None, + ) + return key, addon_profiles.get(key) if key else None + + def _is_container_network_logs_enabled_on_mc(mc, addon_consts): """Return True if container network logs are already enabled on the cluster. @@ -209,8 +222,7 @@ def _is_container_network_logs_enabled_on_mc(mc, addon_consts): cnl = getattr(cnl, "value", cnl) return str(cnl).lower() == "enabled" if mc.addon_profiles: - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_profile = mc.addon_profiles.get(mk) + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) if monitoring_profile and monitoring_profile.config: return str( monitoring_profile.config.get("enableRetinaNetworkFlags", "") @@ -222,12 +234,7 @@ def _clear_legacy_container_network_logs_config(mc, addon_consts): """Remove the legacy CNL config when the modern containerInsights field is explicitly written.""" if not mc or not mc.addon_profiles: return - monitoring_addon_name = addon_consts.get("CONST_MONITORING_ADDON_NAME") - monitoring_key = next( - (key for key in mc.addon_profiles if key.lower() == monitoring_addon_name.lower()), - None, - ) - monitoring_profile = mc.addon_profiles.get(monitoring_key) if monitoring_key else None + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) if not monitoring_profile or not monitoring_profile.config: return for key in list(monitoring_profile.config): @@ -243,12 +250,10 @@ def _is_monitoring_enabled_on_mc(mc, addon_consts): """ if mc is None: return False - if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and - mc.azure_monitor_profile.container_insights.enabled): - return True + if mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights is not None: + return bool(mc.azure_monitor_profile.container_insights.enabled) if mc.addon_profiles: - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_profile = mc.addon_profiles.get(mk) + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) return bool(monitoring_profile and monitoring_profile.enabled) return False @@ -256,27 +261,89 @@ def _is_monitoring_enabled_on_mc(mc, addon_consts): def _is_monitoring_msi_auth_on_mc(mc, addon_consts): """Return True if Container Insights uses MSI/AAD auth on the cluster. - The modern ``containerInsights`` surface is always MSI/AAD (the RP defaults ``useAADAuth=true`` - when it mirrors the profile), so treat an enabled containerInsights as MSI. Otherwise fall back - to the legacy ``omsagent.config.useAADAuth`` flag. + Respect an explicit legacy ``omsagent.config.useAADAuth`` value when present. Otherwise, the + modern ``containerInsights`` surface implies MSI/AAD because the RP defaults ``useAADAuth=true`` + when it synthesizes the addon. """ if mc is None: return False - if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights and - mc.azure_monitor_profile.container_insights.enabled): - return True CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") if mc.addon_profiles: - mk = _get_monitoring_addon_key_from_consts(mc.addon_profiles, addon_consts) - monitoring_profile = mc.addon_profiles.get(mk) + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) addon_config = (monitoring_profile.config or {}) if monitoring_profile else {} - return ( - CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config and - str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" - ) + if CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config: + return str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" + if mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights is not None: + return bool(mc.azure_monitor_profile.container_insights.enabled) return False +def _azure_monitor_logs_amp_controls_requested(raw_param): + """Return True when any Container Insights AMP-only control was supplied.""" + return ( + raw_param.get("syslog_port") is not None or + bool(raw_param.get("enable_prometheus_metrics_scraping")) or + bool(raw_param.get("disable_prometheus_metrics_scraping")) + ) + + +def _apply_azure_monitor_logs_amp_controls(container_insights, raw_param): + """Apply explicitly supplied AMP-only controls without disturbing other profile fields.""" + syslog_port = raw_param.get("syslog_port") + if syslog_port is not None: + container_insights.syslog_port = syslog_port + + if raw_param.get("enable_prometheus_metrics_scraping"): + container_insights.disable_prometheus_metrics_scraping = False + elif raw_param.get("disable_prometheus_metrics_scraping"): + container_insights.disable_prometheus_metrics_scraping = True + + +def _get_legacy_monitoring_workspace_id(mc, addon_consts): + """Read the legacy omsagent workspace ID without changing addon-key casing.""" + if not mc or not mc.addon_profiles: + return None + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) + if not monitoring_profile or not monitoring_profile.config: + return None + workspace_key = addon_consts.get("CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") + for key, value in monitoring_profile.config.items(): + if key.lower() == workspace_key.lower(): + return value + return None + + +def _get_legacy_container_network_logs_value(mc, addon_consts): + """Map the legacy Retina flow-log flag to the modern ContainerNetworkLogs enum value.""" + if not mc or not mc.addon_profiles: + return None + _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) + if not monitoring_profile or not monitoring_profile.config: + return None + for key, value in monitoring_profile.config.items(): + if key.lower() == "enableretinanetworkflags": + return "Enabled" if str(value).lower() == "true" else "Disabled" + return None + + +def _get_or_create_container_insights_profile(mc, models, addon_consts): + """Materialize a complete AMP profile while preserving brownfield addon state.""" + monitoring_enabled = _is_monitoring_enabled_on_mc(mc, addon_consts) + if mc.azure_monitor_profile is None: + mc.azure_monitor_profile = models.ManagedClusterAzureMonitorProfile() + container_insights = mc.azure_monitor_profile.container_insights + if container_insights is None: + container_insights = models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=monitoring_enabled, + log_analytics_workspace_resource_id=_get_legacy_monitoring_workspace_id( + mc, addon_consts), + container_network_logs=_get_legacy_container_network_logs_value( + mc, addon_consts), + ) + mc.azure_monitor_profile.container_insights = container_insights + return container_insights + + def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): """Return the omsagent-style addon profile the shared DCR/DCRA engine consumes. @@ -291,15 +358,10 @@ def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") - monitoring_addon_key = ( - _get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts) - if cluster.addon_profiles - else None - ) - if (monitoring_addon_key and cluster.addon_profiles and - monitoring_addon_key in cluster.addon_profiles and - cluster.addon_profiles[monitoring_addon_key].config): - return cluster.addon_profiles[monitoring_addon_key] + _, monitoring_addon_profile = _find_monitoring_addon_profile( + cluster.addon_profiles, addon_consts) + if monitoring_addon_profile and monitoring_addon_profile.config: + return monitoring_addon_profile workspace_resource_id = None container_insights = ( @@ -576,6 +638,11 @@ def get_enable_msi_auth_for_monitoring(self) -> Union[bool, None]: result = True elif enable_azure_monitor_logs: result = True + elif (self.decorator_mode == DecoratorMode.CREATE and + enable_msi_auth is None and not disable_msi_auth): + # Preserve the legacy create-path default after changing the command-signature default + # to None so validators can distinguish an omitted flag from an explicit true/false. + result = True elif enable_msi_auth_for_monitoring is False: # The base class returns False when service_principal_profile.client_id is not None, # but MSI-based clusters set client_id to "msi". Check the modern containerInsights @@ -4691,12 +4758,9 @@ def set_up_addon_profiles(self, mc: ManagedCluster) -> ManagedCluster: if container_network_logs_enabled is not None: # Container network logs are configured on the modern azureMonitorProfile.containerInsights # surface (containerNetworkLogs enum) rather than the legacy omsagent addon config. - self._ensure_azure_monitor_profile(mc) - if mc.azure_monitor_profile.container_insights is None: - mc.azure_monitor_profile.container_insights = ( - self.models.ManagedClusterAzureMonitorProfileContainerInsights() - ) - mc.azure_monitor_profile.container_insights.container_network_logs = ( + container_insights = _get_or_create_container_insights_profile( + mc, self.models, addon_consts) + container_insights.container_network_logs = ( "Enabled" if container_network_logs_enabled else "Disabled" ) _clear_legacy_container_network_logs_config(mc, addon_consts) @@ -5155,6 +5219,7 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: container_insights = self.models.ManagedClusterAzureMonitorProfileContainerInsights() container_insights.enabled = True container_insights.log_analytics_workspace_resource_id = workspace_resource_id + _apply_azure_monitor_logs_amp_controls(container_insights, self.context.raw_param) mc.azure_monitor_profile.container_insights = container_insights # DCR and DCRA creation is deferred to postprocessing_after_mc_created @@ -6552,16 +6617,13 @@ def update_monitoring_profile_flow_logs(self, mc: ManagedCluster) -> ManagedClus container_network_logs_enabled = self.context.get_container_network_logs(mc) if container_network_logs_enabled is not None: # Container network logs are configured on the modern containerInsights surface. - self._ensure_azure_monitor_profile(mc) - if mc.azure_monitor_profile.container_insights is None: - mc.azure_monitor_profile.container_insights = ( - self.models.ManagedClusterAzureMonitorProfileContainerInsights() - ) - mc.azure_monitor_profile.container_insights.container_network_logs = ( + addon_consts = self.context.get_addon_consts() + container_insights = _get_or_create_container_insights_profile( + mc, self.models, addon_consts) + container_insights.container_network_logs = ( "Enabled" if container_network_logs_enabled else "Disabled" ) - _clear_legacy_container_network_logs_config( - mc, self.context.get_addon_consts()) + _clear_legacy_container_network_logs_config(mc, addon_consts) # When enabling CNL, the DCR must be updated to add the high-scale stream. # Set the postprocessing intermediate so that the update path calls ensure_container_insights. @@ -7716,6 +7778,10 @@ def update_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: logs_port = self.context.get_opentelemetry_logs_port() mc.azure_monitor_profile.app_monitoring.open_telemetry_logs_and_traces.http_port = logs_port + # Apply Container Insights AMP-only controls independently of the enable flag so users can + # tune an already-onboarded cluster without re-enabling or disturbing other profile fields. + self._apply_azure_monitor_logs_amp_control_updates(mc) + # TODO: should remove get value from enable_azuremonitormetrics once the option is removed # TODO: should remove get value from disable_azuremonitormetrics once the option is removed azure_monitor_metrics = (self.context.raw_param.get("enable_azuremonitormetrics") or @@ -8548,6 +8614,33 @@ def _ensure_azure_monitor_profile(self, mc: ManagedCluster) -> None: if mc.azure_monitor_profile is None: mc.azure_monitor_profile = self.models.ManagedClusterAzureMonitorProfile() + def _apply_azure_monitor_logs_amp_control_updates(self, mc: ManagedCluster) -> None: + """Apply standalone syslog/scraping updates to an already-onboarded cluster.""" + if not _azure_monitor_logs_amp_controls_requested(self.context.raw_param): + return + + addon_consts = self.context.get_addon_consts() + enabling_monitoring = bool(self.context.raw_param.get("enable_azure_monitor_logs")) + if not enabling_monitoring and not _is_monitoring_enabled_on_mc(mc, addon_consts): + raise RequiredArgumentMissingError( + "'--syslog-port' and Prometheus-scraping controls require Azure Monitor logs " + "(Container Insights) to already be enabled, or '--enable-azure-monitor-logs' " + "in the same command." + ) + if not enabling_monitoring and not _is_monitoring_msi_auth_on_mc(mc, addon_consts): + raise ArgumentUsageError( + "'--syslog-port' and Prometheus-scraping controls are available only on the " + "managed-identity Azure Monitor profile path. Re-onboard with " + "'--enable-azure-monitor-logs' before using these controls." + ) + + # Defensive brownfield fallback: materialize a complete AMP profile from the legacy addon + # before changing one AMP-only field, because the RP treats AMP as authoritative. + container_insights = _get_or_create_container_insights_profile( + mc, self.models, addon_consts) + _apply_azure_monitor_logs_amp_controls(container_insights, self.context.raw_param) + mc.azure_monitor_profile.container_insights = container_insights + def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: """Set up Azure Monitor logs (Container Insights) via azureMonitorProfile.containerInsights. @@ -8592,6 +8685,7 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: "Enabled" if container_network_logs_enabled else "Disabled" ) + _apply_azure_monitor_logs_amp_controls(container_insights, self.context.raw_param) mc.azure_monitor_profile.container_insights = container_insights self.context.set_intermediate("monitoring_addon_enabled", True, overwrite_exists=True) @@ -8600,7 +8694,9 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: Clears ``azureMonitorProfile.containerInsights`` (the RP source of truth) as the primary surface, and also clears the legacy ``addonProfiles.omsagent`` addon when present (brownfield - clusters / RP-mirrored state) so the RP does not re-enable the addon. + clusters / RP-mirrored state) so a full-cluster PUT cannot retain stale legacy state and the + RP does not re-enable the addon. Clearing both is compatibility cleanup; disable does not + depend on the addon being present. """ addon_consts = self.context.get_addon_consts() CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index e5be2836366..112b2468491 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -6015,6 +6015,26 @@ def test_get_enable_msi_auth_for_monitoring_with_msi_service_principal(self): result = ctx.get_enable_msi_auth_for_monitoring() self.assertTrue(result) + def test_create_monitoring_defaults_to_msi_when_auth_flag_omitted(self): + """Legacy create keeps its existing MSI default while the raw flag remains None.""" + ctx = AKSPreviewManagedClusterContext( + self.cmd, + AKSManagedClusterParamDict({ + "enable_addons": "monitoring", + "enable_msi_auth_for_monitoring": None, + }), + self.models, + decorator_mode=DecoratorMode.CREATE, + ) + mc = self.models.ManagedCluster( + location="test_location", + service_principal_profile=self.models.ManagedClusterServicePrincipalProfile( + client_id="msi", + ), + ) + ctx.attach_mc(mc) + self.assertTrue(ctx.get_enable_msi_auth_for_monitoring()) + def test_get_enable_msi_auth_for_monitoring_with_containerinsights_only(self): """The modern containerInsights profile implies MSI even without an omsagent mirror.""" ctx = AKSPreviewManagedClusterContext( @@ -9894,6 +9914,34 @@ def test_set_up_health_monitor_profile(self): ) self.assertEqual(dec_mc_2, ground_truth_mc_2) + def test_setup_azure_monitor_logs_applies_amp_controls(self): + dec = AKSPreviewManagedClusterCreateDecorator( + self.cmd, + self.client, + { + "enable_azure_monitor_logs": True, + "workspace_resource_id": "/subscriptions/test/resourceGroups/rg/providers/" + "Microsoft.OperationalInsights/workspaces/ws", + "syslog_port": 28331, + "disable_prometheus_metrics_scraping": True, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster(location="test_location") + dec.context.attach_mc(mc) + + with patch.object( + dec.context.external_functions, + "sanitize_loganalytics_ws_resource_id", + side_effect=lambda value: value, + ): + dec._setup_azure_monitor_logs(mc) + + container_insights = mc.azure_monitor_profile.container_insights + self.assertTrue(container_insights.enabled) + self.assertEqual(container_insights.syslog_port, 28331) + self.assertTrue(container_insights.disable_prometheus_metrics_scraping) + class AKSPreviewManagedClusterUpdateDecoratorTestCase(unittest.TestCase): def setUp(self): @@ -9904,6 +9952,117 @@ def setUp(self): self.models = AKSPreviewManagedClusterModels(self.cmd, CUSTOM_MGMT_AKS_PREVIEW) self.client = MockClient() + def test_update_amp_controls_preserve_existing_container_insights(self): + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "syslog_port": 29000, + "enable_prometheus_metrics_scraping": True, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + log_analytics_workspace_resource_id="/subscriptions/test/workspaces/ws", + syslog_port=28330, + disable_prometheus_metrics_scraping=True, + container_network_logs="Enabled", + ), + ), + ) + dec.context.attach_mc(mc) + dec.update_azure_monitor_profile(mc) + + container_insights = mc.azure_monitor_profile.container_insights + self.assertTrue(container_insights.enabled) + self.assertEqual( + container_insights.log_analytics_workspace_resource_id, + "/subscriptions/test/workspaces/ws", + ) + self.assertEqual(container_insights.syslog_port, 29000) + self.assertFalse(container_insights.disable_prometheus_metrics_scraping) + self.assertEqual(container_insights.container_network_logs, "Enabled") + self.assertFalse( + dec.context.get_intermediate( + "monitoring_addon_postprocessing_required", default_value=False) + ) + + def test_update_amp_controls_materialize_brownfield_profile(self): + workspace_id = "/subscriptions/test/resourceGroups/rg/providers/" \ + "Microsoft.OperationalInsights/workspaces/ws" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "disable_prometheus_metrics_scraping": True, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + "omsAgent": self.models.ManagedClusterAddonProfile( + enabled=True, + config={ + "logAnalyticsWorkspaceResourceID": workspace_id, + "useAADAuth": "true", + "enableRetinaNetworkFlags": "true", + }, + ), + }, + ) + dec.context.attach_mc(mc) + dec._apply_azure_monitor_logs_amp_control_updates(mc) + + container_insights = mc.azure_monitor_profile.container_insights + self.assertTrue(container_insights.enabled) + self.assertEqual(container_insights.log_analytics_workspace_resource_id, workspace_id) + self.assertTrue(container_insights.disable_prometheus_metrics_scraping) + self.assertEqual(container_insights.container_network_logs, "Enabled") + + def test_update_amp_controls_require_monitoring(self): + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "syslog_port": 28331, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster(location="test_location") + dec.context.attach_mc(mc) + with self.assertRaises(RequiredArgumentMissingError): + dec._apply_azure_monitor_logs_amp_control_updates(mc) + + def test_update_amp_controls_reject_legacy_shared_key_cluster(self): + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "syslog_port": 28331, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + "omsagent": self.models.ManagedClusterAddonProfile( + enabled=True, + config={ + "logAnalyticsWorkspaceResourceID": "/subscriptions/test/workspaces/ws", + "useAADAuth": "false", + }, + ), + }, + ) + dec.context.attach_mc(mc) + with self.assertRaises(ArgumentUsageError): + dec._apply_azure_monitor_logs_amp_control_updates(mc) + def test_check_raw_parameters(self): # default value in `aks_create` dec_1 = AKSPreviewManagedClusterUpdateDecorator( @@ -15360,6 +15519,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Enabled", ), ), @@ -15416,6 +15576,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Disabled", ), ), @@ -15630,6 +15791,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Enabled", ), ), @@ -15825,6 +15987,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Enabled", ), ), @@ -15886,6 +16049,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Disabled", ), ), @@ -15942,6 +16106,7 @@ def test_enable_container_network_logs(self): }, azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, container_network_logs="Disabled", ), ), diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py index f4ca07a88d6..96347e30029 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py @@ -2418,7 +2418,74 @@ def test_validate_addons_warns_for_create_enable_addons_monitoring(self): with patch.object(validators.logger, "warning") as warning: validators.validate_addons(namespace) warning.assert_called_once() - self.assertIn("deprecated", warning.call_args.args[0]) + self.assertIn("legacy Container Insights onboarding path", warning.call_args.args[0]) + + def test_validate_addons_warns_twice_when_legacy_auth_flag_is_explicit(self): + namespace = SimpleNamespace( + addons="monitoring", + enable_msi_auth_for_monitoring=False, + ) + with patch.object(validators.logger, "warning") as warning: + validators.validate_addons(namespace) + self.assertEqual(warning.call_count, 2) + messages = [call.args[0] for call in warning.call_args_list] + self.assertTrue(any("legacy Container Insights onboarding path" in message for message in messages)) + self.assertTrue(any("'--enable-msi-auth-for-monitoring' is deprecated" in message for message in messages)) + + def test_validate_addons_does_not_warn_for_auth_flag_when_omitted(self): + namespace = SimpleNamespace(addons="monitoring") + with patch.object(validators.logger, "warning") as warning: + validators.validate_addons(namespace) + warning.assert_called_once() + self.assertNotIn("--enable-msi-auth-for-monitoring", warning.call_args.args[0]) + + def test_enable_azure_monitor_logs_rejects_explicit_msi_auth_true(self): + namespace = SimpleNamespace( + enable_azure_monitor_logs=True, + enable_addons=None, + enable_msi_auth_for_monitoring=True, + ) + with self.assertRaises(ArgumentUsageError): + validators.validate_azure_monitor_logs_and_enable_addons(namespace) + + def test_enable_azure_monitor_logs_rejects_explicit_msi_auth_false(self): + namespace = SimpleNamespace( + enable_azure_monitor_logs=True, + enable_addons=None, + enable_msi_auth_for_monitoring=False, + ) + with self.assertRaises(ArgumentUsageError): + validators.validate_azure_monitor_logs_and_enable_addons(namespace) + + def test_amp_controls_create_require_enable_azure_monitor_logs(self): + namespace = SimpleNamespace( + enable_azure_monitor_logs=False, + syslog_port=28331, + enable_prometheus_metrics_scraping=False, + disable_prometheus_metrics_scraping=False, + ) + with self.assertRaises(RequiredArgumentMissingError): + validators.validate_azure_monitor_logs_amp_controls_for_create(namespace) + + def test_amp_controls_reject_invalid_syslog_port(self): + namespace = SimpleNamespace( + enable_azure_monitor_logs=True, + syslog_port=65536, + enable_prometheus_metrics_scraping=False, + disable_prometheus_metrics_scraping=False, + ) + with self.assertRaises(InvalidArgumentValueError): + validators.validate_azure_monitor_logs_amp_controls_for_create(namespace) + + def test_amp_controls_reject_conflicting_scraping_flags(self): + namespace = SimpleNamespace( + enable_azure_monitor_logs=True, + syslog_port=None, + enable_prometheus_metrics_scraping=True, + disable_prometheus_metrics_scraping=True, + ) + with self.assertRaises(MutuallyExclusiveArgumentError): + validators.validate_azure_monitor_logs_amp_controls_for_create(namespace) class TestValidateAzureMonitorLogsEnableDisable(unittest.TestCase): From 9493cc1295593e25e41f519723f27832cac44b1f Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Mon, 27 Jul 2026 18:35:56 +0100 Subject: [PATCH 7/9] aks-preview: fix AMP DCR postprocessing blockers --- src/aks-preview/HISTORY.rst | 4 +- .../managed_cluster_decorator.py | 98 ++++--- .../latest/test_managed_cluster_decorator.py | 249 +++++++++++++++++- 3 files changed, 307 insertions(+), 44 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index ff3ba58f71e..986d700e102 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -18,7 +18,9 @@ Pending * `az aks create/update`: `--enable-azure-monitor-logs` / `--disable-azure-monitor-logs` now configure the modern ``azureMonitorProfile.containerInsights`` profile instead of the legacy ``addonProfiles.omsagent`` addon. The containerInsights path always uses managed-identity (MSI/AAD) auth. * `az aks create/update`: Add ``--syslog-port`` and ``--enable/--disable-prometheus-metrics-scraping`` controls for the Container Insights Azure Monitor profile. * `az aks create/update`: Reject the legacy ``--enable-msi-auth-for-monitoring`` flag when used with ``--enable-azure-monitor-logs`` because the Azure Monitor profile is managed-identity only. -* `az aks enable-addons`: Show guidance toward ``--enable-azure-monitor-logs`` for the legacy ``monitoring`` addon and warn when ``--enable-msi-auth-for-monitoring`` is explicitly supplied. +* `az aks update`: Reject migration of an already-enabled legacy shared-key monitoring addon through ``--enable-azure-monitor-logs`` with guidance to migrate the addon to managed-identity authentication first. +* `az aks update`: Avoid DCR/DCRA postprocessing when Container Insights is disabled or has no workspace, and explicitly set container network logs to ``Disabled`` when Azure Monitor logs are disabled. +* `az aks create --enable-addons monitoring` and `az aks enable-addons -a monitoring`: Show guidance toward ``--enable-azure-monitor-logs`` for the legacy ``monitoring`` addon and warn when ``--enable-msi-auth-for-monitoring`` is explicitly supplied. 21.0.0b13 ++++++++ diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index e911cf6a121..40d8c9e6b2e 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -205,6 +205,16 @@ def _find_monitoring_addon_profile(addon_profiles, addon_consts): return key, addon_profiles.get(key) if key else None +def _get_monitoring_addon_config_value(monitoring_profile, config_key): + """Read a monitoring addon config value without assuming key casing.""" + if not monitoring_profile or not monitoring_profile.config or not config_key: + return None + for key, value in monitoring_profile.config.items(): + if key.lower() == config_key.lower(): + return value + return None + + def _is_container_network_logs_enabled_on_mc(mc, addon_consts): """Return True if container network logs are already enabled on the cluster. @@ -270,9 +280,10 @@ def _is_monitoring_msi_auth_on_mc(mc, addon_consts): CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") if mc.addon_profiles: _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) - addon_config = (monitoring_profile.config or {}) if monitoring_profile else {} - if CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config: - return str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" + use_aad_auth = _get_monitoring_addon_config_value( + monitoring_profile, CONST_MONITORING_USING_AAD_MSI_AUTH) + if use_aad_auth is not None: + return str(use_aad_auth).lower() == "true" if mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights is not None: return bool(mc.azure_monitor_profile.container_insights.enabled) return False @@ -304,13 +315,8 @@ def _get_legacy_monitoring_workspace_id(mc, addon_consts): if not mc or not mc.addon_profiles: return None _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) - if not monitoring_profile or not monitoring_profile.config: - return None workspace_key = addon_consts.get("CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") - for key, value in monitoring_profile.config.items(): - if key.lower() == workspace_key.lower(): - return value - return None + return _get_monitoring_addon_config_value(monitoring_profile, workspace_key) def _get_legacy_container_network_logs_value(mc, addon_consts): @@ -318,12 +324,11 @@ def _get_legacy_container_network_logs_value(mc, addon_consts): if not mc or not mc.addon_profiles: return None _, monitoring_profile = _find_monitoring_addon_profile(mc.addon_profiles, addon_consts) - if not monitoring_profile or not monitoring_profile.config: + value = _get_monitoring_addon_config_value( + monitoring_profile, "enableRetinaNetworkFlags") + if value is None: return None - for key, value in monitoring_profile.config.items(): - if key.lower() == "enableretinanetworkflags": - return "Enabled" if str(value).lower() == "true" else "Disabled" - return None + return "Enabled" if str(value).lower() == "true" else "Disabled" def _get_or_create_container_insights_profile(mc, models, addon_consts): @@ -349,10 +354,10 @@ def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): Container Insights is now configured via ``azureMonitorProfile.containerInsights`` and the AKS RP normalizes it into ``addonProfiles.omsagent`` on the PUT response. When that mirrored addon is - present (and has config) we use it directly. Otherwise (e.g. mocked responses in unit tests, or a - hypothetical response without the mirror) we build a transient omsagent-style profile from - ``containerInsights`` so the DCR engine still gets the workspace id and MSI/AAD auth signal. The - returned object is only passed to the engine, never persisted on the cluster. + present, enabled, and has a workspace we use it directly. Otherwise (e.g. mocked responses in + unit tests, or a hypothetical response without the mirror) we build a transient omsagent-style + profile only when ``containerInsights`` is enabled and has a workspace. The returned object is + only passed to the engine, never persisted on the cluster. """ CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID = addon_consts.get( "CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID") @@ -360,17 +365,24 @@ def _build_monitoring_addon_profile_for_dcr(addon_consts, models, cluster): _, monitoring_addon_profile = _find_monitoring_addon_profile( cluster.addon_profiles, addon_consts) - if monitoring_addon_profile and monitoring_addon_profile.config: + monitoring_workspace_id = _get_monitoring_addon_config_value( + monitoring_addon_profile, CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID) + if monitoring_addon_profile and monitoring_addon_profile.enabled and monitoring_workspace_id: return monitoring_addon_profile - workspace_resource_id = None container_insights = ( cluster.azure_monitor_profile.container_insights if cluster.azure_monitor_profile else None ) - if container_insights is not None: - workspace_resource_id = container_insights.log_analytics_workspace_resource_id + workspace_resource_id = ( + container_insights.log_analytics_workspace_resource_id + if container_insights + else None + ) + if not (container_insights and container_insights.enabled and workspace_resource_id): + return monitoring_addon_profile + return models.ManagedClusterAddonProfile( enabled=True, config={ @@ -8648,6 +8660,18 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None: ``addonProfiles.omsagent`` addon. The AKS RP normalizes containerInsights into omsagent on the PUT (defaulting ``useAADAuth=true``), so MSI/AAD auth is implied on this path. """ + addon_consts = self.context.get_addon_consts() + if (_is_monitoring_enabled_on_mc(mc, addon_consts) and + not _is_monitoring_msi_auth_on_mc(mc, addon_consts)): + resource_group_name = self.context.get_resource_group_name() + cluster_name = self.context.get_name() + raise ArgumentUsageError( + "Azure Monitor logs are already enabled using legacy shared-key authentication. " + "Migrate monitoring to managed-identity authentication first with " + f"'az aks addon update -g {resource_group_name} -n {cluster_name} -a monitoring " + "--enable-msi-auth-for-monitoring true', then rerun this command." + ) + # Get or create workspace resource ID workspace_resource_id = self.context.raw_param.get("workspace_resource_id") if not workspace_resource_id: @@ -8795,9 +8819,8 @@ def _disable_azure_monitor_logs(self, mc: ManagedCluster) -> None: if (mc.azure_monitor_profile and mc.azure_monitor_profile.container_insights): mc.azure_monitor_profile.container_insights.enabled = False - # Clear container network logs so a later re-enable (without a CNL flag) does not - # silently resurrect CNL from stale state. - mc.azure_monitor_profile.container_insights.container_network_logs = None + # Explicitly disable CNL so the PUT cannot preserve stale enabled state. + mc.azure_monitor_profile.container_insights.container_network_logs = "Disabled" # Also disable OpenTelemetry logs when disabling Azure Monitor logs if opentelemetry_logs_enabled: @@ -9038,30 +9061,25 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: ) if monitoring_addon_postprocessing_required: addon_consts = self.context.get_addon_consts() - CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH") # Resolve the omsagent-style profile the DCR engine consumes. Prefer the RP-mirrored # omsagent from the PUT response; fall back to a transient profile synthesized from - # containerInsights so a missing/unmirrored addon does not silently skip DCR creation - # (or crash on a None config). + # a complete, enabled containerInsights profile. monitoring_addon_profile = _build_monitoring_addon_profile_for_dcr( addon_consts, self.models, cluster) - # Container Insights is considered enabled if either surface reports it. - container_insights_enabled = bool( - cluster.azure_monitor_profile and - cluster.azure_monitor_profile.container_insights and - cluster.azure_monitor_profile.container_insights.enabled - ) - monitoring_enabled = bool(monitoring_addon_profile.enabled) or container_insights_enabled - - addon_config = monitoring_addon_profile.config or {} + monitoring_enabled = _is_monitoring_enabled_on_mc(cluster, addon_consts) msi_auth_enabled = ( - CONST_MONITORING_USING_AAD_MSI_AUTH in addon_config and - str(addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true" + bool(self.context.raw_param.get("enable_azure_monitor_logs")) or + _is_monitoring_msi_auth_on_mc(cluster, addon_consts) + ) + workspace_id = _get_monitoring_addon_config_value( + monitoring_addon_profile, + addon_consts.get("CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID"), ) - if monitoring_enabled and msi_auth_enabled: + if (monitoring_enabled and msi_auth_enabled and monitoring_addon_profile and + monitoring_addon_profile.enabled and workspace_id): # Check parameter sizes to identify what might be causing large headers data_collection_settings = self.context.get_data_collection_settings() diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index 112b2468491..62e2d68191e 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -16666,6 +16666,9 @@ def test_update_postprocessing_with_camelcase_addon_key(self): enabled=True, config={ CONST_MONITORING_USING_AAD_MSI_AUTH: "true", + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: + "/subscriptions/test/resourceGroups/test/providers/" + "Microsoft.OperationalInsights/workspaces/test", }, ) }, @@ -16682,6 +16685,164 @@ def test_update_postprocessing_with_camelcase_addon_key(self): self.assertTrue(kwargs["create_dcr"]) self.assertTrue(kwargs["enable_high_log_scale_mode"]) + def test_update_postprocessing_skips_dcr_without_monitoring_profile(self): + """Explicitly disabling HLSM is a no-op when Container Insights was never configured.""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_high_log_scale_mode": False, + "name": "test_name", + "resource_group_name": "test_rg_name", + "location": "test_location", + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster(location="test_location") + dec.context.attach_mc(mc) + dec.context.set_intermediate("subscription_id", "test_subscription_id") + dec.update_monitoring_profile_flow_logs(mc) + + with patch.object( + dec.context.external_functions, + "ensure_container_insights_for_monitoring", + return_value=None, + ) as mock_ecifm: + dec.postprocessing_after_mc_created(mc) + + mock_ecifm.assert_not_called() + + def test_update_postprocessing_skips_dcr_for_configless_monitoring_addon(self): + """A config-less omsagent response must not reach the DCR engine with a null workspace.""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_high_log_scale_mode": False, + "name": "test_name", + "resource_group_name": "test_rg_name", + "location": "test_location", + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( + enabled=True, + config={}, + ), + }, + ) + dec.context.attach_mc(mc) + dec.context.set_intermediate("subscription_id", "test_subscription_id") + dec.update_monitoring_profile_flow_logs(mc) + + with patch.object( + dec.context.external_functions, + "ensure_container_insights_for_monitoring", + return_value=None, + ) as mock_ecifm: + dec.postprocessing_after_mc_created(mc) + + mock_ecifm.assert_not_called() + + def test_update_postprocessing_uses_amp_intent_when_mirrored_auth_is_false_or_absent(self): + """A valid AMP enable response must create DCR/DCRA even if the legacy auth echo is stale.""" + workspace_id = "/subscriptions/test/resourceGroups/rg/providers/" \ + "Microsoft.OperationalInsights/workspaces/ws" + for use_aad_auth in ("false", None): + with self.subTest(use_aad_auth=use_aad_auth): + addon_config = { + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_id, + } + if use_aad_auth is not None: + addon_config[CONST_MONITORING_USING_AAD_MSI_AUTH] = use_aad_auth + cluster = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( + enabled=True, + config=addon_config, + ), + }, + azure_monitor_profile=self.models.ManagedClusterAzureMonitorProfile( + container_insights=self.models.ManagedClusterAzureMonitorProfileContainerInsights( + enabled=True, + log_analytics_workspace_resource_id=workspace_id, + ), + ), + ) + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_azure_monitor_logs": True, + "name": "test_name", + "resource_group_name": "test_rg_name", + "location": "test_location", + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + dec.context.attach_mc(cluster) + dec.context.set_intermediate("subscription_id", "test_subscription_id") + dec.context.set_intermediate( + "monitoring_addon_postprocessing_required", True) + + with patch.object( + dec.context.external_functions, + "ensure_container_insights_for_monitoring", + return_value=None, + ) as mock_ecifm: + dec.postprocessing_after_mc_created(cluster) + + mock_ecifm.assert_called_once() + _, kwargs = mock_ecifm.call_args + self.assertTrue(kwargs["aad_route"]) + self.assertTrue(kwargs["create_dcr"]) + self.assertTrue(kwargs["create_dcra"]) + + def test_update_postprocessing_respects_shared_key_auth_without_amp_enable_intent(self): + """An unrelated HLSM update must not create MSI DCR artifacts for a shared-key cluster.""" + workspace_id = "/subscriptions/test/resourceGroups/rg/providers/" \ + "Microsoft.OperationalInsights/workspaces/ws" + cluster = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( + enabled=True, + config={ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_id, + CONST_MONITORING_USING_AAD_MSI_AUTH: "false", + }, + ), + }, + ) + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_high_log_scale_mode": False, + "name": "test_name", + "resource_group_name": "test_rg_name", + "location": "test_location", + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + dec.context.attach_mc(cluster) + dec.context.set_intermediate("subscription_id", "test_subscription_id") + dec.context.set_intermediate( + "monitoring_addon_postprocessing_required", True) + + with patch.object( + dec.context.external_functions, + "ensure_container_insights_for_monitoring", + return_value=None, + ) as mock_ecifm: + dec.postprocessing_after_mc_created(cluster) + + mock_ecifm.assert_not_called() + def test_update_node_provisioning_profile(self): dec_0 = AKSPreviewManagedClusterUpdateDecorator( self.cmd, @@ -18392,6 +18553,86 @@ def test_setup_azure_monitor_logs_no_retina_flags_without_cnl(self): self.assertIsNotNone(mc.azure_monitor_profile.container_insights) self.assertIsNone(mc.azure_monitor_profile.container_insights.container_network_logs) + def test_setup_azure_monitor_logs_rejects_enabled_shared_key_cluster(self): + """An already-enabled shared-key cluster must migrate auth before switching to AMP.""" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_azure_monitor_logs": True, + "workspace_resource_id": "/subscriptions/test/resourceGroups/rg/providers/" + "Microsoft.OperationalInsights/workspaces/ws", + "resource_group_name": "test-rg", + "name": "test-cluster", + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( + enabled=True, + config={ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: + "/subscriptions/test/resourceGroups/rg/providers/" + "Microsoft.OperationalInsights/workspaces/ws", + "UseAADAuth": "false", + }, + ), + }, + ) + dec.context.attach_mc(mc) + + with self.assertRaises(ArgumentUsageError) as cm: + dec._setup_azure_monitor_logs(mc) + + self.assertIn( + "az aks addon update -g test-rg -n test-cluster -a monitoring " + "--enable-msi-auth-for-monitoring true", + str(cm.exception), + ) + self.assertIsNone(mc.azure_monitor_profile) + + def test_setup_azure_monitor_logs_allows_reenable_from_disabled_shared_key_addon(self): + """The RP migrates auth on a disabled-to-enabled transition, so this case remains valid.""" + workspace_id = "/subscriptions/test/resourceGroups/rg/providers/" \ + "Microsoft.OperationalInsights/workspaces/ws" + dec = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + { + "enable_azure_monitor_logs": True, + "workspace_resource_id": workspace_id, + }, + CUSTOM_MGMT_AKS_PREVIEW, + ) + mc = self.models.ManagedCluster( + location="test_location", + addon_profiles={ + CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile( + enabled=False, + config={ + CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: workspace_id, + CONST_MONITORING_USING_AAD_MSI_AUTH: "false", + }, + ), + }, + ) + dec.context.attach_mc(mc) + + with patch.object( + dec.context.external_functions, + "sanitize_loganalytics_ws_resource_id", + side_effect=lambda value: value, + ): + dec._setup_azure_monitor_logs(mc) + + self.assertTrue(mc.azure_monitor_profile.container_insights.enabled) + self.assertEqual( + mc.azure_monitor_profile.container_insights.log_analytics_workspace_resource_id, + workspace_id, + ) + # ------------------------------------------------------------------ # Tests for _setup_azure_monitor_logs workspace change detection # ------------------------------------------------------------------ @@ -18638,8 +18879,7 @@ def test_disable_azure_monitor_logs_no_container_insights_skips(self): self.assertFalse(mc.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled) def test_disable_azure_monitor_logs_clears_container_network_logs(self): - """Disabling monitoring clears containerInsights.container_network_logs so a later - re-enable (without a CNL flag) does not silently resurrect CNL (rubber-duck finding #2).""" + """Disabling monitoring explicitly disables CNL so it cannot be resurrected later.""" dec = AKSPreviewManagedClusterUpdateDecorator( self.cmd, self.client, @@ -18664,7 +18904,10 @@ def test_disable_azure_monitor_logs_clears_container_network_logs(self): dec.client.get = Mock(return_value=mc) dec._disable_azure_monitor_logs(mc) self.assertFalse(mc.azure_monitor_profile.container_insights.enabled) - self.assertIsNone(mc.azure_monitor_profile.container_insights.container_network_logs) + self.assertEqual( + mc.azure_monitor_profile.container_insights.container_network_logs, + "Disabled", + ) def test_hlsm_standalone_allowed_on_containerinsights_only_cluster(self): """--enable-high-log-scale-mode is accepted when monitoring is on via containerInsights only From 9d7008484e52d80918f9bf8d686ea313581f0b10 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Tue, 4 Aug 2026 16:58:30 +0100 Subject: [PATCH 8/9] aks-preview: remove unused CONST_MONITORING_ADDON_NAME_CAMELCASE import --- src/aks-preview/azext_aks_preview/managed_cluster_decorator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 40d8c9e6b2e..8dbebf58140 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -54,7 +54,6 @@ CONST_ACNS_DATAPATH_ACCELERATION_MODE_NONE, CONST_TRANSIT_ENCRYPTION_TYPE_MTLS, CONST_ADVANCED_NETWORKPOLICIES_L7, - CONST_MONITORING_ADDON_NAME_CAMELCASE, ) from azext_aks_preview.azurecontainerstorage._consts import ( CONST_ACSTOR_EXT_INSTALLATION_NAME, From 555b4f4f0a76bc5d0dda20bfe72a719853b91f07 Mon Sep 17 00:00:00 2001 From: Christoph Butz Date: Tue, 4 Aug 2026 17:27:09 +0100 Subject: [PATCH 9/9] aks-preview: exclude prometheus-metrics-scraping flags from option_length_too_long linter rule --- src/aks-preview/linter_exclusions.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/aks-preview/linter_exclusions.yml b/src/aks-preview/linter_exclusions.yml index 685a4f9efc2..6bd267647e7 100644 --- a/src/aks-preview/linter_exclusions.yml +++ b/src/aks-preview/linter_exclusions.yml @@ -186,6 +186,12 @@ aks create: enable_on_demand_monitor: rule_exclusions: - option_length_too_long + enable_prometheus_metrics_scraping: + rule_exclusions: + - option_length_too_long + disable_prometheus_metrics_scraping: + rule_exclusions: + - option_length_too_long aks update: parameters: enable_pod_identity_with_kubenet: @@ -395,6 +401,12 @@ aks update: disable_on_demand_monitor: rule_exclusions: - option_length_too_long + enable_prometheus_metrics_scraping: + rule_exclusions: + - option_length_too_long + disable_prometheus_metrics_scraping: + rule_exclusions: + - option_length_too_long aks delete: parameters: ignore_pod_disruption_budget: