Skip to content

Conversation

@MartinForReal
Copy link
Contributor

@MartinForReal MartinForReal commented May 9, 2025


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

Related command

az update

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented May 9, 2025

⚠️Azure CLI Extensions Breaking Change Test
⚠️aks-preview
rule cmd_name rule_message suggest_message
⚠️ 1006 - ParaAdd aks update cmd aks update added parameter load_balancer_sku

@azure-client-tools-bot-prd
Copy link

Hi @MartinForReal,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented May 9, 2025

Thank you for your contribution! We will review the pull request and get back to you soon.

@github-actions
Copy link

github-actions bot commented May 9, 2025

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

@github-actions
Copy link

github-actions bot commented May 9, 2025

@MartinForReal
Copy link
Contributor Author

related: Azure/AKS#1687

@MartinForReal MartinForReal reopened this May 21, 2025
@MartinForReal MartinForReal marked this pull request as ready for review May 21, 2025 13:47
Copilot AI review requested due to automatic review settings May 21, 2025 13:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for migrating from Basic to Standard Azure Load Balancer SKU in AKS commands and decorators.

  • Add a new --load-balancer-sku parameter for aks update and update decorators to apply the SKU change.
  • Extend unit and integration tests to cover creating with Basic LB and updating to Standard LB.
  • Update CLI parameter registration, help documentation, version bump, and changelog.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/aks-preview/setup.py Bump extension version to 18.0.0b3
src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py Add blank lines cleanup and migration unit test for update
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Add E2E test for Basic→Standard LB migration; adjust vm-set-type casing
src/aks-preview/azext_aks_preview/managed_cluster_decorator.py Introduce _get_load_balancer_sku and apply SKU in update flow
src/aks-preview/azext_aks_preview/custom.py Expose load_balancer_sku in aks_update signature
src/aks-preview/azext_aks_preview/_params.py Register load_balancer_sku enum (currently only "standard")
src/aks-preview/azext_aks_preview/_help.py Document --load-balancer-sku parameter
src/aks-preview/HISTORY.rst Add changelog entry for basic LB migration
Comments suppressed due to low confidence (4)

src/aks-preview/azext_aks_preview/_params.py:1001

  • The enum for load_balancer_sku only allows standard, preventing passing basic when creating with Basic LB. Include CONST_LOAD_BALANCER_SKU_BASIC in the choices or separate parameter registration for create vs. update commands.
c.argument("load_balancer_sku", arg_type=get_enum_type([CONST_LOAD_BALANCER_SKU_STANDARD]),

src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py:367

  • [nitpick] The vm-set-type value availabilityset should match the canonical casing availabilitySet or VirtualMachineScaleSets to avoid confusion and align with other tests.
--vm-set-type availabilityset -c 1 

src/aks-preview/azext_aks_preview/managed_cluster_decorator.py:350

  • You added _get_load_balancer_sku to the decorator class, but update_network_profile calls self.context.get_load_balancer_sku(). Implement get_load_balancer_sku on the context class or route calls correctly to avoid an AttributeError at runtime.
def _get_load_balancer_sku(self, enable_validation: bool = False) -> Union[str, None]:

src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py:4301

  • [nitpick] There are two consecutive blank lines before def test_set_up_api_server_access_profile. Remove one to match project style guidelines.
+

Copy link
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.

Queued live test to validate the change, test passed!

  • test_aks_create_with_basiclb_and_update_to_standardlb

@MartinForReal MartinForReal requested a review from FumingZhang May 22, 2025 01:12
@FumingZhang
Copy link
Member

Please fix the failed CI checks

        load_balancer_sku = self.context.get_load_balancer_sku()
        if load_balancer_sku != CONST_LOAD_BALANCER_SKU_BASIC:
>           network_profile.nat_gateway_profile = nat_gateway_profile
E           AttributeError: 'NoneType' object has no attribute 'nat_gateway_profile'

../azure-cli/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py:6000: AttributeError

https://dev.azure.com/azclitools/public/_build/results?buildId=246636&view=logs&j=155127ab-52f1-52e7-0e45-625d9d7884a4&t=4bb67950-ef03-52d4-44f5-d1a8cd56c919&l=41865

Copy link
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

live test passed!

@zhoxing-ms zhoxing-ms merged commit bde8dce into Azure:main May 23, 2025
32 checks passed
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ aks-preview-18.0.0b3 ] : https://dev.azure.com/msazure/One/_build/results?buildId=125122846&view=results

bavneetsingh16 pushed a commit to AzureArcForKubernetes/k8s-extension that referenced this pull request May 23, 2025
* add support for basic lb migration

* add support for basic lb migration
@sata-sa
Copy link

sata-sa commented Jun 4, 2025

Is this already available on AZ CLI 2.74-0?

Didn't see any mention on the release notes...

@MartinForReal
Copy link
Contributor Author

@sata-sa The feature is still in preview. So the cli change is only available in aks-preview extension.

@sata-sa
Copy link

sata-sa commented Jun 11, 2025

@MartinForReal

It seems although that is still not possible to use:
image

Command used: az aks update --load-balancer-sku standard -g RG_NAME -n AKS_NAME --debug

@MartinForReal
Copy link
Contributor Author

@sata-sa, please register feature Microsoft.ContainerService/BasicLBMigrationToStandardLBPreview first. The doc is still under preview.

@furystormss
Copy link

@MartinForReal With around 3.5 months to go untill deprecation of the basic LB, any estimate when the first stable release will be?

@MartinForReal
Copy link
Contributor Author

I've left Azure. The feature will be announced soon. Please stay tunned.

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

Labels

AKS Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants