Skip to content

Commit

Permalink
fix for decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
FumingZhang committed Dec 27, 2023
1 parent cad08ff commit f06e77e
Show file tree
Hide file tree
Showing 4 changed files with 704 additions and 381 deletions.
200 changes: 115 additions & 85 deletions src/aks-preview/azext_aks_preview/addonconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import json
from knack.log import get_logger
from knack.util import CLIError
from azure.cli.core.azclierror import ArgumentUsageError
Expand Down Expand Up @@ -42,29 +41,32 @@
logger = get_logger(__name__)


def enable_addons(cmd,
client,
resource_group_name,
name,
addons,
check_enabled=True,
workspace_resource_id=None,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
appgw_subnet_cidr=None,
appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None,
enable_sgxquotehelper=False,
enable_secret_rotation=False,
rotation_poll_interval=None,
no_wait=False,
dns_zone_resource_id=None,
dns_zone_resource_ids=None,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None):
# pylint: disable=too-many-locals
def enable_addons(
cmd,
client,
resource_group_name,
name,
addons,
check_enabled=True,
workspace_resource_id=None,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
appgw_subnet_cidr=None,
appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None,
enable_sgxquotehelper=False,
enable_secret_rotation=False,
rotation_poll_interval=None,
no_wait=False,
dns_zone_resource_id=None,
dns_zone_resource_ids=None,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None
):
instance = client.get(resource_group_name, name)
# this is overwritten by _update_addons(), so the value needs to be recorded here
msi_auth = False
Expand All @@ -74,18 +76,33 @@ def enable_addons(cmd,
enable_msi_auth_for_monitoring = False

subscription_id = get_subscription_id(cmd.cli_ctx)
instance = update_addons(cmd, instance, subscription_id, resource_group_name, name, addons, enable=True,
check_enabled=check_enabled,
workspace_resource_id=workspace_resource_id,
enable_msi_auth_for_monitoring=enable_msi_auth_for_monitoring, subnet_name=subnet_name,
appgw_name=appgw_name, appgw_subnet_prefix=appgw_subnet_prefix,
appgw_subnet_cidr=appgw_subnet_cidr, appgw_id=appgw_id, appgw_subnet_id=appgw_subnet_id,
appgw_watch_namespace=appgw_watch_namespace,
enable_sgxquotehelper=enable_sgxquotehelper,
enable_secret_rotation=enable_secret_rotation, rotation_poll_interval=rotation_poll_interval, no_wait=no_wait,
dns_zone_resource_id=dns_zone_resource_id, dns_zone_resource_ids=dns_zone_resource_ids,
enable_syslog=enable_syslog,
data_collection_settings=data_collection_settings)
instance = update_addons(
cmd,
instance,
subscription_id,
resource_group_name,
name,
addons,
enable=True,
check_enabled=check_enabled,
workspace_resource_id=workspace_resource_id,
enable_msi_auth_for_monitoring=enable_msi_auth_for_monitoring,
subnet_name=subnet_name,
appgw_name=appgw_name,
appgw_subnet_prefix=appgw_subnet_prefix,
appgw_subnet_cidr=appgw_subnet_cidr,
appgw_id=appgw_id,
appgw_subnet_id=appgw_subnet_id,
appgw_watch_namespace=appgw_watch_namespace,
enable_sgxquotehelper=enable_sgxquotehelper,
enable_secret_rotation=enable_secret_rotation,
rotation_poll_interval=rotation_poll_interval,
no_wait=no_wait,
dns_zone_resource_id=dns_zone_resource_id,
dns_zone_resource_ids=dns_zone_resource_ids,
enable_syslog=enable_syslog,
data_collection_settings=data_collection_settings,
)

if CONST_MONITORING_ADDON_NAME in instance.addon_profiles and instance.addon_profiles[
CONST_MONITORING_ADDON_NAME].enabled:
Expand All @@ -95,21 +112,20 @@ def enable_addons(cmd,
if not msi_auth:
raise ArgumentUsageError(
"--enable-msi-auth-for-monitoring can not be used on clusters with service principal auth.")
else:
# create a Data Collection Rule (DCR) and associate it with the cluster
ensure_container_insights_for_monitoring(
cmd,
instance.addon_profiles[CONST_MONITORING_ADDON_NAME],
subscription_id,
resource_group_name,
name,
instance.location,
aad_route=True,
create_dcr=True,
create_dcra=True,
enable_syslog=enable_syslog,
data_collection_settings=data_collection_settings
)
# create a Data Collection Rule (DCR) and associate it with the cluster
ensure_container_insights_for_monitoring(
cmd,
instance.addon_profiles[CONST_MONITORING_ADDON_NAME],
subscription_id,
resource_group_name,
name,
instance.location,
aad_route=True,
create_dcr=True,
create_dcra=True,
enable_syslog=enable_syslog,
data_collection_settings=data_collection_settings
)
else:
# monitoring addon will use legacy path
if enable_syslog:
Expand Down Expand Up @@ -163,31 +179,34 @@ def enable_addons(cmd,
return result


def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
instance,
subscription_id,
resource_group_name,
name,
addons,
enable,
check_enabled=True,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=True,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
appgw_subnet_cidr=None,
appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None,
enable_sgxquotehelper=False,
enable_secret_rotation=False,
rotation_poll_interval=None,
dns_zone_resource_id=None,
dns_zone_resource_ids=None,
no_wait=False, # pylint: disable=unused-argument
enable_syslog=False,
data_collection_settings=None):
# pylint: disable=too-many-locals, too-many-branches, too-many-statements
def update_addons(
cmd,
instance,
subscription_id,
resource_group_name,
name,
addons,
enable,
check_enabled=True,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=True,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
appgw_subnet_cidr=None,
appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None,
enable_sgxquotehelper=False,
enable_secret_rotation=False,
rotation_poll_interval=None,
dns_zone_resource_id=None,
dns_zone_resource_ids=None,
no_wait=False, # pylint: disable=unused-argument
enable_syslog=False, # pylint: disable=unused-argument
data_collection_settings=None, # pylint: disable=unused-argument
):
# parse the comma-separated addons argument
addon_args = addons.split(',')

Expand Down Expand Up @@ -242,7 +261,7 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
continue

if addon_arg not in ADDONS:
raise CLIError("Invalid addon name: {}.".format(addon_arg))
raise CLIError(f"Invalid addon name: {addon_arg}.")
addon = ADDONS[addon_arg]
if addon == CONST_VIRTUAL_NODE_ADDON_NAME:
# only linux is supported for now, in the future this will be a user flag
Expand Down Expand Up @@ -274,13 +293,22 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements

cloud_name = cmd.cli_ctx.cloud.name
if enable_msi_auth_for_monitoring and (cloud_name.lower() == 'ussec' or cloud_name.lower() == 'usnat'):
if instance.identity is not None and instance.identity.type is not None and instance.identity.type == "userassigned":
logger.warning("--enable_msi_auth_for_monitoring is not supported in %s cloud and continuing monitoring enablement without this flag.", cloud_name)
if (
instance.identity is not None and
instance.identity.type is not None and
instance.identity.type == "userassigned"
):
logger.warning(
"--enable_msi_auth_for_monitoring is not supported in %s cloud and continuing "
"monitoring enablement without this flag.", cloud_name
)
enable_msi_auth_for_monitoring = False

addon_profile.config = {
logAnalyticsConstName: workspace_resource_id}
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = "true" if enable_msi_auth_for_monitoring else "false"
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = (
"true" if enable_msi_auth_for_monitoring else "false"
)
elif addon == (CONST_VIRTUAL_NODE_ADDON_NAME + os_type):
if addon_profile.enabled and check_enabled:
raise CLIError('The virtual-node addon is already enabled for this managed cluster.\n'
Expand Down Expand Up @@ -333,10 +361,11 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
elif addon == CONST_AZURE_KEYVAULT_SECRETS_PROVIDER_ADDON_NAME:
if addon_profile.enabled and check_enabled:
raise CLIError(
'The azure-keyvault-secrets-provider addon is already enabled for this managed cluster.\n'
'To change azure-keyvault-secrets-provider configuration, run '
f'"az aks disable-addons -a azure-keyvault-secrets-provider -n {name} -g {resource_group_name}" '
'before enabling it again.')
"The azure-keyvault-secrets-provider addon is already enabled for this managed cluster.\n"
"To change azure-keyvault-secrets-provider configuration, run "
'"az aks disable-addons -a azure-keyvault-secrets-provider '
f'-n {name} -g {resource_group_name}" before enabling it again.'
)
addon_profile = ManagedClusterAddonProfile(
enabled=True, config={CONST_SECRET_ROTATION_ENABLED: "false", CONST_ROTATION_POLL_INTERVAL: "2m"})
if enable_secret_rotation:
Expand All @@ -352,7 +381,8 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
enabled=False)
else:
raise CLIError(
"The addon {} is not installed.".format(addon))
f"The addon {addon} is not installed."
)
addon_profiles[addon].config = None
addon_profiles[addon].enabled = enable

