Skip to content

Commit ac1b206

Browse files
carlotaarvelacarlotaarvela
committed
PR comments
1 parent f7fef4e commit ac1b206

File tree

3 files changed

+162
-4
lines changed

3 files changed

+162
-4
lines changed

‎src/aks-preview/azext_aks_preview/custom.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ def __init__(self, location, resource_id):
553553
)
554554

555555
resources = get_resources_client(cmd.cli_ctx, cluster_subscription)
556-
for attempt in range(3):
556+
for _ in range(3):
557557
try:
558558
if enable_syslog:
559559
resources.begin_create_or_update_by_id(
@@ -571,9 +571,6 @@ def __init__(self, location, resource_id):
571571
break
572572
except (CLIError, HttpResponseError) as e:
573573
error = e
574-
# Wait before retry to allow workspace tables to become available
575-
if attempt < 2:
576-
time.sleep(30)
577574
else:
578575
raise error
579576

‎src/aks-preview/azext_aks_preview/managed_cluster_decorator.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7836,6 +7836,13 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None:
78367836

78377837
if existing_key:
78387838
addon_profile = mc.addon_profiles[existing_key]
7839+
# Detect workspace change: if the workspace is different from the existing one,
7840+
# trigger DCR postprocessing so the DCR destination gets updated.
7841+
old_config = addon_profile.config or {}
7842+
old_workspace = old_config.get(CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, "")
7843+
if old_workspace and old_workspace.lower() != workspace_resource_id.lower():
7844+
self.context.set_intermediate(
7845+
"monitoring_addon_postprocessing_required", True, overwrite_exists=True)
78397846
else:
78407847
addon_profile = self.models.ManagedClusterAddonProfile(enabled=False)
78417848
existing_key = CONST_MONITORING_ADDON_NAME

‎src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16340,6 +16340,160 @@ def test_setup_azure_monitor_logs_no_retina_flags_without_cnl(self):
1634016340
self.assertIsNotNone(addon_profile)
1634116341
self.assertNotIn("enableRetinaNetworkFlags", addon_profile.config)
1634216342

16343+
# ------------------------------------------------------------------
16344+
# Tests for _setup_azure_monitor_logs workspace change detection
16345+
# ------------------------------------------------------------------
16346+
def test_setup_azure_monitor_logs_workspace_change_triggers_postprocessing(self):
16347+
"""_setup_azure_monitor_logs sets monitoring_addon_postprocessing_required when workspace changes."""
16348+
old_ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/old-ws"
16349+
new_ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/new-ws"
16350+
dec = AKSPreviewManagedClusterUpdateDecorator(
16351+
self.cmd,
16352+
self.client,
16353+
{
16354+
"enable_azure_monitor_logs": True,
16355+
"workspace_resource_id": new_ws,
16356+
},
16357+
CUSTOM_MGMT_AKS_PREVIEW,
16358+
)
16359+
mc = self.models.ManagedCluster(
16360+
location="test_location",
16361+
addon_profiles={
16362+
CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile(
16363+
enabled=True,
16364+
config={
16365+
"logAnalyticsWorkspaceResourceID": old_ws,
16366+
"useAADAuth": "true",
16367+
},
16368+
)
16369+
},
16370+
)
16371+
dec.context.attach_mc(mc)
16372+
dec.context.set_intermediate("subscription_id", "test-subscription-id")
16373+
16374+
with patch.object(
16375+
dec.context.external_functions,
16376+
"sanitize_loganalytics_ws_resource_id",
16377+
side_effect=lambda x: x,
16378+
):
16379+
dec._setup_azure_monitor_logs(mc)
16380+
16381+
self.assertTrue(
16382+
dec.context.get_intermediate("monitoring_addon_postprocessing_required", default_value=False)
16383+
)
16384+
# Verify workspace was updated
16385+
actual_key = next(k for k in mc.addon_profiles if k.lower() == "omsagent")
16386+
self.assertEqual(mc.addon_profiles[actual_key].config["logAnalyticsWorkspaceResourceID"], new_ws)
16387+
16388+
def test_setup_azure_monitor_logs_same_workspace_no_postprocessing(self):
16389+
"""_setup_azure_monitor_logs does NOT set monitoring_addon_postprocessing_required when workspace is unchanged."""
16390+
ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/same-ws"
16391+
dec = AKSPreviewManagedClusterUpdateDecorator(
16392+
self.cmd,
16393+
self.client,
16394+
{
16395+
"enable_azure_monitor_logs": True,
16396+
"workspace_resource_id": ws,
16397+
},
16398+
CUSTOM_MGMT_AKS_PREVIEW,
16399+
)
16400+
mc = self.models.ManagedCluster(
16401+
location="test_location",
16402+
addon_profiles={
16403+
CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile(
16404+
enabled=True,
16405+
config={
16406+
"logAnalyticsWorkspaceResourceID": ws,
16407+
"useAADAuth": "true",
16408+
},
16409+
)
16410+
},
16411+
)
16412+
dec.context.attach_mc(mc)
16413+
dec.context.set_intermediate("subscription_id", "test-subscription-id")
16414+
16415+
with patch.object(
16416+
dec.context.external_functions,
16417+
"sanitize_loganalytics_ws_resource_id",
16418+
side_effect=lambda x: x,
16419+
):
16420+
dec._setup_azure_monitor_logs(mc)
16421+
16422+
self.assertFalse(
16423+
dec.context.get_intermediate("monitoring_addon_postprocessing_required", default_value=False)
16424+
)
16425+
16426+
def test_setup_azure_monitor_logs_workspace_change_case_insensitive(self):
16427+
"""_setup_azure_monitor_logs compares workspaces case-insensitively (no false positives on casing)."""
16428+
ws_lower = "/subscriptions/test/resourcegroups/test/providers/microsoft.operationalinsights/workspaces/my-ws"
16429+
ws_mixed = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/my-ws"
16430+
dec = AKSPreviewManagedClusterUpdateDecorator(
16431+
self.cmd,
16432+
self.client,
16433+
{
16434+
"enable_azure_monitor_logs": True,
16435+
"workspace_resource_id": ws_mixed,
16436+
},
16437+
CUSTOM_MGMT_AKS_PREVIEW,
16438+
)
16439+
mc = self.models.ManagedCluster(
16440+
location="test_location",
16441+
addon_profiles={
16442+
CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile(
16443+
enabled=True,
16444+
config={
16445+
"logAnalyticsWorkspaceResourceID": ws_lower,
16446+
"useAADAuth": "true",
16447+
},
16448+
)
16449+
},
16450+
)
16451+
dec.context.attach_mc(mc)
16452+
dec.context.set_intermediate("subscription_id", "test-subscription-id")
16453+
16454+
with patch.object(
16455+
dec.context.external_functions,
16456+
"sanitize_loganalytics_ws_resource_id",
16457+
side_effect=lambda x: x,
16458+
):
16459+
dec._setup_azure_monitor_logs(mc)
16460+
16461+
# Same workspace (different casing) should NOT trigger postprocessing
16462+
self.assertFalse(
16463+
dec.context.get_intermediate("monitoring_addon_postprocessing_required", default_value=False)
16464+
)
16465+
16466+
def test_setup_azure_monitor_logs_new_addon_no_postprocessing(self):
16467+
"""_setup_azure_monitor_logs does NOT trigger postprocessing when there is no existing addon (fresh enable)."""
16468+
new_ws = "/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/new-ws"
16469+
dec = AKSPreviewManagedClusterUpdateDecorator(
16470+
self.cmd,
16471+
self.client,
16472+
{
16473+
"enable_azure_monitor_logs": True,
16474+
"workspace_resource_id": new_ws,
16475+
},
16476+
CUSTOM_MGMT_AKS_PREVIEW,
16477+
)
16478+
mc = self.models.ManagedCluster(
16479+
location="test_location",
16480+
addon_profiles={},
16481+
)
16482+
dec.context.attach_mc(mc)
16483+
dec.context.set_intermediate("subscription_id", "test-subscription-id")
16484+
16485+
with patch.object(
16486+
dec.context.external_functions,
16487+
"sanitize_loganalytics_ws_resource_id",
16488+
side_effect=lambda x: x,
16489+
):
16490+
dec._setup_azure_monitor_logs(mc)
16491+
16492+
# Fresh enable — no old workspace to compare, should NOT trigger postprocessing
16493+
self.assertFalse(
16494+
dec.context.get_intermediate("monitoring_addon_postprocessing_required", default_value=False)
16495+
)
16496+
1634316497
# ------------------------------------------------------------------
1634416498
# Tests for _disable_azure_monitor_logs disabling containerInsights
1634516499
# ------------------------------------------------------------------

0 commit comments

Comments
 (0)