-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[AKS] aks-preview: add --enable/--disable-control-plane-metrics #9855
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
base: main
Are you sure you want to change the base?
Changes from all commits
30532b7
e3c6dff
a7b923c
9cda233
6101e76
3f343fe
552ef2c
620006a
88a3e2e
5fe4614
b2a70ba
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 |
|---|---|---|
|
|
@@ -2596,6 +2596,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." | ||
| ) | ||
| 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 | ||
|
|
||
| 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=True) -> 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 | ||
|
|
@@ -4622,6 +4707,13 @@ def _setup_azure_monitor_metrics(self, mc: ManagedCluster) -> None: | |
| metric_annotations_allow_list=str(ksm_metric_annotations_allow_list) | ||
| )) | ||
| mc.azure_monitor_profile.metrics.kube_state_metrics = kube_state_metrics | ||
|
|
||
| # Enable control plane metrics if requested. | ||
| if self.context.get_enable_control_plane_metrics(): | ||
| mc.azure_monitor_profile.metrics.control_plane = ( | ||
| self.models.ManagedClusterAzureMonitorProfileMetricsControlPlane(enabled=True) | ||
| ) | ||
|
|
||
|
Comment on lines
+4711
to
+4716
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. Fixed in a7b923c — set_up_azure_monitor_profile now calls get_enable_control_plane_metrics() unconditionally so passing --enable-control-plane-metrics without the parent flag raises RequiredArgumentMissingError on create instead of being silently ignored when _setup_azure_monitor_metrics is skipped. |
||
| self.context.set_intermediate("azuremonitormetrics_addon_enabled", True, overwrite_exists=True) | ||
|
|
||
| def _setup_azure_monitor_app_monitoring(self, mc: ManagedCluster) -> None: | ||
|
|
@@ -4741,6 +4833,11 @@ def set_up_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: | |
| """ | ||
| self._ensure_mc(mc) | ||
|
|
||
| # 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() | ||
|
|
||
| if self.context.get_enable_azure_monitor_metrics(): | ||
| self._setup_azure_monitor_metrics(mc) | ||
|
|
||
|
|
@@ -6908,6 +7005,30 @@ def update_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: | |
| if self.context.get_disable_azure_monitor_metrics(): | ||
| self._disable_azure_monitor_metrics(mc) | ||
|
|
||
| # 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() # pylint: disable=no-member | ||
| 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) # pylint: disable=no-member | ||
| ) | ||
| mc.azure_monitor_profile.metrics.control_plane = ( | ||
| self.models.ManagedClusterAzureMonitorProfileMetricsControlPlane(enabled=True) # pylint: disable=no-member | ||
| ) | ||
|
|
||
| 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) # pylint: disable=no-member | ||
| ) | ||
|
Comment on lines
+7008
to
+7030
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. Fixed in 3f343fe — added 5 update-decorator tests covering: (1) enable-cp without parent flag raises RequiredArgumentMissingError, (2) enable-cp succeeds when AM metrics already on cluster, (3) enable-cp + disable-am-metrics raises MutuallyExclusiveArgumentError, (4) enable-cp + disable-cp together raises MutuallyExclusiveArgumentError, (5) disable-cp writes control_plane.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.