Expand Down
26 changes: 13 additions & 13 deletions src/aks-preview/azext_aks_preview/agentpool_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@
CONST_VIRTUAL_MACHINE_SCALE_SETS,
CONST_AVAILABILITY_SET,
CONST_VIRTUAL_MACHINES,
CONST_OS_SKU_UBUNTU,
)
from azext_aks_preview._params import node_os_skus_update
from azext_aks_preview._helpers import get_nodepool_snapshot_by_snapshot_id

logger = get_logger(__name__)
Expand Down Expand Up @@ -98,7 +96,9 @@ def get_vm_set_type(self) -> str:
elif vm_set_type.lower() == CONST_VIRTUAL_MACHINES.lower():
vm_set_type = CONST_VIRTUAL_MACHINES
else:
raise InvalidArgumentValueError("--vm-set-type can only be VirtualMachineScaleSets, AvailabilitySet or VirtualMachines(Preview)")
raise InvalidArgumentValueError(
"--vm-set-type can only be VirtualMachineScaleSets, AvailabilitySet or VirtualMachines(Preview)"
)
# this parameter does not need validation
return vm_set_type

Expand Down Expand Up @@ -132,9 +132,7 @@ def get_message_of_the_day(self) -> Union[str, None]:
if message_of_the_day_file_path:
if not os.path.isfile(message_of_the_day_file_path):
raise InvalidArgumentValueError(
"{} is not valid file, or not accessable.".format(
message_of_the_day_file_path
)
f"{message_of_the_day_file_path} is not valid file, or not accessable."
)
message_of_the_day = read_file_content(
message_of_the_day_file_path)
Expand Down Expand Up @@ -507,7 +505,7 @@ def set_up_agentpool_windows_profile(self, agentpool: AgentPool) -> AgentPool:

