-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[AKS] az aks update: Add --network-policy to support updating the mode of a network policy
#27466
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.
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
Conversation
🔄AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| aks update | cmd aks update added parameter network_policy |
|
AKS |
--network-policy to az aks update
de1b429 to
8ada376
Compare
| long-summary: | | ||
| Using together with "azure" network plugin. | ||
| Specify "azure" for Azure network policy manager and "calico" for calico network policy controller. | ||
| Specify "azure" for Azure network policy manager, "calico" for calico network policy controller, "cilium" for Azure CNI Overlay powered by Cilium. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Azure CNI Powered by Cilium"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed description
FumingZhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
|
||
| # update to enable cilium dataplane | ||
| update_cmd = 'aks update -g {resource_group} -n {name} --network-dataplane=cilium' | ||
| update_cmd = 'aks update -g {resource_group} -n {name} --network-dataplane=cilium --network-policy=cilium' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Queued live test to validate the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add check for this new added param and update the recording file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to keep the argument consistent between az aks create and az aks update. We will add the enum check in a follow-up pr.
Also see the comment below:
We want to avoid referencing 'none' (that was introduced in API version 2023-08-02-preview) until it is ready. See comment: Azure/azure-cli-extensions#6809 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the live test passed? Please upload the recoding file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uploaded the recording file and attached to the description
|
Suggested title: |
| c.argument('nat_gateway_managed_outbound_ip_count', type=int, validator=validate_nat_gateway_managed_outbound_ip_count) | ||
| c.argument('nat_gateway_idle_timeout', type=int, validator=validate_nat_gateway_idle_timeout) | ||
| c.argument('network_dataplane', arg_type=get_enum_type(network_dataplanes)) | ||
| c.argument('network_policy') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the value of this param enumerable? If so, please add arg_type=get_enum_type to the param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to avoid referencing 'none' (that was introduced in API version 2023-08-02-preview) until it is ready. See comment: Azure/azure-cli-extensions#6809 (comment)
--network-policy to az aks updateaz aks update: Add --network-policy to support updating the mode of a network policy
az aks update: Add --network-policy to support updating the mode of a network policyaz aks update : Add --network-policy to support updating the mode of a network policy
az aks update : Add --network-policy to support updating the mode of a network policyaz aks update: Add --network-policy to support updating the mode of a network policy
az aks update: Add --network-policy to support updating the mode of a network policyaz aks update : Add --network-policy to support updating the mode of a network policy
az aks update : Add --network-policy to support updating the mode of a network policyaz aks update: Add --network-policy to support updating the mode of a network policy
FumingZhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robogatikov, could you please rebase your PR from dev branch and requeue the live test? I bumped the SDK and default API version yesterday, so your recording file is outdated
@FumingZhang , I rebased my PR from dev branch and requeued the live test |
|
@FumingZhang @yanzhudd Could you please review this PR again? |
FumingZhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
All cases passed except one failed. The failed one is transient and not related to the change. |
… mode of a network policy (Azure#27466)
Related command
az aks update --network-policy=noneaz aks update --network-policy=azureaz aks update --network-policy=ciliumDescription
Add the --network-policy flag to the az aks update command. This will allow to customers to update cluster to enable --network-policy "azure".
Also when updating an existing cluster to enable Azure CNI Powered by Cilium, customers may specify --network-policy that they want to use explicitly (--network-policy "azure", or --network-policy "cilium", or --network-policy "none")
Testing Guide
live test recording file: test_aks_migrate_cluster_to_cilium_dataplane.txt
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.