Skip to content
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

{AKS} Fix enabled virtual node addon showing wrong status in aks addon list #5099

Merged
merged 3 commits into from
Jul 13, 2022

Conversation

navba-MSFT
Copy link
Contributor

@navba-MSFT navba-MSFT commented Jul 11, 2022

fixes #5098

az aks addon list command always returns "enable:false" for virtual-node addon. REST API returns "enable:true", but CLI displays result as "enable:false"

az aks addon list -g TestRG -n TestCluster --debug
# REST API returns the addon status to be "enabled:true" in debug trace.
   "addonProfiles": {
    "aciConnectorLinux": {
     "enabled": true,            <<<<<<<
     "config": {
      "SubnetName": "VirtualNodeSubnet"
     },


#However, CLI displays "enabled:false". It appears to be matching with "aciConnector" instead of "aciConnectorLinux".

  {
    "api_key": "aciConnector",
    "enabled": false,           <<<<<<<<<
    "name": "virtual-node"
  },

As per this and this source code we see that Linux is appended to the VirtualNode addon, so this check should be done while listing the addon:

os_type = 'Linux'
enable_virtual_node = False
if CONST_VIRTUAL_NODE_ADDON_NAME + os_type in instance.addon_profiles:
        enable_virtual_node = True
        addon = ADDONS[addon_arg]
        if addon == CONST_VIRTUAL_NODE_ADDON_NAME:
            # only linux is supported for now, in the future this will be a user flag
            addon += os_type

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

Related command

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?

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.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

fixes Azure#5098

az aks addon list command always returns "enable:false" for virtual-node addon. REST API returns "enable:true", but CLI displays result as "enable:false"

```
> az aks addon list -g TestRG -n TestCluster --debug
# REST API returns the addon status to be "enabled:true" in debug trace.
   "addonProfiles": {
    "aciConnectorLinux": {
     "enabled": true,            <<<<<<<
     "config": {
      "SubnetName": "VirtualNodeSubnet"
     },


#However, CLI displays "enabled:false". It appears to be matching with "aciConnector" instead of "aciConnectorLinux".

  {
    "api_key": "aciConnector",
    "enabled": false,           <<<<<<<<<
    "name": "virtual-node"
  },
```
@yonzhan
Copy link
Collaborator

yonzhan commented Jul 11, 2022

AKS

@yonzhan yonzhan requested a review from zhoxing-ms July 11, 2022 12:57
@yonzhan yonzhan added this to the Jul 2022 (2022-08-02) milestone Jul 11, 2022
@yonzhan yonzhan requested a review from jsntcy July 11, 2022 12:57
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

@zhoxing-ms
Copy link
Contributor

Could you please add a test for this case?

@zhoxing-ms
Copy link
Contributor

By the way, please add the description of this change in the history notes HISTORY.rst. If you want to release a new version for this change, please also modify the setup.py

@FumingZhang FumingZhang changed the title {AzureKubernetesService} fixes Azure/azure-cli-extensions#5098 {AKS} Fix enabled virtual node addon showing wrong status in aks addon list Jul 13, 2022
@FumingZhang
Copy link
Member

Could you please add a test for this case?

I've opened a patch PR #5113 to add a test case for this modification.

@zhoxing-ms zhoxing-ms merged commit b585fdb into Azure:main Jul 13, 2022
@navba-MSFT navba-MSFT added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved.
Projects
None yet
4 participants