Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

{AKS} Fix azdev style issues for decorators #7123

Merged
merged 2 commits into from
Jan 2, 2024
Merged

{AKS} Fix azdev style issues for decorators #7123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f06e77e
fix for decorators
FumingZhang Dec 26, 2023
157bac6
fix
FumingZhang Dec 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 115 additions & 85 deletions src/aks-preview/azext_aks_preview/addonconfiguration.py
View file
Open in desktop
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
View file
Open in desktop
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
Loading