From 6823f49cee016f40558a65b06e993d7ff17f9e2e Mon Sep 17 00:00:00 2001 From: bragi92 Date: Mon, 12 May 2025 13:42:51 -0700 Subject: [PATCH 1/5] fix: simplify logic and enable correct recording rule groups for managed prom (#2) * fix: simplify logic and enable correct recording rule groups for managed prom * Update create.py * Update delete.py --- .../recordingrules/create.py | 87 ++++++------------- .../recordingrules/delete.py | 12 +++ 2 files changed, 40 insertions(+), 59 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py index 3b2868b78ec..84793fc338c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py @@ -61,69 +61,38 @@ def put_rules(cmd, default_rule_group_id, default_rule_group_name, mac_region, c # pylint: disable=line-too-long def create_rules(cmd, cluster_subscription, cluster_resource_group_name, cluster_name, azure_monitor_workspace_resource_id, mac_region, raw_parameters): - # limit rule group name to 260 characters - # with urllib.request.urlopen("https://defaultrulessc.blob.core.windows.net/defaultrules/ManagedPrometheusDefaultRecordingRules.json") as url: - # default_rules_template = json.loads(url.read().decode()) default_rules_template = get_recording_rules_template(cmd, azure_monitor_workspace_resource_id) - default_rule_group_name = truncate_rule_group_name("{0}-{1}".format(default_rules_template[0]["name"], cluster_name)) - default_rule_group_id = "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{2}".format( - cluster_subscription, - cluster_resource_group_name, - default_rule_group_name - ) - cluster_resource_id = \ - "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.ContainerService/managedClusters/{2}".format( - cluster_subscription, - cluster_resource_group_name, - cluster_name - ) - url = "{0}{1}?api-version={2}".format( - cmd.cli_ctx.cloud.endpoints.resource_manager, - default_rule_group_id, - RULES_API - ) - put_rules(cmd, default_rule_group_id, default_rule_group_name, mac_region, cluster_resource_id, azure_monitor_workspace_resource_id, cluster_name, default_rules_template, url, True, 0) - default_rule_group_name = truncate_rule_group_name("{0}-{1}".format(default_rules_template[1]["name"], cluster_name)) - default_rule_group_id = "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{2}".format( - cluster_subscription, - cluster_resource_group_name, - default_rule_group_name + cluster_resource_id = ( + f"/subscriptions/{cluster_subscription}/resourceGroups/{cluster_resource_group_name}/providers/Microsoft.ContainerService/managedClusters/{cluster_name}" ) - url = "{0}{1}?api-version={2}".format( - cmd.cli_ctx.cloud.endpoints.resource_manager, - default_rule_group_id, - RULES_API - ) - put_rules(cmd, default_rule_group_id, default_rule_group_name, mac_region, cluster_resource_id, azure_monitor_workspace_resource_id, cluster_name, default_rules_template, url, True, 1) - enable_windows_recording_rules = raw_parameters.get("enable_windows_recording_rules") + enable_windows_recording_rules = raw_parameters.get("enable_windows_recording_rules", False) - if enable_windows_recording_rules is not True: - enable_windows_recording_rules = False + for index, rule_template in enumerate(default_rules_template): + rule_name = rule_template["name"] + is_windows_rule = "win" in rule_name.lower() - default_rule_group_name = truncate_rule_group_name("{0}-{1}".format(default_rules_template[2]["name"], cluster_name)) - default_rule_group_id = "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{2}".format( - cluster_subscription, - cluster_resource_group_name, - default_rule_group_name - ) - url = "{0}{1}?api-version={2}".format( - cmd.cli_ctx.cloud.endpoints.resource_manager, - default_rule_group_id, - RULES_API - ) - put_rules(cmd, default_rule_group_id, default_rule_group_name, mac_region, cluster_resource_id, azure_monitor_workspace_resource_id, cluster_name, default_rules_template, url, enable_windows_recording_rules, 2) + # Determine whether the rule group should be enabled: + # - If the rule is a Windows rule AND windows recording rules are NOT enabled → disable the rule group (enable_rules = False) + # - If the rule is a Windows rule AND windows recording rules are enabled → enable the rule group (enable_rules = True) + # - If the rule is NOT a Windows rule (i.e., a Linux or general rule) → always enable the rule group (enable_rules = True) + enable_rules = not (is_windows_rule and not enable_windows_recording_rules) + + rule_group_name = truncate_rule_group_name(f"{rule_template['name']}-{cluster_name}") + rule_group_id = f"/subscriptions/{cluster_subscription}/resourceGroups/{cluster_resource_group_name}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{rule_group_name}" + url = f"{cmd.cli_ctx.cloud.endpoints.resource_manager}{rule_group_id}?api-version={RULES_API}" - default_rule_group_name = truncate_rule_group_name("{0}-{1}".format(default_rules_template[3]["name"], cluster_name)) - default_rule_group_id = "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{2}".format( - cluster_subscription, - cluster_resource_group_name, - default_rule_group_name - ) - url = "{0}{1}?api-version={2}".format( - cmd.cli_ctx.cloud.endpoints.resource_manager, - default_rule_group_id, - RULES_API - ) - put_rules(cmd, default_rule_group_id, default_rule_group_name, mac_region, cluster_resource_id, azure_monitor_workspace_resource_id, cluster_name, default_rules_template, url, enable_windows_recording_rules, 3) + put_rules( + cmd, + rule_group_id, + rule_group_name, + mac_region, + cluster_resource_id, + azure_monitor_workspace_resource_id, + cluster_name, + default_rules_template, + url, + enable_rules, + index + ) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/delete.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/delete.py index d12ac5d5643..50e495f7f55 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/delete.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/delete.py @@ -49,3 +49,15 @@ def delete_rules(cmd, cluster_subscription, cluster_resource_group_name, cluster cluster_resource_group_name, truncate_rule_group_name("NodeAndKubernetesRecordingRulesRuleGroup-Win-{0}".format(cluster_name)) ) + delete_rule( + cmd, + cluster_subscription, + cluster_resource_group_name, + truncate_rule_group_name("UXRecordingRulesRuleGroup - {0}".format(cluster_name)) + ) + delete_rule( + cmd, + cluster_subscription, + cluster_resource_group_name, + truncate_rule_group_name("UXRecordingRulesRuleGroup-Win - {0}".format(cluster_name)) + ) From bb3d33cd87119423430455000edcb764aba3e43a Mon Sep 17 00:00:00 2001 From: Kaveesh Dubey Date: Tue, 27 May 2025 11:07:41 -0700 Subject: [PATCH 2/5] add unit tests --- .../recordingrules/tests/test_create.py | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py new file mode 100644 index 00000000000..5f6f57c2431 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py @@ -0,0 +1,115 @@ +import unittest +from unittest.mock import patch, MagicMock +import json + +from azure.cli.command_modules.acs.azuremonitormetrics.recordingrules.create import ( + create_rules, + get_recording_rules_template +) +from azure.cli.core.util import send_raw_request + + +class TestCreateRulesFunction(unittest.TestCase): + @patch('azure.cli.core.util.send_raw_request') + def test_get_recording_rules_template_filters_valid_templates(self, mock_send): + mock_response = MagicMock() + mock_response.text = json.dumps({ + "value": [ + { + "name": "validRuleGroup", + "properties": { + "alertRuleType": "microsoft.alertsmanagement/prometheusrulegroups", + "rulesArmTemplate": { + "resources": [ + { + "type": "Microsoft.AlertsManagement/prometheusRuleGroups", + "properties": { + "rules": [ + {"record": "cpu_usage", "expression": "some_expr"} + ] + } + } + ] + } + } + }, + { + "name": "invalidRuleGroup", + "properties": { + "alertRuleType": "microsoft.alertsmanagement/somethingelse" + } + } + ] + }) + mock_send.return_value = mock_response + + result = get_recording_rules_template(MagicMock(), "/dummy/resource/id") + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "validRuleGroup") + + + @patch('azure.cli.command_modules.acs.azuremonitormetrics.recordingrules.create.get_recording_rules_template') + @patch('azure.cli.command_modules.acs.azuremonitormetrics.recordingrules.create.put_rules') + def test_create_rules_respects_windows_flag(self, mock_put_rules, mock_get_templates): + mock_get_templates.return_value = [ + { + "name": "NodeRecordingRulesRuleGroup-Win", + "properties": { + "rulesArmTemplate": { + "resources": [ + { + "type": "Microsoft.AlertsManagement/prometheusRuleGroups", + "properties": { + "rules": [{"record": "win_cpu", "expression": "usage"}] + } + } + ] + } + } + }, + { + "name": "KubernetesRecordingRulesRuleGroup", + "properties": { + "rulesArmTemplate": { + "resources": [ + { + "type": "Microsoft.AlertsManagement/prometheusRuleGroups", + "properties": { + "rules": [{"record": "kube_cpu", "expression": "usage"}] + } + } + ] + } + } + } + ] + + cmd = MagicMock() + cmd.cli_ctx.cloud.endpoints.resource_manager = "https://management.azure.com/" + raw_parameters = {"enable_windows_recording_rules": False} + + create_rules( + cmd, + cluster_subscription="sub123", + cluster_resource_group_name="rg1", + cluster_name="mycluster", + azure_monitor_workspace_resource_id="/dummy/ampls", + mac_region="eastus", + raw_parameters=raw_parameters + ) + + # Check put_rules was called for both rule groups + self.assertEqual(mock_put_rules.call_count, 2) + + for call in mock_put_rules.call_args_list: + rule_group_name = call.args[2] # 3rd positional argument + enable_rules = call.args[9] # 10th positional argument + + if "Win" in rule_group_name: + self.assertFalse(enable_rules, f"Expected enable_rules=False for {rule_group_name}") + else: + self.assertTrue(enable_rules, f"Expected enable_rules=True for {rule_group_name}") + + +if __name__ == '__main__': + unittest.main() From 903a6ef1ad035c6ac641756da5a4ff111ba41f3a Mon Sep 17 00:00:00 2001 From: Kaveesh Dubey Date: Tue, 27 May 2025 11:14:36 -0700 Subject: [PATCH 3/5] more delete calls --- .../acs/azuremonitormetrics/recordingrules/tests/test_delete.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_delete.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_delete.py index 32ac96815b5..10cdb920f5c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_delete.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_delete.py @@ -19,7 +19,7 @@ def test_delete_rules(self, mock_delete_rule): delete_rules(cmd, cluster_subscription, cluster_resource_group_name, cluster_name) # Assertions - self.assertEqual(mock_delete_rule.call_count, 4) # Ensure delete_rule is called 4 times with different arguments + self.assertEqual(mock_delete_rule.call_count, 6) # Ensure delete_rule is called 6 times with different arguments if __name__ == '__main__': From ce08f1fcc6bcf0efc32ede7c66ec3145772f0bd0 Mon Sep 17 00:00:00 2001 From: Kaveesh Dubey Date: Tue, 27 May 2025 11:15:24 -0700 Subject: [PATCH 4/5] remove white space from blank line --- .../acs/azuremonitormetrics/recordingrules/create.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py index 84793fc338c..78544002268 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/create.py @@ -78,7 +78,7 @@ def create_rules(cmd, cluster_subscription, cluster_resource_group_name, cluster # - If the rule is a Windows rule AND windows recording rules are enabled → enable the rule group (enable_rules = True) # - If the rule is NOT a Windows rule (i.e., a Linux or general rule) → always enable the rule group (enable_rules = True) enable_rules = not (is_windows_rule and not enable_windows_recording_rules) - + rule_group_name = truncate_rule_group_name(f"{rule_template['name']}-{cluster_name}") rule_group_id = f"/subscriptions/{cluster_subscription}/resourceGroups/{cluster_resource_group_name}/providers/Microsoft.AlertsManagement/prometheusRuleGroups/{rule_group_name}" url = f"{cmd.cli_ctx.cloud.endpoints.resource_manager}{rule_group_id}?api-version={RULES_API}" From 807ea67391a58a6f3be0825d422a99aad4ca86be Mon Sep 17 00:00:00 2001 From: Kaveesh Dubey Date: Wed, 28 May 2025 11:45:38 -0700 Subject: [PATCH 5/5] missed license header --- .../azuremonitormetrics/recordingrules/tests/test_create.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py index 5f6f57c2431..ac536d8ed21 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/recordingrules/tests/test_create.py @@ -1,3 +1,7 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- import unittest from unittest.mock import patch, MagicMock import json