Skip to content

[AKS] Add options for users to specify guardrails#6235

Closed
NickKeller wants to merge 34 commits into
Azure:mainfrom
NickKeller:nikelle/aks-guardrails
Closed

[AKS] Add options for users to specify guardrails#6235
NickKeller wants to merge 34 commits into
Azure:mainfrom
NickKeller:nikelle/aks-guardrails

Conversation

@NickKeller
Copy link
Copy Markdown
Member

@NickKeller NickKeller commented Apr 24, 2023


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

Related command

az aks create|update -g <RGNAME> -n <CLUSTERNAME> --guardrails-level Warning --guardrails-version v1.0.0 --guardrails-excluded-ns ns1,ns2 --enable-addons azure-policy

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

Hi @NickKeller,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@ghost ghost requested review from yanzhudd and zhoxing-ms April 24, 2023 18:30
@ghost ghost assigned zhoxing-ms Apr 24, 2023
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Apr 24, 2023

AKS

@ghost ghost added the Auto-Assign Auto assign by bot label Apr 24, 2023
@ghost ghost requested a review from yonzhan April 24, 2023 18:30
@ghost ghost added the AKS label Apr 24, 2023
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.

Queued live test pipeline to verify the newly added test cases.

Comment thread src/aks-preview/HISTORY.rst Outdated
Comment thread src/aks-preview/azext_aks_preview/_params.py
Comment thread src/aks-preview/azext_aks_preview/_params.py Outdated
Comment thread src/aks-preview/azext_aks_preview/managed_cluster_decorator.py Outdated
Comment thread src/aks-preview/azext_aks_preview/_help.py Outdated
Comment thread src/aks-preview/azext_aks_preview/managed_cluster_decorator.py
Comment thread src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Outdated
@zhoxing-ms
Copy link
Copy Markdown
Contributor

@NickKeller Could you please resolve these CI issues?

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

Comment thread src/aks-preview/HISTORY.rst Outdated
Comment thread src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Outdated
@NickKeller
Copy link
Copy Markdown
Member Author

@NickKeller Could you please resolve these CI issues?

@zhoxing-ms These tests don't fail when I run azdev test --no-exitfirst --discover aks-preview locally. Is there another flag I should add?

@zhoxing-ms
Copy link
Copy Markdown
Contributor

@NickKeller Could you please try to re-record the test test_set_up_addon_profiles in live mode? Such as azdev test test_set_up_addon_profiles --live

@zhoxing-ms zhoxing-ms closed this May 29, 2023
@zhoxing-ms zhoxing-ms reopened this May 29, 2023
@NickKeller
Copy link
Copy Markdown
Member Author

@zhoxing-ms I'm running it now, but I fail to see how that will help, considering it's a unit test, code of which I did not touch, is failing

@zhoxing-ms
Copy link
Copy Markdown
Contributor

    self.assertEqual(dec_mc_2, ground_truth_mc_2)
    AssertionError: <azex[67 chars]iew.models._models_py3.ManagedCluster object at 0x7f9f279a0fa0> != <azex[67 chars]iew.models._models_py3.ManagedCluster object at 0x7f9f279a3190>

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

  • generated xml file: /home/cloudtest/.azdev/env_config/mnt/vss/_work/1/s/env/test_results.xml -
    =========================== short test summary info ============================
    FAILED src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py::AKSPreviewManagedClusterCreateDecoratorTestCase::test_set_up_addon_profiles

This seems to be some logic issue from the unit testing itself + @FumingZhang help take a look~

@FumingZhang
Copy link
Copy Markdown
Member

Checked the error, should change "True" to "true" in line 3740 of file test_managed_cluster_decorator.py. Probably caused by change in previous PR #6311.

@FumingZhang
Copy link
Copy Markdown
Member

Replaced by PR #6357

@FumingZhang FumingZhang closed this Jun 2, 2023
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.

5 participants