Skip to content

Aks [nodepool] update without args prompts to reconcile. #4759

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

Merged
zhoxing-ms merged 13 commits into from
May 5, 2022
Merged

Aks [nodepool] update without args prompts to reconcile. #4759

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
42f4b4f
update with no args to reconcile
May 1, 2022
8da34a7
allow no argument to agentpool update to reconcile that
May 1, 2022
cb85d4e
keep patch
May 2, 2022
46a065f
try and test reconcile case
May 2, 2022
ef35758
help for update with out arguments
May 2, 2022
f7114f7
Update _help.py
paulgmiller May 4, 2022
1a63bfb
Merge branch 'main' of github.com:Azure/azure-cli-extensions into upd…
May 4, 2022
f0e54e5
xMerge remote-tracking branch 'paul/updateasreconcile' into updateasr…
May 4, 2022
ea452ce
short summaries
May 4, 2022
a8308a9
Update src/aks-preview/azext_aks_preview/_help.py
paulgmiller May 4, 2022
beabdb5
Update src/aks-preview/azext_aks_preview/_help.py
paulgmiller May 4, 2022
fb94ed7
matchris comments
May 4, 2022
a991245
bump vesion
May 5, 2022
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
10 changes: 8 additions & 2 deletions src/aks-preview/azext_aks_preview/_help.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,8 @@

