Skip to content

Commit d67a6a4

Browse files
bragi92Copilot
andcommitted
Separate control-plane-metrics validation from the getters
Per FumingZhang review feedback on Azure/azure-cli#33537: calling get_enable_control_plane_metrics() purely to trigger validation and discarding the return value is a confusing pattern. Extract the validation block into a new private _validate_control_plane_metrics_params method, expose a public validate_control_plane_metrics_params, and have the getters delegate to it when enable_validation=True (preserves existing API). The two _setup_azure_monitor_profile call sites now call the validator directly instead of discarding a getter result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent efde8eb commit d67a6a4

1 file changed

Lines changed: 73 additions & 46 deletions

File tree

src/aks-preview/azext_aks_preview/managed_cluster_decorator.py

Lines changed: 73 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,13 +2651,70 @@ def get_disable_azure_monitor_metrics(self) -> bool:
26512651
"""
26522652
return self._get_disable_azure_monitor_metrics(enable_validation=True)
26532653

2654+
def _validate_control_plane_metrics_params(self) -> None:
2655+
"""Validate the --enable/--disable-control-plane-metrics flag combo and
2656+
its interaction with --enable/--disable-azure-monitor-metrics.
2657+
2658+
Raises MutuallyExclusiveArgumentError or RequiredArgumentMissingError on
2659+
an invalid combination. Returns nothing — use this when you want to
2660+
surface validation errors without consuming a parameter value.
2661+
2662+
Reads raw_param directly to avoid recursing back into the getters that
2663+
themselves delegate here when enable_validation is True.
2664+
"""
2665+
enable_cp = self.raw_param.get("enable_control_plane_metrics")
2666+
disable_cp = self.raw_param.get("disable_control_plane_metrics")
2667+
# On create, the property may already be set on the incoming mc object.
2668+
if self.decorator_mode == DecoratorMode.CREATE:
2669+
if (
2670+
self.mc and
2671+
self.mc.azure_monitor_profile and
2672+
self.mc.azure_monitor_profile.metrics and
2673+
self.mc.azure_monitor_profile.metrics.control_plane
2674+
):
2675+
enable_cp = self.mc.azure_monitor_profile.metrics.control_plane.enabled
2676+
2677+
if enable_cp and disable_cp:
2678+
raise MutuallyExclusiveArgumentError(
2679+
"Cannot specify --enable-control-plane-metrics and --disable-control-plane-metrics "
2680+
"at the same time."
2681+
)
2682+
2683+
if enable_cp:
2684+
# Reject combining enable-control-plane-metrics with disable-azure-monitor-metrics
2685+
# in the same command — the resulting payload would be inconsistent.
2686+
if self._get_disable_azure_monitor_metrics(False):
2687+
raise MutuallyExclusiveArgumentError(
2688+
"Cannot specify --enable-control-plane-metrics together with "
2689+
"--disable-azure-monitor-metrics."
2690+
)
2691+
# Must have Azure Monitor metrics enabled (either already or in this command).
2692+
already_enabled = (
2693+
self.mc and
2694+
self.mc.azure_monitor_profile and
2695+
self.mc.azure_monitor_profile.metrics and
2696+
self.mc.azure_monitor_profile.metrics.enabled
2697+
)
2698+
enabling_now = self._get_enable_azure_monitor_metrics(False)
2699+
if not already_enabled and not enabling_now:
2700+
raise RequiredArgumentMissingError(
2701+
"--enable-control-plane-metrics requires Azure Monitor metrics to be enabled. "
2702+
"Specify --enable-azure-monitor-metrics or run on a cluster that already has "
2703+
"Azure Monitor metrics enabled."
2704+
)
2705+
2706+
def validate_control_plane_metrics_params(self) -> None:
2707+
"""Public entry-point for validating the control-plane-metrics flag combo
2708+
without consuming a parameter value. Useful in code paths that need to
2709+
surface validation errors even when the parent --enable-azure-monitor-metrics
2710+
flag was not passed.
2711+
"""
2712+
self._validate_control_plane_metrics_params()
2713+
26542714
def _get_enable_control_plane_metrics(self, enable_validation: bool = False) -> bool:
26552715
"""Internal function to obtain the value of enable_control_plane_metrics.
2656-
This function supports the option of enable_validation. When enabled, if both
2657-
enable_control_plane_metrics and disable_control_plane_metrics are specified, raise a
2658-
MutuallyExclusiveArgumentError. Additionally, --enable-control-plane-metrics requires Azure
2659-
Monitor metrics to either already be enabled on the cluster or to be enabled in the same
2660-
command via --enable-azure-monitor-metrics.
2716+
When enable_validation is True, the flag combinations are validated via
2717+
_validate_control_plane_metrics_params before the value is returned.
26612718
26622719
:return: bool
26632720
"""
@@ -2672,35 +2729,8 @@ def _get_enable_control_plane_metrics(self, enable_validation: bool = False) ->
26722729
self.mc.azure_monitor_profile.metrics.control_plane
26732730
):
26742731
enable_control_plane_metrics = self.mc.azure_monitor_profile.metrics.control_plane.enabled
2675-
# This parameter does not need dynamic completion.
26762732
if enable_validation:
2677-
if enable_control_plane_metrics and self._get_disable_control_plane_metrics(False):
2678-
raise MutuallyExclusiveArgumentError(
2679-
"Cannot specify --enable-control-plane-metrics and --disable-control-plane-metrics "
2680-
"at the same time."
2681-
)
2682-
if enable_control_plane_metrics:
2683-
# Reject combining enable-control-plane-metrics with disable-azure-monitor-metrics
2684-
# in the same command — the resulting payload would be inconsistent.
2685-
if self._get_disable_azure_monitor_metrics(False):
2686-
raise MutuallyExclusiveArgumentError(
2687-
"Cannot specify --enable-control-plane-metrics together with "
2688-
"--disable-azure-monitor-metrics."
2689-
)
2690-
# Must have Azure Monitor metrics enabled (either already or in this command).
2691-
already_enabled = (
2692-
self.mc and
2693-
self.mc.azure_monitor_profile and
2694-
self.mc.azure_monitor_profile.metrics and
2695-
self.mc.azure_monitor_profile.metrics.enabled
2696-
)
2697-
enabling_now = self._get_enable_azure_monitor_metrics(False)
2698-
if not already_enabled and not enabling_now:
2699-
raise RequiredArgumentMissingError(
2700-
"--enable-control-plane-metrics requires Azure Monitor metrics to be enabled. "
2701-
"Specify --enable-azure-monitor-metrics or run on a cluster that already has "
2702-
"Azure Monitor metrics enabled."
2703-
)
2733+
self._validate_control_plane_metrics_params()
27042734
return bool(enable_control_plane_metrics)
27052735

27062736
def get_enable_control_plane_metrics(self) -> bool:
@@ -2713,19 +2743,15 @@ def get_enable_control_plane_metrics(self) -> bool:
27132743

27142744
def _get_disable_control_plane_metrics(self, enable_validation: bool = False) -> bool:
27152745
"""Internal function to obtain the value of disable_control_plane_metrics.
2716-
This function supports the option of enable_validation. When enabled, if both
2717-
enable_control_plane_metrics and disable_control_plane_metrics are specified, raise a
2718-
MutuallyExclusiveArgumentError.
2746+
When enable_validation is True, the flag combinations are validated via
2747+
_validate_control_plane_metrics_params before the value is returned.
2748+
27192749
:return: bool
27202750
"""
27212751
# Read the original value passed by the command.
27222752
disable_control_plane_metrics = self.raw_param.get("disable_control_plane_metrics")
27232753
if enable_validation:
2724-
if disable_control_plane_metrics and self._get_enable_control_plane_metrics(False):
2725-
raise MutuallyExclusiveArgumentError(
2726-
"Cannot specify --enable-control-plane-metrics and --disable-control-plane-metrics "
2727-
"at the same time."
2728-
)
2754+
self._validate_control_plane_metrics_params()
27292755
return bool(disable_control_plane_metrics)
27302756

27312757
def get_disable_control_plane_metrics(self) -> bool:
@@ -4788,7 +4814,7 @@ def _setup_azure_monitor_metrics(self, mc: ManagedCluster) -> None:
47884814
# link_azure_monitor_profile_artifacts (postprocessing_after_mc_created), which
47894815
# runs *after* DCRA creation. The validator still runs here to surface flag
47904816
# combination errors early.
4791-
self.context.get_enable_control_plane_metrics()
4817+
self.context.validate_control_plane_metrics_params()
47924818

47934819
self.context.set_intermediate("azuremonitormetrics_addon_enabled", True, overwrite_exists=True)
47944820

@@ -4909,10 +4935,11 @@ def set_up_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster:
49094935
"""
49104936
self._ensure_mc(mc)
49114937

4912-
# Trigger control-plane-metrics validation even if the parent metrics flag was
4913-
# not specified, so users get a clear error instead of silent ignore when they
4914-
# pass --enable-control-plane-metrics on its own.
4915-
self.context.get_enable_control_plane_metrics()
4938+
# Surface control-plane-metrics flag combination errors even when the
4939+
# parent metrics flag was not specified, so users get a clear error
4940+
# instead of a silent ignore when they pass --enable-control-plane-metrics
4941+
# on its own.
4942+
self.context.validate_control_plane_metrics_params()
49164943

49174944
if self.context.get_enable_azure_monitor_metrics():
49184945
self._setup_azure_monitor_metrics(mc)

0 commit comments

Comments
 (0)