Skip to content

Commit bd80f27

Browse files
carlotaarvelacarlotaarvela
committed
PR comments
1 parent 9b24712 commit bd80f27

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
@@ -552,7 +552,7 @@ def __init__(self, location, resource_id):
552552
)
553553

554554
resources = get_resources_client(cmd.cli_ctx, cluster_subscription)
555-
for attempt in range(3):
555+
for _ in range(3):
556556
try:
557557
if enable_syslog:
558558
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.
@@ -4674,8 +4673,8 @@ def _setup_azure_monitor_logs(self, mc: ManagedCluster) -> None:
46744673
mc.addon_profiles[CONST_MONITORING_ADDON_NAME] = addon_profile
46754674

46764675
# DCR and DCRA creation is deferred to postprocessing_after_mc_created
4677-
# (_postprocess_monitoring_enable) so that all flags are finalized and
4678-
# the cluster exists. Only MSI clusters need a DCR.
4676+
# so that all flags are finalized and the cluster exists.
4677+
# Only MSI clusters need a DCR.
46794678
self.context.set_intermediate("monitoring_addon_enabled", True, overwrite_exists=True)
46804679

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

53185356
# Handle monitoring addon postprocessing (disable case) - same logic as aks_disable_addons
53195357
monitoring_addon_disable_postprocessing_required = self.context.get_intermediate(
53205358
"monitoring_addon_disable_postprocessing_required", default_value=False
53215359
)
53225360

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

53265393
# ingress appgw addon
53275394
ingress_appgw_addon_enabled = self.context.get_intermediate("ingress_appgw_addon_enabled", default_value=False)
@@ -5501,91 +5568,14 @@ def _should_create_dcra(self) -> bool:
55015568
)
55025569

55035570
def _is_cnl_or_hlsm_changing(self) -> bool:
5504-
"""Return True if any CNL or High Log Scale Mode flag was provided."""
5571+
"""Return True if any CNL or High Log Scale Mode enable flag was provided."""
55055572
params = self.context.raw_param
55065573
return (
55075574
params.get("enable_container_network_logs") is not None or
55085575
params.get("enable_retina_flow_logs") is not None or
5509-
params.get("disable_container_network_logs") is not None or
5510-
params.get("disable_retina_flow_logs") is not None or
55115576
params.get("enable_high_log_scale_mode") is not None
55125577
)
55135578

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

55905580

55915581
class AKSPreviewManagedClusterUpdateDecorator(AKSManagedClusterUpdateDecorator):

0 commit comments

Comments
 (0)