Repository navigation
feat: add tolerations support - #855
Conversation
|
|
2afaa4e to
bed96c1
Compare
Add scheduling tolerations at both levels asked for in k8sgpt-ai#767: - `spec.tolerations` (`[]corev1.Toleration`) on the K8sGPT CRD, copied into the managed K8sGPT Deployment pod template in GetDeployment. - `controllerManager.tolerations` in the operator Helm chart, rendered into the controller-manager pod template, following the existing nodeSelector pattern. Original implementation by @preko-p in k8sgpt-ai#820; this is that change rebased onto main. The conflict in chart/operator/crds/k8sgpt-crd.yaml is resolved and both CRD copies are now the output of the pinned controller-gen v0.21.0 (`make manifests`), so the tolerations schema matches the rest of the generated file and the chart copy is byte-identical to the generated one. Fixes k8sgpt-ai#767 Co-authored-by: preko-p <278021202+preko-p@users.noreply.github.com> Signed-off-by: CJstate <142857225+CJstate@users.noreply.github.com>
AlexsJones
left a comment
There was a problem hiding this comment.
Thanks @CJstate, this is a careful carry-forward of #820, and regenerating both CRD copies with the pinned controller-gen instead of hand-merging was the right move.
The change matches what was agreed on #767: spec.tolerations copied into the K8sGPT Deployment in GetDeployment, and controllerManager.tolerations in the chart behind the same if guard as nodeSelector, at pod-spec level. The test covers both fields. I approved the fork runs, and tests and the container build passed on this exact tree (your re-authored commit has the same tree as 2afaa4e).
Two things before it can land: EasyCLA still needs your signature, and which of #820 and #855 goes in is a maintainer call, so I've flagged that.
|
Thanks for the review and the approval, @AlexsJones — CI is green on the current head ( The only red check left is the EasyCLA signature, and that one is on my side, so please hold off merging for a moment while I get it sorted. (The commit author is me for exactly that reason; |
|
@AlexsJones EasyCLA is now signed and all checks are green. Thanks for flagging the maintainer call on #820 vs #855 — ready whenever the team decides how you'd like to proceed. |
|
Correction to my previous comment — nothing is pending on my side any more. EasyCLA re-evaluated this PR and the check on the current head ( So please disregard the "hold off for a moment" in my last comment: from my side the PR is ready whenever you decide between #820 and #855. #820 has not been touched, and @preko-p is credited in the commit message and in the description either way. |
|
Thanks for the kind words and for pushing this forward, @AlexsJones. I'll defer to the maintainers on the #820 vs #855 decision—either way works for me, my goal was just to unblock this feature. Feel free to reach out if anything else is needed. |
Summary
Adds scheduling tolerations at both levels asked for in #767:
spec.tolerations([]corev1.Toleration) on theK8sGPTCRD, copied into the managed K8sGPTDeploymentpod template inGetDeployment.controllerManager.tolerationson the operator Helm chart, rendered into the controller-manager pod template, following the existingnodeSelectorpattern.Fixes #767.
This is @preko-p's change from #820, rebased onto
main. @AlexsJones reviewed #820 and found the change complete ("the whole checklist"); the only blockers left there were the stale branch and EasyCLA. Rather than writing a second implementation I carried #820 forward:chart/operator/crds/k8sgpt-crd.yamlis resolved,controller-gen v0.21.0(make manifests) instead of hand-edited, so the tolerations schema matches the rest of the generated file and the chart copy is byte-identical to the generated one,zz_generated.deepcopy.gois unchanged on regeneration (make generateis a no-op on top of it),chart/operator/README.mdis dropped.@preko-p is credited as the commit author. I asked on #820 first whether they plan to rebase it (comment); given the branch has been stale since June and they have not been active on GitHub since 2026-06-09, I went ahead per @AlexsJones's suggestion in #767 — and I'll close this PR immediately if @preko-p would rather finish it on #820.
Because this branch also lands a commit under my name, the EasyCLA check will need a signature from me; I'll sort that out as soon as a maintainer wants to take this forward.
Tests
Unit test in
pkg/resources:Helm rendering — with
controllerManager.tolerationsset, the toleration reaches the controller-manager pod template:With the default
tolerations: []the chart renders notolerationskey at all, the same{{- if }}guard behaviour asnodeSelector.Build, vet and manifest generation:
go test ./...is green except for the two envtest suites, which cannot start a control plane in my sandbox because the kubebuilder assets are not installed (they fail the same way on unmodifiedmain):Note on process: the rebase, the CRD regeneration and the verification above were done with an AI coding assistant, which is why I am being explicit about who wrote what.