Skip to content

Added --enable-azure-rbac flag in ManagedClusterAadProfile (#2)#1887

Merged
arrownj merged 4 commits into
Azure:masterfrom
Anumita:mergeconflicts
Jul 3, 2020
Merged

Added --enable-azure-rbac flag in ManagedClusterAadProfile (#2)#1887
arrownj merged 4 commits into
Azure:masterfrom
Anumita:mergeconflicts

Conversation

@Anumita

@Anumita Anumita commented Jun 15, 2020

Copy link
Copy Markdown
Contributor

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

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 PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@Anumita

Anumita commented Jun 15, 2020

Copy link
Copy Markdown
Contributor Author

@weinong can you review the PR?

@yonzhan yonzhan requested a review from qwordy June 15, 2020 12:34
@yonzhan yonzhan added this to the S171 milestone Jun 15, 2020
@yonzhan

yonzhan commented Jun 15, 2020

Copy link
Copy Markdown
Collaborator

AKS

@gtxistxgao

gtxistxgao commented Jun 15, 2020

Copy link
Copy Markdown
Contributor

Seems that 2020-06-01 has not yet been published. https://msazure.visualstudio.com/CloudNativeCompute/_git/manifests?path=%2Frp%2Fglobal.json&version=GBmaster&line=2067&lineEnd=2068&lineStartColumn=1&lineEndColumn=25&lineStyle=plain

weinong
weinong reviewed Jun 15, 2020
View reviewed changes
Comment thread src/aks-preview/azext_aks_preview/_help.py Outdated
weinong
weinong approved these changes Jun 15, 2020
View reviewed changes

@weinong weinong left a comment

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.

LGTM. Thanks!

qwordy
qwordy reviewed Jun 16, 2020
View reviewed changes
Comment thread src/aks-preview/azext_aks_preview/custom.py Outdated
yungezz
yungezz reviewed Jun 16, 2020
View reviewed changes
Comment thread src/aks-preview/azext_aks_preview/_help.py Outdated
Comment thread src/aks-preview/azext_aks_preview/__init__.py Outdated
@Anumita Anumita force-pushed the master branch 2 times, most recently from bca3370 to 0a693bf Compare June 16, 2020 08:02
@bingosummer bingosummer mentioned this pull request Jun 19, 2020
Merged
3 tasks
@arrownj

arrownj commented Jun 23, 2020

Copy link
Copy Markdown
Contributor

Hi @bingosummer , could you please help resolve the merge conflict ?

bingosummer
bingosummer reviewed Jun 29, 2020
View reviewed changes
Comment thread src/aks-preview/HISTORY.md Outdated
bingosummer
bingosummer reviewed Jun 29, 2020
View reviewed changes
Comment thread src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Outdated
@Anumita Anumita force-pushed the mergeconflicts branch 3 times, most recently from 96bcebc to e514af5 Compare July 1, 2020 06:43
@palma21

palma21 commented Jul 1, 2020

Copy link
Copy Markdown
Member

@Anumita are we close to have this merged? Help needed?

@Anumita

Anumita commented Jul 2, 2020

Copy link
Copy Markdown
Contributor Author

@Anumita are we close to have this merged? Help needed?

Hey, the builds are failing due to some tests failing for all pr right now. I have contacted the poc for the tests. Once that is fixed , i think the pr needs to be approved by gtracer or zquingqing1 and then can be merged.

bingosummer
bingosummer reviewed Jul 2, 2020
View reviewed changes
Comment thread ...w/azext_aks_preview/tests/latest/recordings/test_aks_byo_appgw_with_ingress_appgw_addon.yaml Outdated

@bingosummer bingosummer Jul 2, 2020

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.

Is "bad request" expected?

@Anumita Anumita Jul 2, 2020

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.

not sure, i ran the tests again just to be sure, but getting the same

zqingqing1
zqingqing1 approved these changes Jul 2, 2020
View reviewed changes

@zqingqing1 zqingqing1 left a comment

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.

lgtm

@arrownj arrownj merged commit 0ee6b82 into Azure:master Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gtracer gtracer Awaiting requested review from gtracer

9 more reviewers

@jluk jluk jluk left review comments

@bingosummer bingosummer bingosummer approved these changes

@palma21 palma21 palma21 left review comments

@yungezz yungezz yungezz left review comments

@arrownj arrownj arrownj approved these changes

@weinong weinong weinong approved these changes

@shashankbarsin shashankbarsin shashankbarsin approved these changes

@qwordy qwordy qwordy approved these changes

@zqingqing1 zqingqing1 zqingqing1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@arrownj arrownj

@yungezz yungezz

Labels

do-not-merge

Projects

None yet

Milestone

S171

Development

Successfully merging this pull request may close these issues.