Skip to content

CRDs: wait for CRDs instead of exiting - #169

Merged
darkmuggle merged 2 commits into
mainfrom
bh/pool-on-crds
Jul 30, 2026
Merged

CRDs: wait for CRDs instead of exiting#169
darkmuggle merged 2 commits into
mainfrom
bh/pool-on-crds

Conversation

@darkmuggle

Copy link
Copy Markdown
Contributor

Adjusted the controller startup logic to use a discovery client to wait for CRDs instead of just existing. The new behavior is more consistent with the other Kubernetes Operators.

Resolves #166

@darkmuggle
darkmuggle requested a review from lukebond July 27, 2026 18:09

@lukebond lukebond left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this! you've got the logic right but for a couple of rusty things.

however something else i would push back on a little... we're doing this PR because we want it to more gracefully handle the fact that the CRDs aren't there yet, it needn't go into crashloop backoff and have people waiting ages for it. but nor do we want it silently unready, forever, if it's a scenario that gets an infinite retry but doesn't surface it loudly somewhere. there is the log message but i think we can do better.

one solution is to add a new ready endpoint that reflects that all controllers have passed wait_for_crds and then moving the health endpoint to a liveness probe, but that's got too much blast radius for an open source project and for a relatively trivial fixing PR that you're trying to do. and it requires a chart change. but i do want to avoid the silent unreadiness reporting as ready somehow, in a proportional way for this PR.

i would offer two suggestions:

  1. a metric that we can alert on to detect a controller that's waiting forever for CRDs that aren't coming
  2. emit a k8s event, attached tot he opererator's pod or deployment, e.g. with message reason: WaitingForCRD, so the API can aggregate repeated occurrences of it. it also needs RBAC 🫠 - i'm sure you know all this though!

we could do both, but i think we should at least do the k8s event. up to you if you want to do both in this PR or file the metric as a follow-up.

Comment thread src/controllers/mod.rs Outdated
Comment thread src/controllers/mod.rs Outdated
Comment thread src/controllers/mod.rs Outdated
Adjusted the controller startup logic to use a discovery client to
wait for CRDs instead of just existing. The new behavior is more
consistent with the other Kubernetes Operators.

Additionally, health checks and events are emitted to indicate
controllers are not ready.

Resolves #166
@darkmuggle

Copy link
Copy Markdown
Contributor Author

@lukebond fixed up, let me know what you think.

@lukebond lukebond left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great. you went and did the whole health & readiness thing! that's great :)

however it might be a surprise to some users, especially those who get a chart bump but not a container image bump, it would wait forever for the non-existent ready endpoint to pass. this is not a problem, just a problem if it's a surprise.

so i suggest we do a major version bump of the helm chart and the operator version itself. not everyone reads release notes but a major version bump gives users the best chance of doing so.

so please go ahead and do that, it's good education in the process anyway (none of which will be surprising to you i'm sure). you've already got the release note ready to go. i think it'll be a good time with the recent CRD packaging changes.

@darkmuggle
darkmuggle merged commit aae3289 into main Jul 30, 2026
5 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Operator should tolerate missing optional CRDs instead of exiting at startup

2 participants