Skip to content

feat(k8s-reporter): pod scheduling controls to avoid blocking Karpenter consolidation#985

Merged
AlexKantor87 merged 1 commit into
mainfrom
chart/k8s-reporter-node-scheduling
Jul 1, 2026
Merged

feat(k8s-reporter): pod scheduling controls to avoid blocking Karpenter consolidation#985
AlexKantor87 merged 1 commit into
mainfrom
chart/k8s-reporter-node-scheduling

Conversation

@AlexKantor87

Copy link
Copy Markdown
Contributor

What

Adds pod scheduling controls to the k8s-reporter chart so the environment reporter can be kept off Karpenter-managed nodes, and fixes a podAnnotations rendering bug found along the way.

Closes #984.

Why

On EKS with Karpenter, the reporter CronJob (default every 5 minutes) can prevent node consolidation. The root cause is Karpenter's consolidateAfter churn timer: every reporter pod event resets the node's quiet-period, so a node whose consolidateAfter is longer than the reporter interval never becomes consolidatable (kubernetes-sigs/karpenter#1921, confirmed by the maintainers as intended behaviour). The chart previously had no way to pin the reporter to a stable node group.

Corrected during review: karpenter.sh/do-not-disrupt is not a fix (it prevents disruption, making consolidation less likely), and cluster-autoscaler.kubernetes.io/safe-to-evict is ignored by Karpenter. The correct mitigations are widening the schedule, pinning to a stable node group (this change), or running out-of-cluster. All three are documented in the new README section.

Changes

  • New values (all default to empty, non-breaking): nodeSelector, tolerations, affinity, podTemplateAnnotations, rendered into the CronJob pod template (spec.jobTemplate.spec.template.spec and .metadata.annotations).
  • Bugfix: podAnnotations was rendered inline as annotations: {{ toYaml ... }} on the CronJob metadata, which produces invalid YAML for any non-empty value (the chart was effectively unusable with a real podAnnotations). It is now rendered with the idiomatic {{- with }} ... {{- toYaml . | nindent }} and kept CronJob-scoped. Pod-level annotations use the new podTemplateAnnotations.
  • Docs: added a "Running on EKS with Karpenter" section to both the README (_templates.gotmpl) and Mintlify (_mintlify_templates.gotmpl) sources; regenerated README.md. The four new values are documented automatically in the values tables.
  • Chart version bumped 2.2.12.3.0 (minor, backwards-compatible).

Testing

  • helm lint (CI-equivalent, with required values) passes.
  • helm template verified with defaults (all new blocks omitted cleanly) and with values set (nodeSelector/affinity/tolerations land under the pod spec; podTemplateAnnotations on the pod, podAnnotations on the CronJob object).
  • README.md regenerated with helm-docs v1.14.2; Mintlify output rendered and checked.

Not included (follow-up)

A long-running Deployment mode (an alternative to the CronJob, which would also remove the per-run pod churn) needs a watch/loop mode in the CLI and is out of scope here. Worth a separate issue if there is demand.

🤖 Generated with Claude Code

…ns rendering

Add nodeSelector, tolerations, affinity and podTemplateAnnotations to the k8s-reporter chart, rendered into the CronJob pod template, so the reporter can be pinned to a stable node group away from Karpenter-managed nodes.

Also fix podAnnotations, which was rendered inline on the CronJob metadata and produced invalid YAML for any non-empty value. It is now rendered correctly and kept CronJob-scoped; pod-level annotations use the new podTemplateAnnotations.

Document 'Running on EKS with Karpenter' in the README and Mintlify docs (root cause is the consolidateAfter churn timer, ref kubernetes-sigs/karpenter#1921), regenerate README, and bump the chart to 2.3.0.

Refs #984

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @AlexKantor87's task in 1m 39s —— View job


PR Review: k8s-reporter pod scheduling controls

  • Read changed chart files (cronjob.yaml, values.yaml, templates)
  • Verify template rendering correctness (defaults + values set)
  • Check podAnnotations bugfix correctness
  • Verify generated README matches templates
  • Post review feedback

Nice, focused chart change. The templating is correct and idiomatic, the docs are thorough and technically accurate, and the version bump is right. Overall LGTM with one minor UX note.

Note: helm and helm-docs aren't available in this review sandbox (permission-restricted), so I verified rendering/indentation by inspection rather than running helm template/helm lint. No Go files changed, so the Go static-analysis phase (go build/go vet/go mod tidy) is not applicable.

What's correct

  • podAnnotations bugfix is real and correctly fixed. The old annotations: {{ toYaml .Values.podAnnotations }} emits the mapping starting on the same line as the key — invalid YAML for any non-empty value. The new {{- with }} + toYaml . | nindent 4 pattern is the idiomatic fix, and it correctly renders nothing when the value is empty ({}/[]), so defaults stay clean.
  • Indentation is right for all four new blocks: CronJob annotationsnindent 4; pod-template annotationsnindent 12 (key at 10 spaces); nodeSelector/affinity/tolerations under the pod specnindent 12 (keys at 10 spaces). All consistent with the surrounding template.
  • Non-breaking: all new values default to empty and are guarded by {{- with }}, so helm template with defaults omits every new block. podAnnotations behaviour is preserved (still CronJob-scoped).
  • README is correctly regenerated — the values table and new Karpenter section match the .gotmpl sources, and both README + Mintlify templates were updated symmetrically.
  • Good docs judgement: the callout that karpenter.sh/do-not-disrupt and safe-to-evict are not fixes (and why) is accurate and preempts the common wrong turns. Ordering the mitigations by preference is a nice touch.

Minor (non-blocking)

  • Naming asymmetry between podLabels (applied to the pod) and podAnnotations (applied to the CronJob object) — see the inline comment on values.yaml. The backwards-compat decision is reasonable; just worth making the doc string explicit so users don't expect podAnnotations to land on pods.

Optional follow-ups

  • Consider a lightweight chart-render regression guard (e.g. a helm template golden-output check in CI) so the exact class of bug fixed here — invalid YAML from inline toYaml — gets caught automatically. Not needed for this PR.
    · chart/k8s-reporter-node-scheduling

Comment on lines +136 to +139
# -- annotations to add to the CronJob object itself. For pod-level annotations (added to each reporter pod), use `podTemplateAnnotations` instead.
podAnnotations: {}

# -- annotations to add to the reporter pod template (applied to each Job pod that the CronJob creates)

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.

Minor naming asymmetry worth calling out for users: podLabels is applied to the pod template (cronjob.yaml L27–30), while podAnnotations is applied to the CronJob object and the new podTemplateAnnotations to the pod. So podLabels → pod but podAnnotations → CronJob, which is easy to trip over.

Keeping podAnnotations CronJob-scoped for backwards-compat is a reasonable call, but consider making the doc string explicit that it pairs with podLabels differently, e.g.:

Suggested change
# -- annotations to add to the CronJob object itself. For pod-level annotations (added to each reporter pod), use `podTemplateAnnotations` instead.
podAnnotations: {}
# -- annotations to add to the reporter pod template (applied to each Job pod that the CronJob creates)
# -- annotations to add to the CronJob object itself. NOTE: unlike `podLabels` (which apply to the pod), these apply to the CronJob resource. For pod-level annotations (added to each reporter pod), use `podTemplateAnnotations`.
podAnnotations: {}

@AlexKantor87 AlexKantor87 merged commit 006e880 into main Jul 1, 2026
20 checks passed
@AlexKantor87 AlexKantor87 deleted the chart/k8s-reporter-node-scheduling branch July 1, 2026 09:27
AlexKantor87 added a commit to kosli-dev/docs that referenced this pull request Jul 1, 2026
## What

Documents the Karpenter node-scheduling controls added to the
`k8s-reporter` chart in
[kosli-dev/cli#985](kosli-dev/cli#985) (chart
2.3.0), and fixes the reporting tutorial's Helm example.

## Changes

- **`helm/k8s_reporter.mdx`** — regenerated from chart 2.3.0. Adds a
"Running on EKS with Karpenter (or another node autoscaler)" section and
documents the new `nodeSelector`, `tolerations`, `affinity` and
`podTemplateAnnotations` values; clarifies that `podAnnotations` is
CronJob-scoped. This page is the Mintlify output of the chart's doc
templates, so it was regenerated rather than hand-edited.
- **`tutorials/report_k8s_envs.md`** — fix the Helm tab's
`tutorial-values.yaml`, which still used the pre-v2
`kosliEnvironmentName` / `namespaces` fields (removed in chart v2.0.0),
to the current `reporterConfig.environments` list format. Add a short
Karpenter callout linking to the reference.

## Why

On EKS with Karpenter the reporter CronJob can prevent node
consolidation, via the `consolidateAfter` churn timer
([kubernetes-sigs/karpenter#1921](kubernetes-sigs/karpenter#1921)).
The chart now exposes scheduling controls to avoid it; this documents
them and the correct mitigations (widen the schedule, pin to a stable
node group, or run out-of-cluster). It also corrects the tutorial
example, which no longer works with chart v2.x.

## Validation

- `mint broken-links` passes for the changed pages. (It surfaces one
unrelated, pre-existing broken link, `/getting_started/service-accounts`
in `working_with_controls.mdx`, which this PR does not touch.)

Refs kosli-dev/cli#984, kosli-dev/cli#985

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

k8s-reporter chart: expose nodeSelector/tolerations/affinity and pod-template annotations (Karpenter node consolidation)

2 participants