Skip to content

Commit e8747b5

Browse files
Jenny LiuJenny Liu
authored andcommitted
Fix operator precedence bug in get_container_network_logs() validation
The validation logic had two issues: 1. Operator precedence bug - the condition evaluated as (enable_cnl AND acns_not_enabled) OR monitoring_not_enabled which would error if monitoring wasn't configured, regardless of enable_cnl 2. Timing issue during cluster create - the validation only checked mc.addon_profiles, but during create this is empty even when --enable-addons monitoring is specified Fix: Check both mc.addon_profiles (for update) AND --enable-addons parameter (for create), with correct operator precedence.
1 parent 158e302 commit e8747b5

File tree

1 file changed

+25
-10
lines changed

1 file changed

+25
-10
lines changed

src/aks-preview/azext_aks_preview/managed_cluster_decorator.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -941,17 +941,32 @@ def get_container_network_logs(self, mc: ManagedCluster) -> Union[bool, None]:
941941
"Cannot specify --enable-container-network-logs and "
942942
"--disable-container-network-logs at the same time."
943943
)
944-
if (
945-
enable_cnl and
946-
(not self.raw_param.get("enable_acns", False) and
947-
not (mc.network_profile and mc.network_profile.advanced_networking and
948-
mc.network_profile.advanced_networking.enabled)) or
949-
not (mc.addon_profiles and mc.addon_profiles.get("omsagent") and mc.addon_profiles["omsagent"].enabled)
950-
):
951-
raise InvalidArgumentValueError(
952-
"Container network logs requires '--enable-acns', advanced networking "
953-
"to be enabled, and the monitoring addon to be enabled."
944+
945+
if enable_cnl:
946+
# Check if ACNS is enabled (either via param or already on the cluster)
947+
acns_enabled = (
948+
self.raw_param.get("enable_acns", False) or
949+
(mc.network_profile and mc.network_profile.advanced_networking and
950+
mc.network_profile.advanced_networking.enabled)
951+
)
952+
953+
# Check if monitoring addon is enabled:
954+
# - Already configured in mc.addon_profiles, OR
955+
# - Being enabled via --enable-addons parameter (for cluster create scenario)
956+
monitoring_already_enabled = (
957+
mc.addon_profiles and
958+
mc.addon_profiles.get("omsagent") and
959+
mc.addon_profiles["omsagent"].enabled
954960
)
961+
monitoring_being_enabled = "monitoring" in (self.raw_param.get("enable_addons") or "")
962+
monitoring_enabled = monitoring_already_enabled or monitoring_being_enabled
963+
964+
if not acns_enabled or not monitoring_enabled:
965+
raise InvalidArgumentValueError(
966+
"Container network logs requires '--enable-acns', advanced networking "
967+
"to be enabled, and the monitoring addon to be enabled."
968+
)
969+
955970
enable_cnl = bool(enable_cnl) if enable_cnl is not None else False
956971
disable_cnl = bool(disable_cnl) if disable_cnl is not None else False
957972
return enable_cnl or not disable_cnl

0 commit comments

Comments
 (0)