Skip to content

Conversation

@mboersma
Copy link
Member

@mboersma mboersma commented Jun 7, 2018

Fixes an issue where az aks get-credentials didn't distinguish between the admin and user versions when merging into ~/.kube/config.

Before:

$ rm -f ~/.kube/config
$ az aks get-credentials -g aztest -n aztest
Merged "aztest" as current context in /Users/matt/.kube/config
$ az aks get-credentials -g aztest -n aztest --admin
Merged "aztest" as current context in /Users/matt/.kube/config
$ kubectl config get-contexts
CURRENT   NAME      CLUSTER   AUTHINFO                     NAMESPACE
*         aztest    aztest    clusterAdmin_aztest_aztest

After:

$ git checkout rename-admin-k8s-config
Switched to branch 'rename-admin-k8s-config'
$ rm -f ~/.kube/config
$ az aks get-credentials -g aztest -n aztest
Merged "aztest" as current context in /Users/matt/.kube/config
$ az aks get-credentials -g aztest -n aztest --admin
Merged "aztest-admin" as current context in /Users/matt/.kube/config
$ kubectl config get-contexts
CURRENT   NAME           CLUSTER   AUTHINFO                     NAMESPACE
          aztest         aztest    clusterUser_aztest_aztest    
*         aztest-admin   aztest    clusterAdmin_aztest_aztest 

cc: @slack


  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@codecov-io
Copy link

codecov-io commented Jun 8, 2018

Codecov Report

Merging #6529 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##           dev   #6529   +/-   ##
===================================
  Coverage    0%      0%           
===================================
  Files       11      11           
  Lines      133     133           
  Branches     9       9           
===================================
  Misses     133     133

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ce769a...a463c37. Read the comment docs.

@mboersma
Copy link
Member Author

mboersma commented Jun 8, 2018

Given #6497 I assume I'll have to update HISTORY.rst in this PR before it can be merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the try catch in the loop instead of out of the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes indeed.

@mboersma mboersma force-pushed the rename-admin-k8s-config branch from 9ee9439 to a463c37 Compare June 12, 2018 16:14
@mboersma
Copy link
Member Author

Rebased to fix merge conflict.

@troydai
Copy link
Contributor

troydai commented Jun 12, 2018

Ok. I'll merge it now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants