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

load kubeconfig filepath when load rest config fail #1201

Merged
merged 1 commit into from
Apr 28, 2024

Conversation

HeavenTonight
Copy link
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

fix(kubean-admission):load kubeconfig filepath when load rest config fail

Which issue(s) this PR fixes:

None

@ErikJiang
Copy link
Member

What is the purpose of this modification?
How can we obtain the kubeconfig file within the container image?

@HeavenTonight
Copy link
Contributor Author

What is the purpose of this modification? How can we obtain the kubeconfig file within the container image?

resetConfig, err := rest.InClusterConfig()
if err != nil {
klog.ErrorS(err, " can not load k8s config")
return err
}
resetConfig.QPS = opt.KubeAPIQPS
resetConfig.Burst = opt.KubeAPIBurst
if err != nil {
resetConfig, err = clientcmd.BuildConfigFromFlags("", os.Getenv("HOME")+"/.kube/config")
if err != nil {
return err
}
}

Because this is code logic error, when first err not nil, below code will never be executed!

resetConfig, err = clientcmd.BuildConfigFromFlags("", os.Getenv("HOME")+"/.kube/config")

same logic you can reference to this code:

resetConfig, err := rest.InClusterConfig()
if err != nil {
resetConfig, err = clientcmd.BuildConfigFromFlags("", os.Getenv("HOME")+"/.kube/config")
if err != nil {
return err
}
}

@HeavenTonight
Copy link
Contributor Author

What is the purpose of this modification? How can we obtain the kubeconfig file within the container image?

And then

How can we obtain the kubeconfig file within the container image?

I don't know why we use it this way, like this code:

resetConfig, err = clientcmd.BuildConfigFromFlags("", os.Getenv("HOME")+"/.kube/config")

resetConfig, err = clientcmd.BuildConfigFromFlags("", os.Getenv("HOME")+"/.kube/config")

@ErikJiang
Copy link
Member

😀 I think it is possible to consider commenting out the BuildConfigFromFlags method used for debugging purposes,
Please refer to:https://github.com/hwameistor/hwameistor/blob/v0.14.2/pkg/pvc-autoresizer/autoresizer.go#L152-L153

@HeavenTonight
Copy link
Contributor Author

😀 I think it is possible to consider commenting out the BuildConfigFromFlags method used for debugging purposes, Please refer to:https://github.com/hwameistor/hwameistor/blob/v0.14.2/pkg/pvc-autoresizer/autoresizer.go#L152-L153

I also think it should be like this. I will fix it.

@HeavenTonight
Copy link
Contributor Author

Please review again, thanks! @ErikJiang

@ErikJiang
Copy link
Member

Signed-off-by: guiyong.ou <guiyong.ou@daocloud.io>
@ErikJiang ErikJiang merged commit 8d2da98 into kubean-io:main Apr 28, 2024
6 checks passed
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.

2 participants