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

ignore discovery errors #67

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Conversation

bergerx
Copy link
Contributor

@bergerx bergerx commented Dec 8, 2023

Currently, it fails when any of the api-resources is failing but this should only be a warning, and there is no need to block the tree to proceed given we print some warnings about the error.

Current version:

$ kubectl  tree --version
kubectl version v0.4.3

$ kubectl tree StatefulSet/matst-gn7lu2-monitor
Error: failed to fetch api groups from kubernetes: unable to retrieve the complete list of server APIs: data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request

With this PR:

$ ./kubectl-tree StatefulSet/matst-gn7lu2-monitor
E1208 00:07:06.589076 2603851 memcache.go:287] couldn't get resource list for data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request
E1208 00:07:06.762539 2603851 memcache.go:121] couldn't get resource list for data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request
NAMESPACE  NAME                                                  READY  REASON  AGE
default    StatefulSet/matst-gn7lu2-monitor                      -              59m
default    ├─ControllerRevision/matst-gn7lu2-monitor-55895d466c  -              59m
default    └─Pod/matst-gn7lu2-monitor-0                          True           59m

I just logged the error rather than printing it to the stderr as client-go already prints the warnings ootb as you can see above. But if you run it with -v 1, you can still see the relevant error:

$ ./kubectl-tree -v1 StatefulSet/matst-gn7lu2-monitor
E1208 00:06:56.769060 2603504 memcache.go:287] couldn't get resource list for data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request
E1208 00:06:56.941860 2603504 memcache.go:121] couldn't get resource list for data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request
I1208 00:06:56.942236 2603504 apis.go:49] failed to fetch api groups from kubernetes: unable to retrieve the complete list of server APIs: data.packaging.carvel.dev/v1alpha1: the server is currently unable to handle the request
NAMESPACE  NAME                                                  READY  REASON  AGE
default    StatefulSet/matst-gn7lu2-monitor                      -              59m
default    ├─ControllerRevision/matst-gn7lu2-monitor-55895d466c  -              59m
default    └─Pod/matst-gn7lu2-monitor-0                          True           59m

@ahmetb ahmetb merged commit 8c32ac5 into ahmetb:master Dec 18, 2023
1 check passed
@ahmetb
Copy link
Owner

ahmetb commented Dec 18, 2023

I hastily merged this but now I'm not sure if this was a good idea. :/

How do we know the API list (including builtin APIs) if client.ServerPreferredResources() fails to run? In your example, how does the code know about ControllerRevision or Pod APIs to iterate on?

Also I think any errors shouldn't be .V(1), they should be always visible IMO. @bergerx

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