Skip to content

[AKS] graduate dual-stack to GA#25256

Merged
zhoxing-ms merged 9 commits into
Azure:devfrom
tyler-lloyd:aksipv6ga
Mar 1, 2023
Merged

[AKS] graduate dual-stack to GA#25256
zhoxing-ms merged 9 commits into
Azure:devfrom
tyler-lloyd:aksipv6ga

Conversation

@tyler-lloyd
Copy link
Copy Markdown
Contributor

@tyler-lloyd tyler-lloyd commented Jan 27, 2023

Related command

az aks create and az aks update

Description

This PR introduces the properties from aks-extensions that support the dual-stack feature.
Dual-stack is moving to GA and part of the work includes baking it into the GA cli to remove
the dependency on aks-preview extension.

Testing Guide

Unit and live tests added, mostly copied from the aks-preview extension work.

Example create:

Creates a dual-stack cluster with user-specified IP ranges

az aks create -n myCluster -g myRG --ip-families ipv4,ipv6 --pod-cidrs 10.244.0.0/16,fd12:3456:f::/64 --service-cidrs 192.168.0.0/16,fd00:abcd:ff::/108

Example update:

Updates the current dual-stack cluster with 4 managed outbound IPv6 IPs

az aks update -n myCluster -g myRG --load-balanacer-managed-outbound-ipv6-count 4

History Notes

[AKS] az aks create: Add new parameter --pod-cidrs for setting the IP ranges used to allocate IPs to pods
[AKS] az aks create: Add new parameter --service-cidrs for setting the K8s service IPs
[AKS] az aks create: Add new parameter --ip-families for setting the IP types that should be used in a cluster (IPv4 or IPv6)
[AKS] az aks create: Add new parameter --load-balanacer-managed-outbound-ipv6-count for setting the number of IPv6 outbound IPs that AKS should managed for a cluster with IPv6 enabled
[AKS] az aks update: Support changing the load balancer managed outbound IPv6 count property


This checklist is used to make sure that common guidelines for a pull request are followed.

@ghost ghost assigned zhoxing-ms Jan 27, 2023
@ghost ghost added this to the Jan 2023 (2023-02-07) milestone Jan 27, 2023
@ghost ghost added the Auto-Assign Auto assign by bot label Jan 27, 2023
@ghost ghost requested a review from yonzhan January 27, 2023 21:09
@ghost ghost added the AKS az aks/acs/openshift label Jan 27, 2023
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Jan 28, 2023

AKS

Copy link
Copy Markdown
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

For the lint failure, please update src/azure-cli/azure/cli/command_modules/acs/linter_exclusions.yml to bypass the check on load_balancer_managed_outbound_ipv6_count.

Please also update the PR title, as it will be included in the cli release note.


def test_configure_load_balancer_profile(self):
managed_outbound_ip_count = 5
managed_outbound_ipv6_count = 4
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.

This set of profile (hybrid_2020_09_01) is using a legacy API version 2020-11-01, does ipv6 related properties introduced at that time? If not, I think we'd better not touch these test files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so 2020-11-01 does not have IPv6 changes but the test failed when I didn't add it because of the call to configure_load_balancer_profile ... should I just pass None instead?

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.

Yes, should ensure that not setting this field (pass None) wouldn't break the function.

Copy link
Copy Markdown
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

LGTM, queued a pipeline to test newly added test cases.

else:
if (
self.get_load_balancer_managed_outbound_ip_count() or
self.get_load_balancer_managed_outbound_ipv6_count() or
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.

This validation is not included in aks-preview

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that's an oversight then. we should raise an error if outbound type is UDR and there is IPv6 count defined similar to the _ip_count parameter. I added a unit test to cover this


def test_configure_load_balancer_profile(self):
managed_outbound_ip_count = 5
managed_outbound_ipv6_count = 4
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.

Yes, should ensure that not setting this field (pass None) wouldn't break the function.

Copy link
Copy Markdown
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@zhoxing-ms
Copy link
Copy Markdown
Contributor

[AKS] az aks create: Add properties that support dual-stack, pod-cidrs, service-cidrs, ip-families, and load-balanacer-managed-outbound-ipv6-count

Please describe the feature of the new parameters more detail in the history notes, it is better to have a separate record for each parameter~

@tyler-lloyd
Copy link
Copy Markdown
Contributor Author

[AKS] az aks create: Add properties that support dual-stack, pod-cidrs, service-cidrs, ip-families, and load-balanacer-managed-outbound-ipv6-count

Please describe the feature of the new parameters more detail in the history notes, it is better to have a separate record for each parameter~

@zhoxing-ms updated the PR description with more details for each parameter. I don't see a HISTORY.rst for the GA cli so let me know if the details should go anywhere else.

@zhoxing-ms
Copy link
Copy Markdown
Contributor

zhoxing-ms commented Feb 27, 2023

[AKS] az aks create: Add properties that support dual-stack, pod-cidrs, service-cidrs, ip-families, and load-balanacer-managed-outbound-ipv6-count

  • pod-cidrs - is a comma separated list of CIDRs that are used for allocating IPs to pods
  • service-cidrs - is a comma separated list of CIDRs that are used for K8s service IPs
  • ip-families - is a comma separated list of the IP types that should be used in a cluster (IPv4 or IPv6)
  • load-balanacer-managed-outbound-ipv6-count - is the number of IPv6 outbound IPs that AKS should managed for a cluster with IPv6 enabled

Since the current history notes cannot support the display of the list format, please write a separate record for each new parameter

@tyler-lloyd
Copy link
Copy Markdown
Contributor Author

tyler-lloyd commented Feb 28, 2023

Since the current history notes cannot support the display of the list format, please write a separate record for each new parameter

@zhoxing-ms sorry what do you mean by separate record? I think I got it. Updated the description to have each parameter as a single line for the history.

@zhoxing-ms
Copy link
Copy Markdown
Contributor

image

@tyler-lloyd I have refined the description of history notes, could you help to see if it meets your expectations?

@tyler-lloyd
Copy link
Copy Markdown
Contributor Author

made one small update to the --pod-cidrs description but lgtm

@zhoxing-ms zhoxing-ms merged commit 43ceaa9 into Azure:dev Mar 1, 2023
- AZURECLI/2.44.1 azsdk-python-azure-mgmt-containerservice/21.1.0 Python/3.10.6
(Linux-5.15.0-1031-azure-x86_64-with-glibc2.35)
method: PUT
uri: https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest000001/providers/Microsoft.ContainerService/managedClusters/cliakstest000002?api-version=2022-11-01
Copy link
Copy Markdown
Contributor

@zhoxing-ms zhoxing-ms Mar 1, 2023

Choose a reason for hiding this comment

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

The api-version here should be 2023-01-01 instead of 2022-11-01 after bumping the api-version of Microsoft.ContainerService PR link: #25524

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tyler-lloyd @FumingZhang Please help pull the latest code from remote dev branch, then please re-recording these failed tests test_aks_create_and_update_ipv6_count and test_aks_create_dualstack_with_default_network

@pie-r
Copy link
Copy Markdown

pie-r commented Aug 16, 2024

We need the flag --pod-cidrs also for the az aks update. Otherwise the update in place is not possible.

@pie-r
Copy link
Copy Markdown

pie-r commented Aug 16, 2024

@yonzhan
@bragi92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AKS az aks/acs/openshift Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants