fix: ignore kubeconfig parse errors when listing profiles#23300
fix: ignore kubeconfig parse errors when listing profiles#23300immanuwell wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: immanuwell The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @immanuwell! |
|
Hi @immanuwell. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Can one of the admins verify this patch? |
|
@immanuwell Thanks of the change. I could reproduce the issue and created #23301. Let's discuss the wanted behavior first in the issue. |
nirs
left a comment
There was a problem hiding this comment.
@immanuwell Thanks!, see suggestion improving the warning.
4f8e482 to
ef12374
Compare
|
@nirs addressed all your suggestions, PTAL |
Testing with the fix:minikube profile list% minikube profile list
❗ Unable to determine the current kubeconfig context: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
E0711 22:44:15.321842 28706 status.go:457] kubeconfig endpoint: get endpoint: read kubeconfig: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
┌──────────┬────────┬─────────┬──────────────┬─────────┬────────┬───────┬────────────────┬────────────────────┐
│ PROFILE │ DRIVER │ RUNTIME │ IP │ VERSION │ STATUS │ NODES │ ACTIVE PROFILE │ ACTIVE KUBECONTEXT │
├──────────┼────────┼─────────┼──────────────┼─────────┼────────┼───────┼────────────────┼────────────────────┤
│ minikube │ vfkit │ docker │ 192.168.64.3 │ v1.36.2 │ OK │ 1 │ * │ │
└──────────┴────────┴─────────┴──────────────┴─────────┴────────┴───────┴────────────────┴────────────────────┘Works - we get the profiles now. Issues:
❗ Kubeconfig '/Users/nir/.kube/config' is invalid, unable to determine active contextminikube delete --all% minikube delete --all
❗ Unable to determine the current kubeconfig context: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
🔥 Deleting "minikube" in vfkit ...
🔥 Trying to delete invalid profile minikube
🔥 Successfully deleted all profilesWorks - profile was deleted. Issues:
minikube stop --all% minikube stop --all
❗ Unable to determine the current kubeconfig context: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
✋ Stopping node "minikube" ...
❌ Exiting due to HOST_KUBECONFIG_DELETE_CTX: delete ctx: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
╭───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ │
│ 😿 If the above advice does not help, please let us know: │
│ 👉 https://github.com/kubernetes/minikube/issues/new/choose │
│ │
│ Please run `minikube logs --file=logs.txt` and attach logs.txt to the GitHub issue. │
│ Please also attach the following file to the GitHub issue: │
│ - /var/folders/v1/lcvpk6v567x43cfsjqg_97t40000gn/T/minikube_stop_5644b90c1b481b75163323bcf8cd2eecc0b02f39_0.log │
│ │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯Works - profile was stopped Issues:
minikube addons list% minikube addons list
┌─────────────────────────────┬──────────┬────────────┬────────────────────────────────────────┐
│ ADDON NAME │ PROFILE │ STATUS │ MAINTAINER │
├─────────────────────────────┼──────────┼────────────┼────────────────────────────────────────┤
│ ambassador │ minikube │ disabled │ 3rd party (Ambassador) │
│ amd-gpu-device-plugin │ minikube │ disabled │ 3rd party (AMD) │
│ auto-pause │ minikube │ disabled │ minikube │
│ cloud-spanner │ minikube │ disabled │ Google │
│ csi-hostpath-driver │ minikube │ disabled │ Kubernetes │
│ dashboard │ minikube │ disabled │ Kubernetes │
│ default-storageclass │ minikube │ enabled ✅ │ Kubernetes │
│ efk │ minikube │ disabled │ 3rd party (Elastic) │
│ freshpod │ minikube │ disabled │ Google │
│ gcp-auth │ minikube │ disabled │ Google │
│ gvisor │ minikube │ disabled │ minikube │
│ headlamp │ minikube │ disabled │ 3rd party (kinvolk.io) │
│ inaccel │ minikube │ disabled │ 3rd party (InAccel [info@inaccel.com]) │
│ ingress │ minikube │ disabled │ Kubernetes │
│ ingress-dns │ minikube │ disabled │ minikube │
│ inspektor-gadget │ minikube │ disabled │ 3rd party (inspektor-gadget.io) │
│ istio │ minikube │ disabled │ 3rd party (Istio) │
│ istio-provisioner │ minikube │ disabled │ 3rd party (Istio) │
│ kong │ minikube │ disabled │ 3rd party (Kong HQ) │
│ kubeflow │ minikube │ disabled │ 3rd party │
│ kubetail │ minikube │ disabled │ 3rd party (kubetail.com) │
│ kubevirt │ minikube │ disabled │ 3rd party (KubeVirt) │
│ logviewer │ minikube │ disabled │ 3rd party (unknown) │
│ metallb │ minikube │ disabled │ 3rd party (MetalLB) │
│ metrics-server │ minikube │ disabled │ Kubernetes │
│ nvidia-device-plugin │ minikube │ disabled │ 3rd party (NVIDIA) │
│ nvidia-driver-installer │ minikube │ disabled │ 3rd party (NVIDIA) │
│ nvidia-gpu-device-plugin │ minikube │ disabled │ 3rd party (NVIDIA) │
│ olm │ minikube │ disabled │ 3rd party (Operator Framework) │
│ pod-security-policy │ minikube │ disabled │ 3rd party (unknown) │
│ portainer │ minikube │ disabled │ 3rd party (Portainer.io) │
│ registry │ minikube │ disabled │ minikube │
│ registry-aliases │ minikube │ disabled │ 3rd party (unknown) │
│ registry-creds │ minikube │ disabled │ 3rd party (UPMC Enterprises) │
│ storage-provisioner │ minikube │ enabled ✅ │ minikube │
│ storage-provisioner-rancher │ minikube │ disabled │ 3rd party (Rancher) │
│ volcano │ minikube │ disabled │ third-party (volcano) │
│ volumesnapshots │ minikube │ disabled │ Kubernetes │
│ yakd │ minikube │ disabled │ 3rd party (marcnuri.com) │
└─────────────────────────────┴──────────┴────────────┴────────────────────────────────────────┘
❗ Unable to determine the current kubeconfig context: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'Works. Issues:
minikube profile% minikube profile minikube
❗ Unable to determine the current kubeconfig context: Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
😿 Error while setting kubectl current context : Error getting kubeconfig status: decode kubeconfig from "/Users/nir/.kube/config": decode data: not: [valid
: yaml: line 1: did not find expected ',' or ']'
✅ minikube profile was successfully set to minikubeBetter - 2 errors replaced with one warning. Issues:
|
nirs
left a comment
There was a problem hiding this comment.
This change is a great improvement, but we still have issues.
I tested with diff:
diff --git a/pkg/minikube/config/profile_test.go b/pkg/minikube/config/profile_test.go
index 59df40341..2db823d18 100644
--- a/pkg/minikube/config/profile_test.go
+++ b/pkg/minikube/config/profile_test.go
@@ -86,7 +86,7 @@ func TestListProfiles(t *testing.T) {
nir@space-gray minikube (fix-list-profiles-invalid-kubeconfig)% git show | cat
commit a7af78f8fcaa24139aaf4b7c2ffb06ff5a58777c
Author: Nir Soffer <nirsof@gmail.com>
Date: Sat Jul 11 19:48:43 2026 +0300
review suggestions
diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go
index e54d80dd3..6c6fe6dba 100644
--- a/pkg/minikube/config/profile.go
+++ b/pkg/minikube/config/profile.go
@@ -191,21 +191,18 @@ var DockerContainers = func() ([]string, error) {
// invalidPs are the profiles that have a directory or config file but not usable
// invalidPs would be suggested to be deleted
func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, err error) {
- return listProfiles(kubeconfig.PathFromEnv(), DockerContainers, miniHome...)
+ containers, _ := DockerContainers()
+ return listProfiles(kubeconfig.PathFromEnv(), containers, miniHome...)
}
-func listProfiles(kubeconfigPath string, dockerContainers func() ([]string, error), miniHome ...string) (validPs []*Profile, inValidPs []*Profile, err error) {
+func listProfiles(kubeconfigPath string, containers []string, miniHome ...string) (validPs []*Profile, inValidPs []*Profile, err error) {
activeP := viper.GetString(ProfileName)
// try to get profiles list based on left over evidences such as directory
pDirs, err := profileDirs(miniHome...)
if err != nil {
return nil, nil, err
}
- // try to get profiles list based on all containers created by docker driver
- cs, err := dockerContainers()
- if err == nil {
- pDirs = append(pDirs, cs...)
- }
+ pDirs = append(pDirs, containers...)
activeKubeContext := ""
if ctx, err := kubeconfig.GetCurrentContext(kubeconfigPath); err != nil {
diff --git a/pkg/minikube/config/profile_test.go b/pkg/minikube/config/profile_test.go
index 59df40341..2db823d18 100644
--- a/pkg/minikube/config/profile_test.go
+++ b/pkg/minikube/config/profile_test.go
@@ -86,7 +86,7 @@ func TestListProfiles(t *testing.T) {
func TestListProfilesWithInvalidKubeconfig(t *testing.T) {
miniDir, err := filepath.Abs("./testdata/profile/.minikube")
if err != nil {
- t.Errorf("error getting dir path for ./testdata/.minikube : %v", err)
+ t.Fatalf("error getting dir path for ./testdata/.minikube : %v", err)
}
tmpDir := t.TempDir()
@@ -94,9 +94,7 @@ func TestListProfilesWithInvalidKubeconfig(t *testing.T) {
if err := os.WriteFile(kubeconfig, []byte("not: [valid\n"), 0600); err != nil {
t.Fatalf("failed to write invalid kubeconfig: %v", err)
}
- val, inv, err := listProfiles(kubeconfig, func() ([]string, error) {
- return []string{}, nil
- }, miniDir)
+ val, inv, err := listProfiles(kubeconfig, nil, miniDir)
if err != nil {
t.Fatalf("ListProfiles returned unexpected error: %v", err)
}| func TestListProfilesWithInvalidKubeconfig(t *testing.T) { | ||
| miniDir, err := filepath.Abs("./testdata/profile/.minikube") | ||
| if err != nil { | ||
| t.Errorf("error getting dir path for ./testdata/.minikube : %v", err) |
| return listProfiles(kubeconfig.PathFromEnv(), DockerContainers, miniHome...) | ||
| } | ||
|
|
||
| func listProfiles(kubeconfigPath string, dockerContainers func() ([]string, error), miniHome ...string) (validPs []*Profile, inValidPs []*Profile, err error) { |
There was a problem hiding this comment.
Accepting function returning profiles makes the code harder to follow and testing harder. We need to create functions returning lists.
Accept list of containers instead.
| // try to get profiles list based on all containers created by docker driver | ||
| cs, err := DockerContainers() | ||
| cs, err := dockerContainers() | ||
| if err == nil { |
There was a problem hiding this comment.
Can be removed if we accept list of containers.
| } | ||
| val, inv, err := listProfiles(kubeconfig, func() ([]string, error) { | ||
| return []string{}, nil | ||
| }, miniDir) |
There was a problem hiding this comment.
Will be much simpler if we pass nil list of containers.
minikube profile listshould not faceplant whenKUBECONFIGis malformed.We only read kubeconfig here to mark the active kubecontext column. This keeps profile discovery working and just leaves
ActiveKubeContextunset if kubeconfig parsing blows up. small bug, pretty user visible.Repro:
Before this, profile discovery could fail and no profiles were returned.
After this, profiles still show up, just no active kubecontext marker. all good.
Tests: