Skip to content

Commit 0c20eba

Browse files
carlotaarvelacarlotaarvela
committed
PR comments
1 parent f81732b commit 0c20eba

3 files changed

Lines changed: 114 additions & 114 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
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(

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

Lines changed: 73 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ def _get_monitoring_addon_key_from_consts(addon_profiles, addon_consts):
180180
addon_consts.get("CONST_MONITORING_ADDON_NAME"),
181181
)
182182

183-
184183
# pylint: disable=too-few-public-methods
185184
class AKSPreviewManagedClusterModels(AKSManagedClusterModels):
186185
"""Store the models used in aks series of commands.
@@ -4679,8 +4678,8 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None:
46794678
mc.addon_profiles[CONST_MONITORING_ADDON_NAME] = addon_profile
46804679

46814680
# DCR and DCRA creation is deferred to postprocessing_after_mc_created
4682-
# (_postprocess_monitoring_enable) so that all flags are finalized and
4683-
# the cluster exists. Only MSI clusters need a DCR.
4681+
# so that all flags are finalized and the cluster exists.
4682+
# Only MSI clusters need a DCR.
46844683
self.context.set_intermediate("monitoring_addon_enabled", True, overwrite_exists=True)
46854684

46864685
def _setup_opentelemetry_metrics(self, mc: ManagedCluster) -> None:
@@ -5318,15 +5317,83 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
53185317
# monitoring addon
53195318
monitoring_addon_enabled = self.context.get_intermediate("monitoring_addon_enabled", default_value=False)
53205319
if monitoring_addon_enabled:
5321-
self._postprocess_monitoring_enable(cluster)
5320+
enable_msi_auth_for_monitoring = self.context.get_enable_msi_auth_for_monitoring()
5321+
if not enable_msi_auth_for_monitoring:
5322+
# add cluster spn/msi Monitoring Metrics Publisher role assignment to publish metrics to MDM
5323+
# mdm metrics is supported only in azure public cloud, so add the role assignment only in this cloud
5324+
cloud_name = self.cmd.cli_ctx.cloud.name
5325+
if cloud_name.lower() == "azurecloud":
5326+
cluster_resource_id = resource_id(
5327+
subscription=self.context.get_subscription_id(),
5328+
resource_group=self.context.get_resource_group_name(),
5329+
namespace="Microsoft.ContainerService",
5330+
type="managedClusters",
5331+
name=self.context.get_name(),
5332+
)
5333+
self.context.external_functions.add_monitoring_role_assignment(
5334+
cluster, cluster_resource_id, self.cmd
5335+
)
5336+
elif self._should_create_dcra():
5337+
addon_consts = self.context.get_addon_consts()
5338+
monitoring_addon_key = (
5339+
_get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts)
5340+
if cluster.addon_profiles
5341+
else addon_consts.get("CONST_MONITORING_ADDON_NAME")
5342+
)
5343+
self.context.external_functions.ensure_container_insights_for_monitoring(
5344+
self.cmd,
5345+
cluster.addon_profiles[monitoring_addon_key],
5346+
self.context.get_subscription_id(),
5347+
self.context.get_resource_group_name(),
5348+
self.context.get_name(),
5349+
self.context.get_location(),
5350+
remove_monitoring=False,
5351+
aad_route=self.context.get_enable_msi_auth_for_monitoring(),
5352+
create_dcr=True,
5353+
create_dcra=True,
5354+
enable_syslog=self.context.get_enable_syslog(),
5355+
data_collection_settings=self.context.get_data_collection_settings(),
5356+
is_private_cluster=self.context.get_enable_private_cluster(),
5357+
ampls_resource_id=self.context.get_ampls_resource_id(),
5358+
enable_high_log_scale_mode=self.context.get_enable_high_log_scale_mode(),
5359+
)
53225360

53235361
# Handle monitoring addon postprocessing (disable case) - same logic as aks_disable_addons
53245362
monitoring_addon_disable_postprocessing_required = self.context.get_intermediate(
53255363
"monitoring_addon_disable_postprocessing_required", default_value=False
53265364
)
53275365

53285366
if monitoring_addon_disable_postprocessing_required:
5329-
self._postprocess_monitoring_disable()
5367+
addon_consts = self.context.get_addon_consts()
5368+
CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME")
5369+
5370+
# Get the current cluster state to check config before it was disabled
5371+
current_cluster = self.client.get(self.context.get_resource_group_name(), self.context.get_name())
5372+
5373+
if (current_cluster.addon_profiles and
5374+
CONST_MONITORING_ADDON_NAME in current_cluster.addon_profiles):
5375+
5376+
addon_profile = current_cluster.addon_profiles[CONST_MONITORING_ADDON_NAME]
5377+
5378+
try:
5379+
self.context.external_functions.ensure_container_insights_for_monitoring(
5380+
self.cmd,
5381+
addon_profile,
5382+
self.context.get_subscription_id(),
5383+
self.context.get_resource_group_name(),
5384+
self.context.get_name(),
5385+
self.context.get_location(),
5386+
remove_monitoring=True,
5387+
aad_route=True,
5388+
create_dcr=False,
5389+
create_dcra=True,
5390+
enable_syslog=False,
5391+
data_collection_settings=None,
5392+
ampls_resource_id=None,
5393+
enable_high_log_scale_mode=False
5394+
)
5395+
except TypeError:
5396+
pass
53305397

53315398
# ingress appgw addon
53325399
ingress_appgw_addon_enabled = self.context.get_intermediate("ingress_appgw_addon_enabled", default_value=False)
@@ -5506,91 +5573,14 @@ def _should_create_dcra(self) -> bool:
55065573
)
55075574

55085575
def _is_cnl_or_hlsm_changing(self) -> bool:
5509-
"""Return True if any CNL or High Log Scale Mode flag was provided."""
5576+
"""Return True if any CNL or High Log Scale Mode enable flag was provided."""
55105577
params = self.context.raw_param
55115578
return (
55125579
params.get("enable_container_network_logs") is not None or
55135580
params.get("enable_retina_flow_logs") is not None or
5514-
params.get("disable_container_network_logs") is not None or
5515-
params.get("disable_retina_flow_logs") is not None or
55165581
params.get("enable_high_log_scale_mode") is not None
55175582
)
55185583

5519-
def _postprocess_monitoring_enable(self, cluster: ManagedCluster) -> None:
5520-
"""Handle monitoring addon postprocessing for the enable case."""
5521-
enable_msi_auth_for_monitoring = self.context.get_enable_msi_auth_for_monitoring()
5522-
if not enable_msi_auth_for_monitoring:
5523-
# add cluster spn/msi Monitoring Metrics Publisher role assignment to publish metrics to MDM
5524-
# mdm metrics is supported only in azure public cloud, so add the role assignment only in this cloud
5525-
cloud_name = self.cmd.cli_ctx.cloud.name
5526-
if cloud_name.lower() == "azurecloud":
5527-
cluster_resource_id = resource_id(
5528-
subscription=self.context.get_subscription_id(),
5529-
resource_group=self.context.get_resource_group_name(),
5530-
namespace="Microsoft.ContainerService",
5531-
type="managedClusters",
5532-
name=self.context.get_name(),
5533-
)
5534-
self.context.external_functions.add_monitoring_role_assignment(
5535-
cluster, cluster_resource_id, self.cmd
5536-
)
5537-
elif self._should_create_dcra():
5538-
addon_consts = self.context.get_addon_consts()
5539-
monitoring_addon_key = (
5540-
_get_monitoring_addon_key_from_consts(cluster.addon_profiles, addon_consts)
5541-
if cluster.addon_profiles
5542-
else addon_consts.get("CONST_MONITORING_ADDON_NAME")
5543-
)
5544-
self.context.external_functions.ensure_container_insights_for_monitoring(
5545-
self.cmd,
5546-
cluster.addon_profiles[monitoring_addon_key],
5547-
self.context.get_subscription_id(),
5548-
self.context.get_resource_group_name(),
5549-
self.context.get_name(),
5550-
self.context.get_location(),
5551-
remove_monitoring=False,
5552-
aad_route=self.context.get_enable_msi_auth_for_monitoring(),
5553-
create_dcr=True,
5554-
create_dcra=True,
5555-
enable_syslog=self.context.get_enable_syslog(),
5556-
data_collection_settings=self.context.get_data_collection_settings(),
5557-
is_private_cluster=self.context.get_enable_private_cluster(),
5558-
ampls_resource_id=self.context.get_ampls_resource_id(),
5559-
enable_high_log_scale_mode=self.context.get_enable_high_log_scale_mode(),
5560-
)
5561-
5562-
def _postprocess_monitoring_disable(self) -> None:
5563-
"""Handle monitoring addon postprocessing for the disable case."""
5564-
addon_consts = self.context.get_addon_consts()
5565-
CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME")
5566-
5567-
# Get the current cluster state to check config before it was disabled
5568-
current_cluster = self.client.get(self.context.get_resource_group_name(), self.context.get_name())
5569-
5570-
if (current_cluster.addon_profiles and
5571-
CONST_MONITORING_ADDON_NAME in current_cluster.addon_profiles):
5572-
5573-
addon_profile = current_cluster.addon_profiles[CONST_MONITORING_ADDON_NAME]
5574-
5575-
try:
5576-
self.context.external_functions.ensure_container_insights_for_monitoring(
5577-
self.cmd,
5578-
addon_profile,
5579-
self.context.get_subscription_id(),
5580-
self.context.get_resource_group_name(),
5581-
self.context.get_name(),
5582-
self.context.get_location(),
5583-
remove_monitoring=True,
5584-
aad_route=True,
5585-
create_dcr=False,
5586-
create_dcra=True,
5587-
enable_syslog=False,
5588-
data_collection_settings=None,
5589-
ampls_resource_id=None,
5590-
enable_high_log_scale_mode=False
5591-
)
5592-
except TypeError:
5593-
pass
55945584

55955585

55965586
class AKSPreviewManagedClusterUpdateDecorator(AKSManagedClusterUpdateDecorator):

0 commit comments

Comments
 (0)