helps['aks update'] = """
type: command
short-summary: Update a managed Kubernetes cluster properties, such as enable/disable cluster-autoscaler
short-summary: Update a managed Kubernetes cluster properties.
long-summary: Update a managed Kubernetes cluster properties, such as enable/disable cluster-autoscaler. No arguments attempts to move the cluster to its goal state without changing the current cluster configuration. This operation can be used to move out of a non succeeded state.
Comment thread
paulgmiller marked this conversation as resolved.
Outdated
parameters:
- name: --enable-cluster-autoscaler -e
type: bool
Expand Down Expand Up @@ -707,6 +708,8 @@
type: string
short-summary: Identifier of Azure Key Vault key.
examples:
- name: Reconcile the cluster back to its current state.
text: az aks update -g MyResourceGroup -n MyManagedCluster
- name: Enable cluster-autoscaler within node count range [1,5]
text: az aks update --enable-cluster-autoscaler --min-count 1 --max-count 5 -g MyResourceGroup -n MyManagedCluster
- name: Disable cluster-autoscaler for an existing cluster
Expand Down Expand Up @@ -1132,7 +1135,8 @@

helps['aks nodepool update'] = """
type: command
short-summary: Update a node pool to enable/disable cluster-autoscaler or change min-count or max-count
short-summary: Update a node pool properties.
long-summary: Update a node pool to enable/disable cluster-autoscaler or change min-count or max-count. No arguments attempts to move the cluster to its goal state without changing the current cluster configuration. This operation can be used to move out of a non succeeded state.
Comment thread
paulgmiller marked this conversation as resolved.
Outdated
parameters:
- name: --enable-cluster-autoscaler -e
type: bool
Expand Down Expand Up @@ -1165,6 +1169,8 @@
type: string
short-summary: The node taints for the node pool.
examples:
- name: Reconcile the nodepool back to its current state.
text: az aks nodepool update -g MyResourceGroup -n nodepool1 --cluster-name MyManagedCluster
- name: Enable cluster-autoscaler within node count range [1,5]
text: az aks nodepool update --enable-cluster-autoscaler --min-count 1 --max-count 5 -g MyResourceGroup -n nodepool1 --cluster-name MyManagedCluster
- name: Disable cluster-autoscaler for an existing cluster
Expand Down
16 changes: 7 additions & 9 deletions src/aks-preview/azext_aks_preview/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
from dateutil.parser import parse # pylint: disable=import-error
from dateutil.relativedelta import relativedelta # pylint: disable=import-error
from knack.log import get_logger
from knack.prompting import NoTTYException, prompt_pass
from knack.prompting import NoTTYException, prompt_pass, prompt_y_n
from knack.util import CLIError
from msrestazure.azure_exceptions import CloudError
from six.moves.urllib.error import URLError # pylint: disable=import-error
Expand Down Expand Up @@ -398,7 +398,6 @@ def delete_role_assignments(cli_ctx, ids=None, assignee=None, role=None, resourc
assignments_client.delete_by_id(i)
return
if not any([ids, assignee, role, resource_group_name, scope, assignee, yes]):
from knack.prompting import prompt_y_n
msg = 'This will delete all role assignments under the subscription. Are you sure?'
if not prompt_y_n(msg, default="n"):
return
Expand Down Expand Up @@ -1058,8 +1057,6 @@ def aks_kollect(cmd, # pylint: disable=too-many-statements,too-many-locals

readonly_sas_token = readonly_sas_token.strip('?')

from knack.prompting import prompt_y_n

print()
print('This will deploy a daemon set to your cluster to collect logs and diagnostic information and '
f'save them to the storage account '
Expand Down Expand Up @@ -1240,7 +1237,6 @@ def aks_upgrade(cmd, # pylint: disable=unused-argument, too-many-return-state
node_image_only=False,
aks_custom_headers=None,
yes=False):
from knack.prompting import prompt_y_n
msg = 'Kubernetes may be unavailable during cluster upgrades.\n Are you sure you want to perform this operation?'
if not yes and not prompt_y_n(msg, default="n"):
return None
Expand Down Expand Up @@ -1837,10 +1833,12 @@ def aks_agentpool_update(cmd, # pylint: disable=unused-argument
disable_cluster_autoscaler + update_cluster_autoscaler

if (update_autoscaler != 1 and not tags and not scale_down_mode and not mode and not max_surge and labels is None and node_taints is None):
raise CLIError('Please specify one or more of "--enable-cluster-autoscaler" or '
'"--disable-cluster-autoscaler" or '
'"--update-cluster-autoscaler" or '
'"--tags" or "--mode" or "--max-surge" or "--scale-down-mode" or "--labels" or "--node-taints')
reconcilePrompt = "no argument specified to update would you like to reconcile to current settings?"

@matthchr matthchr May 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: Use ' rather than " for consistency, since from what I can tell that's the style used elsewhere in this file.

if not prompt_y_n(reconcilePrompt, default="n"):
raise CLIError('Please specify one or more of "--enable-cluster-autoscaler" or '
'"--disable-cluster-autoscaler" or '
'"--update-cluster-autoscaler" or '
'"--tags" or "--mode" or "--max-surge" or "--scale-down-mode" or "--labels" or "--node-taints')

instance = client.get(resource_group_name, cluster_name, nodepool_name)

Expand Down
112 changes: 57 additions & 55 deletions src/aks-preview/azext_aks_preview/decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2345,61 +2345,63 @@ def check_raw_parameters(self):
)

if not is_changed and is_default:
# Note: Uncomment the followings to automatically generate the error message.
Comment thread
paulgmiller marked this conversation as resolved.
# option_names = [
# '"{}"'.format(format_parameter_name_to_option_name(x))
# for x in self.context.raw_param.keys()
# if x not in excluded_keys
# ]
# error_msg = "Please specify one or more of {}.".format(
# " or ".join(option_names)
# )
# raise RequiredArgumentMissingError(error_msg)
raise RequiredArgumentMissingError(
'Please specify "--enable-cluster-autoscaler" or '
'"--disable-cluster-autoscaler" or '
'"--update-cluster-autoscaler" or '
'"--cluster-autoscaler-profile" or '
'"--enable-pod-security-policy" or '
'"--disable-pod-security-policy" or '
'"--api-server-authorized-ip-ranges" or '
'"--attach-acr" or '
'"--detach-acr" or '
'"--uptime-sla" or '
'"--no-uptime-sla" or '
'"--load-balancer-managed-outbound-ip-count" or '
'"--load-balancer-outbound-ips" or '
'"--load-balancer-outbound-ip-prefixes" or '
'"--nat-gateway-managed-outbound-ip-count" or '
'"--nat-gateway-idle-timeout" or '
'"--enable-aad" or '
'"--aad-tenant-id" or '
'"--aad-admin-group-object-ids" or '
'"--enable-ahub" or '
'"--disable-ahub" or '
'"--enable-managed-identity" or '
'"--enable-pod-identity" or '
'"--disable-pod-identity" or '
'"--auto-upgrade-channel" or '
'"--enable-secret-rotation" or '
'"--disable-secret-rotation" or '
'"--rotation-poll-interval" or '
'"--tags" or '
'"--windows-admin-password" or '
'"--enable-azure-rbac" or '
'"--disable-azure-rbac" or '
'"--enable-local-accounts" or '
'"--disable-local-accounts" or '
'"--enable-public-fqdn" or '
'"--disable-public-fqdn"'
'"--enble-windows-gmsa" or '
'"--nodepool-labels" or '
'"--enable-oidc-issuer" or '
'"--http-proxy-config" or '
'"--enable-azure-keyvault-kms" or '
'"--enable-workload-identity" or '
'"--disable-workload-identity".'
)
reconcilePrompt = "no argument specified to update would you like to reconcile to current settings?"
if not prompt_y_n(reconcilePrompt, default="n"):
# Note: Uncomment the followings to automatically generate the error message.
# option_names = [
# '"{}"'.format(format_parameter_name_to_option_name(x))
# for x in self.context.raw_param.keys()
# if x not in excluded_keys
# ]
# error_msg = "Please specify one or more of {}.".format(
# " or ".join(option_names)
# )
# raise RequiredArgumentMissingError(error_msg)
raise RequiredArgumentMissingError(
'Please specify "--enable-cluster-autoscaler" or '
'"--disable-cluster-autoscaler" or '
'"--update-cluster-autoscaler" or '
'"--cluster-autoscaler-profile" or '
'"--enable-pod-security-policy" or '
'"--disable-pod-security-policy" or '
'"--api-server-authorized-ip-ranges" or '
'"--attach-acr" or '
'"--detach-acr" or '
'"--uptime-sla" or '
'"--no-uptime-sla" or '
'"--load-balancer-managed-outbound-ip-count" or '
'"--load-balancer-outbound-ips" or '
'"--load-balancer-outbound-ip-prefixes" or '
'"--nat-gateway-managed-outbound-ip-count" or '
'"--nat-gateway-idle-timeout" or '
'"--enable-aad" or '
'"--aad-tenant-id" or '
'"--aad-admin-group-object-ids" or '
'"--enable-ahub" or '
'"--disable-ahub" or '
'"--enable-managed-identity" or '
'"--enable-pod-identity" or '
'"--disable-pod-identity" or '
'"--auto-upgrade-channel" or '
'"--enable-secret-rotation" or '
'"--disable-secret-rotation" or '
'"--rotation-poll-interval" or '
'"--tags" or '
'"--windows-admin-password" or '
'"--enable-azure-rbac" or '
'"--disable-azure-rbac" or '
'"--enable-local-accounts" or '
'"--disable-local-accounts" or '
'"--enable-public-fqdn" or '
'"--disable-public-fqdn"'
'"--enble-windows-gmsa" or '
'"--nodepool-labels" or '
'"--enable-oidc-issuer" or '
'"--http-proxy-config" or '
'"--enable-azure-keyvault-kms" or '
'"--enable-workload-identity" or '
'"--disable-workload-identity".'
)

def update_load_balancer_profile(self, mc: ManagedCluster) -> ManagedCluster:
"""Update load balancer profile for the ManagedCluster object.
Expand Down
12 changes: 11 additions & 1 deletion src/aks-preview/azext_aks_preview/tests/latest/test_decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3265,7 +3265,17 @@ def test_check_raw_parameters(self):
CUSTOM_MGMT_AKS_PREVIEW,
)
# fail on no updated parameter provided
with self.assertRaises(RequiredArgumentMissingError):
with patch(
"azext_aks_preview.decorator.prompt_y_n",
return_value=False,
),self.assertRaises(RequiredArgumentMissingError):
dec_1.check_raw_parameters()

# unless user says they want to reconcile
with patch(
"azext_aks_preview.decorator.prompt_y_n",
return_value=True,
):
dec_1.check_raw_parameters()

# custom value
Expand Down