# Construct AgentPoolWindowsProfile if one of the fields has been set
if disable_windows_outbound_nat:
agentpool.windows_profile = self.models.AgentPoolWindowsProfile(
agentpool.windows_profile = self.models.AgentPoolWindowsProfile( # pylint: disable=no-member
disable_outbound_nat=disable_windows_outbound_nat
)

Expand All @@ -518,7 +516,7 @@ def set_up_agentpool_network_profile(self, agentpool: AgentPool) -> AgentPool:

asg_ids = self.context.get_asg_ids()
allowed_host_ports = self.context.get_allowed_host_ports()
agentpool.network_profile = self.models.AgentPoolNetworkProfile()
agentpool.network_profile = self.models.AgentPoolNetworkProfile() # pylint: disable=no-member
if allowed_host_ports is not None:
agentpool.network_profile.allowed_host_ports = allowed_host_ports
agentpool.network_profile.application_security_groups = asg_ids
Expand All @@ -544,7 +542,9 @@ def set_up_artifact_streaming(self, agentpool: AgentPool) -> AgentPool:

if self.context.get_enable_artifact_streaming():
if agentpool.artifact_streaming_profile is None:
agentpool.artifact_streaming_profile = self.models.AgentPoolArtifactStreamingProfile()
agentpool.artifact_streaming_profile = (
self.models.AgentPoolArtifactStreamingProfile() # pylint: disable=no-member
)
agentpool.artifact_streaming_profile.enabled = True
return agentpool

Expand Down Expand Up @@ -584,7 +584,7 @@ def set_up_upgrade_settings(self, agentpool: AgentPool) -> AgentPool:
"""
self._ensure_agentpool(agentpool)

upgrade_settings = self.models.AgentPoolUpgradeSettings()
upgrade_settings = self.models.AgentPoolUpgradeSettings() # pylint: disable=no-member
max_surge = self.context.get_max_surge()
if max_surge:
upgrade_settings.max_surge = max_surge
Expand Down Expand Up @@ -654,7 +654,7 @@ def update_network_profile(self, agentpool: AgentPool) -> AgentPool:
asg_ids = self.context.get_asg_ids()
allowed_host_ports = self.context.get_allowed_host_ports()
if not agentpool.network_profile and (asg_ids or allowed_host_ports):
agentpool.network_profile = self.models.AgentPoolNetworkProfile()
agentpool.network_profile = self.models.AgentPoolNetworkProfile() # pylint: disable=no-member
if asg_ids is not None:
agentpool.network_profile.application_security_groups = asg_ids
if allowed_host_ports is not None:
Expand All @@ -669,7 +669,7 @@ def update_artifact_streaming(self, agentpool: AgentPool) -> AgentPool:

if self.context.get_enable_artifact_streaming():
if agentpool.artifact_streaming_profile is None:
agentpool.artifact_streaming_profile = self.models.AgentPoolArtifactStreamingProfile()
agentpool.artifact_streaming_profile = self.models.AgentPoolArtifactStreamingProfile() # pylint: disable=no-member
agentpool.artifact_streaming_profile.enabled = True
return agentpool

Expand Down Expand Up @@ -714,7 +714,7 @@ def update_upgrade_settings(self, agentpool: AgentPool) -> AgentPool:

upgrade_settings = agentpool.upgrade_settings
if upgrade_settings is None:
upgrade_settings = self.models.AgentPoolUpgradeSettings()
upgrade_settings = self.models.AgentPoolUpgradeSettings() # pylint: disable=no-member

max_surge = self.context.get_max_surge()
if max_surge:
Expand Down
Loading

0 comments on commit f06e77e

Please sign in to comment.