Allow setting multiple path in KUBECONFIG #4138
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
| @@ -70,14 +70,25 @@ func ResolveConfig(kubeconfig string) (string, error) { | |||
|
|
|||
| // CreateInterface is a helper function to create Kubernetes interface | |||
| func CreateInterface(kubeconfig string) (*rest.Config, kubernetes.Interface, error) { | |||
There was a problem hiding this comment.
This is used by both the pilot server and istioctl. I believe that the issue you are trying to fix is from istioctl. I don't think pilot is ready to support this yet.
There was a problem hiding this comment.
@baodongli I do not think there is difference between pilot server and istioctl...if pilot server does not specify kubeconfig, in cluster config will be used. This is to ensure that the kubeconfig loaded by istioctl is the same as what is loaded by kubernetes client.
| config, err := clientcmd.BuildConfigFromFlags("", kube) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| kubeconfigenv := os.Getenv("KUBECONFIG") |
There was a problem hiding this comment.
Can you put this to a helper function name as ResolveMultiPathConfig or some others
| if kubeconfig != "" || kubeconfigenv != "" { | ||
| loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() | ||
| loadingRules.ExplicitPath = kubeconfig | ||
| config, err = clientcmd.NewNonInteractiveDeferredLoadingClientConfig( |
There was a problem hiding this comment.
Add a new test for this in controller_test.go？
Codecov Report
@@ Coverage Diff @@
## master #4138 +/- ##
=======================================
- Coverage 71% 71% -<1%
=======================================
Files 324 317 -7
Lines 29547 28882 -665
=======================================
- Hits 20763 20218 -545
+ Misses 7531 7443 -88
+ Partials 1253 1221 -32
Continue to review full report at Codecov.
|
| if err != nil { | ||
| return nil, nil, err | ||
| // switch to use in-cluster config | ||
| if config == nil { |
There was a problem hiding this comment.
Why not put the in-cluster logic to LoadConfigFromMultiPath?
| @@ -0,0 +1,74 @@ | |||
| // Copyright 2017 Istio Authors | |||
|
/ok-to-test |
|
/retest |
|
@qiujian16 PR needs rebase |
|
This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last month. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Change to user default loading rules that will automatically load config in KUBECONFIG environment variable.
fix #4081