Conversation
|
This pull request has been linked to Clubhouse Story #15962: Checks for required cluster API resources. |
| const checkName = "kubernetes-resources" | ||
| results := make([]*api.CheckResult, 0) | ||
| dc := k.client.Discovery() | ||
| lists, err := dc.ServerPreferredResources() |
There was a problem hiding this comment.
the Discovery APIs unfortunately don't accept a context, which means they can't be cancelled -- it's why the version checks re-implement the Discovery stuff
I filed a bug but 🤷♂️ I don't think anything can be done about it, really https://github.com/kubernetes/client-go/issues/1001
There was a problem hiding this comment.
That's unfortunate :( At least here, the worst case is the user gets impatient and hits ^C.
| dc := k.client.Discovery() | ||
| lists, err := dc.ServerPreferredResources() | ||
| if err != nil { | ||
| results = append(results, api.ErrorResult(checkName, "unable to fetch api resources from server", err)) |
There was a problem hiding this comment.
should this be SKIP rather than FAIL? after all, it's an indeterminate result - we don't know that the API resources are missing, and we don't know that they're available, either
There was a problem hiding this comment.
Not being able to check for API resources implies a lack of permissions or some other issue, which I'd argue is a FAIL.
Edit: nah you're right it's a skip. I've to add more resources, so will do this in another PR.
2 participants
Firstly some renaming of the existing RBACRequirements to a more general kind of ResourceRequirement, and making the requirements a map for easier lookup.
Also moved the placed where the requirements are defined to its own file. It seemed right at the time.
The unit tests for this one are a bit of a mess -- they work for now, but I'd be interested to get some input on making them less horrible.