From da766d3206d9037a0dddc86487398b4c31fe5342 Mon Sep 17 00:00:00 2001 From: Pramod Valavala Date: Sat, 14 May 2022 11:41:32 +0530 Subject: [PATCH 1/3] Fix autoscaler parameters for user node pools --- .../azure/cli/command_modules/acs/_help.py | 8 +++---- .../cli/command_modules/acs/_validators.py | 10 ++++---- .../acs/agentpool_decorator.py | 19 +++++++++++++++ .../tests/latest/test_agentpool_decorator.py | 23 ++++++++++++------- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_help.py b/src/azure-cli/azure/cli/command_modules/acs/_help.py index 6f38921b8c0..54d0675218c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_help.py @@ -985,10 +985,10 @@ short-summary: Enable cluster autoscaler. - name: --min-count type: int - short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [1, 1000] + short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] - name: --max-count type: int - short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [1, 1000] + short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] - name: --scale-down-mode type: string short-summary: "Describe how VMs are added to or removed from nodepools." @@ -1101,10 +1101,10 @@ short-summary: Update min-count or max-count for cluster autoscaler. - name: --min-count type: int - short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [1, 1000] + short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] - name: --max-count type: int - short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [1, 1000] + short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] - name: --scale-down-mode type: string short-summary: "Describe how VMs are added to or removed from nodepools." diff --git a/src/azure-cli/azure/cli/command_modules/acs/_validators.py b/src/azure-cli/azure/cli/command_modules/acs/_validators.py index 9dc46c8fba2..83caa8ddc32 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_validators.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_validators.py @@ -224,13 +224,13 @@ def validate_nat_gateway_idle_timeout(namespace): def validate_nodes_count(namespace): - """Validates that min_count and max_count is set between 1-100""" + """Validates that min_count and max_count is set between 0-1000""" if namespace.min_count is not None: - if namespace.min_count < 1 or namespace.min_count > 100: - raise CLIError('--min-count must be in the range [1,100]') + if namespace.min_count < 0 or namespace.min_count > 1000: + raise CLIError('--min-count must be in the range [0,1000]') if namespace.max_count is not None: - if namespace.max_count < 1 or namespace.max_count > 100: - raise CLIError('--max-count must be in the range [1,100]') + if namespace.max_count < 0 or namespace.max_count > 1000: + raise CLIError('--max-count must be in the range [0,1000]') def validate_taints(namespace): diff --git a/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py index 61fbf56cbe8..a85600bd89e 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py @@ -172,6 +172,7 @@ def __validate_counts_in_autoscaler( enable_cluster_autoscaler, min_count, max_count, + mode, decorator_mode, ) -> None: """Helper function to check the validity of serveral count-related parameters in autoscaler. @@ -201,6 +202,11 @@ def __validate_counts_in_autoscaler( raise InvalidArgumentValueError( "node-count is not in the range of min-count and max-count" ) + if mode == CONST_NODEPOOL_MODE_SYSTEM: + if min_count < 1: + raise InvalidArgumentValueError( + "Value of min-count should be greater than or equal to 1 for System node pools" + ) else: if min_count is not None or max_count is not None: option_name = "--enable-cluster-autoscaler" @@ -628,6 +634,13 @@ def get_node_count_and_enable_cluster_autoscaler_min_max_count( if self.agentpool and self.agentpool.max_count is not None: max_count = self.agentpool.max_count + # mode + # read the original value passed by the command + mode = self.raw_param.get("mode") + # try to read the property value corresponding to the parameter from the `agentpool` object + if self.agentpool and self.agentpool.mode is not None: + mode = self.agentpool.mode + # these parameters do not need dynamic completion # validation @@ -636,6 +649,7 @@ def get_node_count_and_enable_cluster_autoscaler_min_max_count( enable_cluster_autoscaler, min_count, max_count, + mode, decorator_mode=DecoratorMode.CREATE, ) return node_count, enable_cluster_autoscaler, min_count, max_count @@ -679,6 +693,10 @@ def get_update_enable_disable_cluster_autoscaler_and_min_max_count( # read the original value passed by the command max_count = self.raw_param.get("max_count") + # mode + # read the original value passed by the command, defaulting to the value from the `agentpool` object + mode = self.raw_param.get("mode", self.agentpool.mode) + # these parameters do not need dynamic completion # validation @@ -701,6 +719,7 @@ def get_update_enable_disable_cluster_autoscaler_and_min_max_count( enable_cluster_autoscaler or update_cluster_autoscaler, min_count, max_count, + mode, decorator_mode=DecoratorMode.UPDATE, ) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_agentpool_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_agentpool_decorator.py index f1a06580a45..500404fe645 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_agentpool_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_agentpool_decorator.py @@ -156,30 +156,37 @@ def common_validate_counts_in_autoscaler(self): self.cmd, AKSAgentPoolParamDict({}), self.models, DecoratorMode.CREATE, self.agentpool_decorator_mode ) # default - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(3, False, None, None, DecoratorMode.CREATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(3, False, None, None, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) # custom value - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 1, 10, DecoratorMode.CREATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 1, 10, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) # fail on min_count/max_count not specified with self.assertRaises(RequiredArgumentMissingError): - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, None, None, DecoratorMode.CREATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, None, None, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) # fail on min_count > max_count with self.assertRaises(InvalidArgumentValueError): - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 3, 1, DecoratorMode.CREATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 3, 1, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) # fail on node_count < min_count in create mode with self.assertRaises(InvalidArgumentValueError): - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 7, 10, DecoratorMode.CREATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 7, 10, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) # skip node_count check in update mode - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 7, 10, DecoratorMode.UPDATE) - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(None, True, 7, 10, DecoratorMode.UPDATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, True, 7, 10, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.UPDATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(None, True, 7, 10, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.UPDATE) # fail on enable_cluster_autoscaler not specified with self.assertRaises(RequiredArgumentMissingError): - ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, False, 3, None, DecoratorMode.UPDATE) + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(5, False, 3, None, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.UPDATE) + + # min_count set to 0 for user node pools + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(0, True, 0, 1, CONST_NODEPOOL_MODE_USER, DecoratorMode.CREATE) + + # fail on min_count < 1 for system node pools + with self.assertRaises(InvalidArgumentValueError): + ctx._AKSAgentPoolContext__validate_counts_in_autoscaler(1, True, 0, 1, CONST_NODEPOOL_MODE_SYSTEM, DecoratorMode.CREATE) def common_get_resource_group_name(self): # default From 33bedbc59b8db35bd7e378998c8eafb206876cdb Mon Sep 17 00:00:00 2001 From: Liming Liu Date: Sun, 15 May 2022 11:24:05 +0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: FumingZhang <81607949+FumingZhang@users.noreply.github.com> --- .../command_modules/acs/agentpool_decorator.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py index a85600bd89e..f30af1dcf6c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/agentpool_decorator.py @@ -634,13 +634,6 @@ def get_node_count_and_enable_cluster_autoscaler_min_max_count( if self.agentpool and self.agentpool.max_count is not None: max_count = self.agentpool.max_count - # mode - # read the original value passed by the command - mode = self.raw_param.get("mode") - # try to read the property value corresponding to the parameter from the `agentpool` object - if self.agentpool and self.agentpool.mode is not None: - mode = self.agentpool.mode - # these parameters do not need dynamic completion # validation @@ -649,7 +642,7 @@ def get_node_count_and_enable_cluster_autoscaler_min_max_count( enable_cluster_autoscaler, min_count, max_count, - mode, + mode=self.get_mode(), decorator_mode=DecoratorMode.CREATE, ) return node_count, enable_cluster_autoscaler, min_count, max_count @@ -693,10 +686,6 @@ def get_update_enable_disable_cluster_autoscaler_and_min_max_count( # read the original value passed by the command max_count = self.raw_param.get("max_count") - # mode - # read the original value passed by the command, defaulting to the value from the `agentpool` object - mode = self.raw_param.get("mode", self.agentpool.mode) - # these parameters do not need dynamic completion # validation @@ -719,7 +708,7 @@ def get_update_enable_disable_cluster_autoscaler_and_min_max_count( enable_cluster_autoscaler or update_cluster_autoscaler, min_count, max_count, - mode, + mode=self.get_mode(), decorator_mode=DecoratorMode.UPDATE, ) From f3dedef65fcd2d402f904ebe53da5c6865906fe4 Mon Sep 17 00:00:00 2001 From: Liming Liu Date: Sun, 15 May 2022 11:30:44 +0800 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: FumingZhang <81607949+FumingZhang@users.noreply.github.com> --- src/azure-cli/azure/cli/command_modules/acs/_help.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_help.py b/src/azure-cli/azure/cli/command_modules/acs/_help.py index 54d0675218c..119f97d4302 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_help.py @@ -985,10 +985,10 @@ short-summary: Enable cluster autoscaler. - name: --min-count type: int - short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] + short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] for user nodepool, and [1,1000] for system nodepool. - name: --max-count type: int - short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] + short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] for user nodepool, and [1,1000] for system nodepool. - name: --scale-down-mode type: string short-summary: "Describe how VMs are added to or removed from nodepools." @@ -1101,10 +1101,10 @@ short-summary: Update min-count or max-count for cluster autoscaler. - name: --min-count type: int - short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] + short-summary: Minimum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] for user nodepool, and [1,1000] for system nodepool. - name: --max-count type: int - short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] + short-summary: Maximum nodes count used for autoscaler, when "--enable-cluster-autoscaler" specified. Please specify the value in the range of [0, 1000] for user nodepool, and [1,1000] for system nodepool. - name: --scale-down-mode type: string short-summary: "Describe how VMs are added to or removed from nodepools."