Skip to content

Commit

Permalink
[AKS] Fix #23468: az aks nodepool wait crashes with error "'Namespa…
Browse files Browse the repository at this point in the history
…ce' object has no attribute 'nodepool_name'" (#23489)
  • Loading branch information
FumingZhang authored Aug 10, 2022
1 parent 68ee692 commit 0625878
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 15 deletions.
17 changes: 9 additions & 8 deletions src/azure-cli/azure/cli/command_modules/acs/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@
CONST_SPOT_EVICTION_POLICY_DELETE, CONST_STABLE_UPGRADE_CHANNEL,
CONST_AZURE_KEYVAULT_NETWORK_ACCESS_PUBLIC, CONST_AZURE_KEYVAULT_NETWORK_ACCESS_PRIVATE)
from azure.cli.command_modules.acs._validators import (
validate_acr, validate_assign_identity, validate_assign_kubelet_identity,
validate_acr, validate_agent_pool_name, validate_assign_identity,
validate_assign_kubelet_identity, validate_azure_keyvault_kms_key_id,
validate_azure_keyvault_kms_key_vault_resource_id,
validate_create_parameters, validate_credential_format,
validate_eviction_policy, validate_ip_ranges, validate_k8s_version,
validate_defender_config_parameter,
validate_defender_disable_and_enable_parameters, validate_eviction_policy,
validate_host_group_id, validate_ip_ranges, validate_k8s_version,
validate_keyvault_secrets_provider_disable_and_enable_parameters,
validate_kubectl_version, validate_kubelogin_version,
validate_linux_host_name, validate_list_of_integers,
validate_load_balancer_idle_timeout,
Expand All @@ -41,11 +46,7 @@
validate_nodes_count, validate_pod_subnet_id, validate_ppg,
validate_priority, validate_snapshot_id, validate_snapshot_name,
validate_spot_max_price, validate_ssh_key, validate_taints,
validate_vm_set_type, validate_vnet_subnet_id,
validate_keyvault_secrets_provider_disable_and_enable_parameters,
validate_defender_disable_and_enable_parameters, validate_defender_config_parameter,
validate_host_group_id,
validate_azure_keyvault_kms_key_id, validate_azure_keyvault_kms_key_vault_resource_id)
validate_vm_set_type, validate_vnet_subnet_id)
from azure.cli.core.commands.parameters import (
edge_zone_type, file_type, get_enum_type,
get_resource_name_completion_list, get_three_state_flag, name_type,
Expand Down Expand Up @@ -454,7 +455,7 @@ def load_arguments(self, _):
with self.argument_context('aks nodepool', resource_type=ResourceType.MGMT_CONTAINERSERVICE, operation_group='managed_clusters') as c:
c.argument('cluster_name', help='The cluster name.')
# the following argument is declared for the wait command
c.argument('agent_pool_name', options_list=['--nodepool-name', '--agent-pool-name'], validator=validate_nodepool_name, help='The node pool name.')
c.argument('agent_pool_name', options_list=['--nodepool-name', '--agent-pool-name'], validator=validate_agent_pool_name, help='The node pool name.')

for sub_command in ['add', 'update', 'upgrade', 'scale', 'show', 'list', 'delete']:
with self.argument_context('aks nodepool ' + sub_command) as c:
Expand Down
20 changes: 15 additions & 5 deletions src/azure-cli/azure/cli/command_modules/acs/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,23 @@ def validate_k8s_version(namespace):
'such as "1.7.12" or "1.7"')


def _validate_nodepool_name(nodepool_name):
"""Validates a nodepool name to be at most 12 characters, alphanumeric only."""
if nodepool_name != "":
if len(nodepool_name) > 12:
raise InvalidArgumentValueError('--nodepool-name can contain at most 12 characters')
if not nodepool_name.isalnum():
raise InvalidArgumentValueError('--nodepool-name should contain only alphanumeric characters')


def validate_nodepool_name(namespace):
"""Validates a nodepool name to be at most 12 characters, alphanumeric only."""
if namespace.nodepool_name != "":
if len(namespace.nodepool_name) > 12:
raise CLIError('--nodepool-name can contain at most 12 characters')
if not namespace.nodepool_name.isalnum():
raise CLIError('--nodepool-name should contain only alphanumeric characters')
_validate_nodepool_name(namespace.nodepool_name)


def validate_agent_pool_name(namespace):
"""Validates a nodepool name to be at most 12 characters, alphanumeric only."""
_validate_nodepool_name(namespace.agent_pool_name)


def validate_kubectl_version(namespace):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,11 @@ def test_valid_keyvault_secret_provider_parameters(self):
)
validators.validate_keyvault_secrets_provider_disable_and_enable_parameters(namespace_3)


class HostGroupIDNamespace:
def __init__(self, host_group_id):
def __init__(self, host_group_id):
self.host_group_id = host_group_id

self.host_group_id = host_group_id

class TestValidateHostGroupID(unittest.TestCase):
def test_invalid_host_group_id(self):
Expand All @@ -495,6 +496,7 @@ class AzureKeyVaultKmsKeyIdNamespace:
def __init__(self, azure_keyvault_kms_key_id):
self.azure_keyvault_kms_key_id = azure_keyvault_kms_key_id


class TestValidateAzureKeyVaultKmsKeyId(unittest.TestCase):
def test_invalid_azure_keyvault_kms_key_id_without_https(self):
invalid_azure_keyvault_kms_key_id = "dummy key id"
Expand Down Expand Up @@ -550,5 +552,71 @@ def test_valid_azure_keyvault_kms_key_vault_resource_id(self):
validators.validate_azure_keyvault_kms_key_vault_resource_id(namespace)


class TestValidateNodepoolName(unittest.TestCase):
def test_invalid_nodepool_name_too_long(self):
namespace = SimpleNamespace(
**{
"nodepool_name": "tooLongNodepoolName",
}
)
with self.assertRaises(InvalidArgumentValueError):
validators.validate_nodepool_name(
namespace
)

def test_invalid_agent_pool_name_too_long(self):
namespace = SimpleNamespace(
**{
"agent_pool_name": "tooLongNodepoolName",
}
)
with self.assertRaises(InvalidArgumentValueError):
validators.validate_agent_pool_name(
namespace
)

def test_invalid_nodepool_name_not_alnum(self):
namespace = SimpleNamespace(
**{
"nodepool_name": "invalid-np*",
}
)
with self.assertRaises(InvalidArgumentValueError):
validators.validate_nodepool_name(
namespace
)

def test_invalid_agent_pool_name_not_alnum(self):
namespace = SimpleNamespace(
**{
"agent_pool_name": "invalid-np*",
}
)
with self.assertRaises(InvalidArgumentValueError):
validators.validate_agent_pool_name(
namespace
)

def test_valid_nodepool_name(self):
namespace = SimpleNamespace(
**{
"nodepool_name": "np100",
}
)
validators.validate_nodepool_name(
namespace
)

def test_valid_agent_pool_name(self):
namespace = SimpleNamespace(
**{
"agent_pool_name": "np100",
}
)
validators.validate_agent_pool_name(
namespace
)


if __name__ == "__main__":
unittest.main()

0 comments on commit 0625878

Please sign in to comment.