Repository navigation
{AKS} az aks create/update: add --enable/--disable-control-plane-metrics
#33537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
5f1deef
56ba118
5911068
138467b
a345556
e18c65d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5956,6 +5956,91 @@ def get_disable_azure_monitor_metrics(self) -> bool: | |
| """ | ||
| return self._get_disable_azure_monitor_metrics(enable_validation=True) | ||
|
|
||
| def _get_enable_control_plane_metrics(self, enable_validation: bool = False) -> bool: | ||
| """Internal function to obtain the value of enable_control_plane_metrics. | ||
| This function supports the option of enable_validation. When enabled, if both | ||
| enable_control_plane_metrics and disable_control_plane_metrics are specified, raise a | ||
| MutuallyExclusiveArgumentError. Additionally, --enable-control-plane-metrics requires | ||
| Azure Monitor metrics to either already be enabled on the cluster or to be enabled in | ||
| the same command via --enable-azure-monitor-metrics. | ||
|
|
||
| :return: bool | ||
| """ | ||
| # Read the original value passed by the command. | ||
| enable_control_plane_metrics = self.raw_param.get("enable_control_plane_metrics") | ||
| # In create mode, try to read the property value corresponding to the parameter from the `mc` object. | ||
| if self.decorator_mode == DecoratorMode.CREATE: | ||
| if ( | ||
| self.mc and | ||
| self.mc.azure_monitor_profile and | ||
| self.mc.azure_monitor_profile.metrics and | ||
| self.mc.azure_monitor_profile.metrics.control_plane | ||
| ): | ||
| enable_control_plane_metrics = self.mc.azure_monitor_profile.metrics.control_plane.enabled | ||
| # This parameter does not need dynamic completion. | ||
| if enable_validation: | ||
| if enable_control_plane_metrics and self._get_disable_control_plane_metrics(False): | ||
| raise MutuallyExclusiveArgumentError( | ||
| "Cannot specify --enable-control-plane-metrics and --disable-control-plane-metrics " | ||
| "at the same time." | ||
| ) | ||
|
bragi92 marked this conversation as resolved.
Outdated
|
||
| if enable_control_plane_metrics: | ||
| # Reject combining enable-control-plane-metrics with disable-azure-monitor-metrics | ||
| # in the same command — the resulting payload would be inconsistent. | ||
| if self._get_disable_azure_monitor_metrics(False): | ||
| raise MutuallyExclusiveArgumentError( | ||
| "Cannot specify --enable-control-plane-metrics together with " | ||
| "--disable-azure-monitor-metrics." | ||
| ) | ||
| # Must have Azure Monitor metrics enabled (either already or in this command). | ||
| already_enabled = ( | ||
| self.mc and | ||
| self.mc.azure_monitor_profile and | ||
| self.mc.azure_monitor_profile.metrics and | ||
| self.mc.azure_monitor_profile.metrics.enabled | ||
| ) | ||
| enabling_now = self._get_enable_azure_monitor_metrics(False) | ||
| if not already_enabled and not enabling_now: | ||
| raise RequiredArgumentMissingError( | ||
| "--enable-control-plane-metrics requires Azure Monitor metrics to be enabled. " | ||
| "Specify --enable-azure-monitor-metrics or run on a cluster that already has " | ||
| "Azure Monitor metrics enabled." | ||
| ) | ||
| return enable_control_plane_metrics | ||
|
bragi92 marked this conversation as resolved.
Outdated
|
||
|
|
||
| def get_enable_control_plane_metrics(self) -> bool: | ||
| """Obtain the value of enable_control_plane_metrics. | ||
| This function will verify the parameter by default. If both enable_control_plane_metrics and | ||
| disable_control_plane_metrics are specified, raise a MutuallyExclusiveArgumentError. | ||
| :return: bool | ||
| """ | ||
| return self._get_enable_control_plane_metrics(enable_validation=True) | ||
|
|
||
| def _get_disable_control_plane_metrics(self, enable_validation: bool = False) -> bool: | ||
| """Internal function to obtain the value of disable_control_plane_metrics. | ||
| This function supports the option of enable_validation. When enabled, if both | ||
| enable_control_plane_metrics and disable_control_plane_metrics are specified, raise a | ||
| MutuallyExclusiveArgumentError. | ||
| :return: bool | ||
| """ | ||
| # Read the original value passed by the command. | ||
| disable_control_plane_metrics = self.raw_param.get("disable_control_plane_metrics") | ||
| if enable_validation: | ||
| if disable_control_plane_metrics and self._get_enable_control_plane_metrics(False): | ||
| raise MutuallyExclusiveArgumentError( | ||
| "Cannot specify --enable-control-plane-metrics and --disable-control-plane-metrics " | ||
| "at the same time." | ||
| ) | ||
| return disable_control_plane_metrics | ||
|
|
||
| def get_disable_control_plane_metrics(self) -> bool: | ||
| """Obtain the value of disable_control_plane_metrics. | ||
| This function will verify the parameter by default. If both enable_control_plane_metrics and | ||
| disable_control_plane_metrics are specified, raise a MutuallyExclusiveArgumentError. | ||
| :return: bool | ||
| """ | ||
| return self._get_disable_control_plane_metrics(enable_validation=True) | ||
|
|
||
| def _get_enable_azure_monitor_app_monitoring(self, enable_validation: bool = False) -> bool: | ||
| """Internal function to obtain the value of enable_azure_monitor_app_monitoring. | ||
| This function supports the option of enable_validation. When enabled, if both | ||
|
|
@@ -7714,13 +7799,24 @@ def set_up_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: | |
| ksm_metric_labels_allow_list = "" | ||
| if ksm_metric_annotations_allow_list is None: | ||
| ksm_metric_annotations_allow_list = "" | ||
| # Trigger control-plane-metrics validation even if the parent metrics flag was | ||
| # not specified, so users get a clear error instead of silent ignore when they | ||
| # pass --enable-control-plane-metrics on its own. | ||
| self.context.get_enable_control_plane_metrics() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps you could separate the validation logic from the getter and call the validator here instead, as it's unusual to retrieve a value only to discard it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
| if self.context.get_enable_azure_monitor_metrics(): | ||
| if mc.azure_monitor_profile is None: | ||
| mc.azure_monitor_profile = self.models.ManagedClusterAzureMonitorProfile() | ||
| mc.azure_monitor_profile.metrics = self.models.ManagedClusterAzureMonitorProfileMetrics(enabled=False) | ||
| mc.azure_monitor_profile.metrics.kube_state_metrics = self.models.ManagedClusterAzureMonitorProfileKubeStateMetrics( # pylint:disable=line-too-long | ||
| metric_labels_allowlist=str(ksm_metric_labels_allow_list), | ||
| metric_annotations_allow_list=str(ksm_metric_annotations_allow_list)) | ||
| # NOTE: control_plane.enabled is intentionally NOT set here on the create flow. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach is somewhat unconventional from a client perspective. I've encountered some CRIs who reported that the monitoring addon wasn't functioning as expected when deploying with an ARM template or similar methods. Would it be possible to handle these types of tasks on the server side instead? This allows users on different clients to have a consistent experience.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, agreed. We've had this in out backlog for a while. The logs and prometheus addon team has been looking into it this semester and currently Rashmi has been working on the design for that. Once the logic is moved to the RP, I'll create a PR to remove it from the CLI. |
||
| # If we set it on this initial PUT, the RP would schedule the control-plane-metrics | ||
| # collection pod (CCP) before the DCRA (Data Collection Rule Association) has been | ||
| # created in postprocessing. The CCP would then crash-loop with "DCRA not found" | ||
| # until the next reconciliation. Instead, we defer the flip to the addon_put step | ||
| # inside link_azure_monitor_profile_artifacts (postprocessing_after_mc_created), | ||
| # which runs *after* DCRA creation. | ||
| # set intermediate | ||
| self.context.set_intermediate("azuremonitormetrics_addon_enabled", True, overwrite_exists=True) | ||
| if self.context.get_enable_azure_monitor_app_monitoring(): | ||
|
|
@@ -9810,6 +9906,30 @@ def update_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: | |
| self.context.get_disable_azure_monitor_metrics(), | ||
| False) | ||
|
|
||
| # Handle enable / disable of control plane metrics independently of the parent metrics flag, | ||
| # so users can toggle control plane metrics on a cluster that already has metrics enabled. | ||
| if self.context.get_enable_control_plane_metrics(): | ||
| if mc.azure_monitor_profile is None: | ||
| mc.azure_monitor_profile = self.models.ManagedClusterAzureMonitorProfile() | ||
| if mc.azure_monitor_profile.metrics is None: | ||
| # Should not normally happen — validation requires metrics to be enabled — but guard | ||
| # against partially-populated profiles to avoid AttributeError. | ||
| mc.azure_monitor_profile.metrics = ( | ||
| self.models.ManagedClusterAzureMonitorProfileMetrics(enabled=True) | ||
| ) | ||
| mc.azure_monitor_profile.metrics.control_plane = ( | ||
| self.models.ManagedClusterAzureMonitorProfileMetricsControlPlane(enabled=True) | ||
| ) | ||
|
|
||
| if self.context.get_disable_control_plane_metrics(): | ||
| if ( | ||
| mc.azure_monitor_profile and | ||
| mc.azure_monitor_profile.metrics | ||
| ): | ||
| mc.azure_monitor_profile.metrics.control_plane = ( | ||
| self.models.ManagedClusterAzureMonitorProfileMetricsControlPlane(enabled=False) | ||
| ) | ||
|
|
||
| if self.context.get_enable_azure_monitor_app_monitoring(): | ||
| if mc.azure_monitor_profile is None: | ||
| mc.azure_monitor_profile = self.models.ManagedClusterAzureMonitorProfile() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.