Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis pull request replaces Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the kube-janitor component with k8s-ttl-controller within the base-cluster chart, updating the associated licenses, trusted registries, and documentation. Feedback indicates that the new ttl-controller template should explicitly set the image tag from the global helm repositories configuration to ensure version consistency, adhering to the pattern established in the previous implementation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/image_licenses.yaml (1)
116-118: Nit: alphabetical ordering.The rest of the
ghcr.io/*entries are sorted alphabetically by path (aquasecurity,jimmidyson,jkroepke,k8up-io,kyverno,teutonet, …). The newtwin/k8s-ttl-controllerentry at line 116 is inserted betweenk8up-ioandkyverno, which breaks that order. Consider moving it belowghcr.io/teutonet/oci-images/solr-ckan.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/image_licenses.yaml around lines 116 - 118, The ghcr.io/twin/k8s-ttl-controller entry is out of alphabetical order; move the block starting with "ghcr.io/twin/k8s-ttl-controller:" so that it appears after "ghcr.io/teutonet/oci-images/solr-ckan" to restore alphabetical ordering of ghcr.io entries in .github/image_licenses.yaml, keeping the existing license and licenseLink values unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/base-cluster/README.md.gotmpl`:
- Around line 418-431: Update the migration note under "11.x.x -> 12.0.0" to
call out two behavioral differences: first, warn that kube-janitor's rule-based
cleanup (the rules: config loaded via --rules-file and JMESPath selectors) is
not supported by k8s-ttl-controller which is strictly annotation-driven, and
instruct operators to audit any cluster-wide `rules` usages because they will
stop running after replacing `kube-janitor` with `ttl-controller`; second,
clarify TTL/expiry value differences by documenting that `kube-janitor` accepted
relative durations (s/m/h/d/w), absolute timestamps via the `janitor/expires`
annotation and the special `forever` token, whereas `k8s-ttl-controller` only
accepts Go-style durations (e.g., 30m, 24h, 7d) for
`k8s-ttl-controller.twin.sh/ttl` (and that the previous statement "format
remains the same" only applies to overlapping duration formats), so users must
convert `janitor/expires` timestamps and `forever` values and re-audit
annotations when switching `janitor/ttl` → `k8s-ttl-controller.twin.sh/ttl`.
---
Nitpick comments:
In @.github/image_licenses.yaml:
- Around line 116-118: The ghcr.io/twin/k8s-ttl-controller entry is out of
alphabetical order; move the block starting with
"ghcr.io/twin/k8s-ttl-controller:" so that it appears after
"ghcr.io/teutonet/oci-images/solr-ckan" to restore alphabetical ordering of
ghcr.io entries in .github/image_licenses.yaml, keeping the existing license and
licenseLink values unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57256271-5865-43d7-bafa-68e52276e8e5
📒 Files selected for processing (8)
.github/image_licenses.yaml.github/trusted_registries.yamlcharts/base-cluster/README.md.gotmplcharts/base-cluster/ci/artifacthub-values.yamlcharts/base-cluster/templates/global/kube-janitor.yamlcharts/base-cluster/templates/global/ttl-controller.yamlcharts/base-cluster/values.schema.jsoncharts/base-cluster/values.yaml
💤 Files with no reviewable changes (1)
- charts/base-cluster/templates/global/kube-janitor.yaml
There was a problem hiding this comment.
Pull request overview
Migrates the base-cluster chart’s TTL cleanup component from the unmaintained kube-janitor deployment to the actively maintained k8s-ttl-controller, including values/schema updates and documentation for the breaking change.
Changes:
- Replace the
kube-janitorHelmRelease with a newttl-controllerHelmRelease sourced from TwiN’s Helm repo (k8s-ttl-controllerchart). - Rename values/config surface from
kube-janitortottl-controller(values.yaml, schema, CI example values). - Update migration docs and GitHub CI metadata (trusted registries + image licenses).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/base-cluster/values.yaml | Switches helm repo entry to TwiN and renames config block to ttl-controller. |
| charts/base-cluster/values.schema.json | Renames schema property from kube-janitor to ttl-controller. |
| charts/base-cluster/templates/global/ttl-controller.yaml | Adds Flux HelmRelease for k8s-ttl-controller. |
| charts/base-cluster/templates/global/kube-janitor.yaml | Removes Flux HelmRelease for kube-janitor. |
| charts/base-cluster/ci/artifacthub-values.yaml | Renames CI values to enable ttl-controller. |
| charts/base-cluster/README.md.gotmpl | Adds breaking-change migration guidance for ttl-controller. |
| .github/trusted_registries.yaml | Removes hjacobs/kube-janitor and adds twin/k8s-ttl-controller as trusted. |
| .github/image_licenses.yaml | Removes kube-janitor license entry and adds k8s-ttl-controller license entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwrau
force-pushed
the
chore/base-cluster/migrate-kube-janitor-ttl-controller
branch
from
cb5ef4e to
860697c
Compare
kube-janitor hasn't been updated in a long time, whereas k8s-ttl-controller seems to be quite active
cwrau
force-pushed
the
chore/base-cluster/migrate-kube-janitor-ttl-controller
branch
from
a4f1b2f to
5ac3187
Compare
cwrau
deleted the
chore/base-cluster/migrate-kube-janitor-ttl-controller
branch
kube-janitor hasn't been updated in a long time, whereas
k8s-ttl-controller seems to be quite active
Summary by CodeRabbit
New Features
Documentation
Chores