Repository navigation
feat: add ttl controller in janitor - #1206
Merged
Merged
Conversation
Signed-off-by: Ajay Mishra <ajmishra@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduce a configurable TTL cleanup system: new generic TTL package and reconciler, controller setup, janitor CLI/Helm wiring to enable/default TTLs, TTL deletion metrics, Helm annotations for default TTLs, and unit + envtest tests for TTL behavior. Changes
Sequence DiagramsequenceDiagram
participant Controller as Controller Runtime
participant Reconciler as TTL Reconciler
participant API as Kubernetes API
participant Metrics as Metrics Registry
Controller->>Reconciler: Reconcile(ctx, Request)
Reconciler->>API: Get CR (NamespacedName)
API-->>Reconciler: CR (may lack annotations)
Reconciler->>Reconciler: Process(ctx, obj, defaultTTL, clock)
alt Apply default TTL (missing & defaultTTL>0)
Reconciler->>API: Update CR annotations (write ttl, expiry)
API-->>Reconciler: Update OK
Reconciler-->>Controller: Result(requeue after ≤ 1h)
else Expired
Reconciler->>Metrics: IncTTLDeletion(kind)
Reconciler->>API: Delete CR
API-->>Reconciler: Deleted / NotFound tolerated
Reconciler-->>Controller: Result(no requeue)
else Preserve or Noop
Reconciler-->>Controller: Result(no requeue)
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
|
🌿 Fern Docs Preview: https://nvidia-preview-pull-request-1206.docs.buildwithfern.com/nvsentinel |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
janitor/pkg/ttl/ttl_test.go (1)
48-214: Collapse theProcessscenario coverage into a table-driven test.These cases all follow the same setup/call/assert pattern with only annotations, default TTL, clock movement, and expected
Resultchanging. A table-driven test will make future TTL edge cases much cheaper to add and review. As per coding guidelines, "Write table-driven tests when testing multiple scenarios in Go".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@janitor/pkg/ttl/ttl_test.go` around lines 48 - 214, Collapse the many similar TestProcess_* cases into a single table-driven test (e.g. TestProcess_TableDriven) that iterates over cases describing name, initialAnnotations, defaultTTL, clockAdvance (duration to advance the newClock/clk), and expected results (expected Action: ActionRequeue/ActionNoop/ActionExpired, expected Changed bool, expected RequeueAfter duration, expected annotation assertions for TTLAnnotation/ExpiryAnnotation/PreserveAnnotation). For each case call Process(ctx, rn, defaultTTL, clk) using newRebootNode and newClock (and use clk.advance where needed), then assert result fields and rn.Annotations per case; include a case for initializing nil annotation map and for maxRequeueInterval behavior using maxRequeueInterval, and cover invalid TTL/Expiry strings and zero TTL; keep existing helper names (Process, newRebootNode, newClock, clk.advance, TTLAnnotation, ExpiryAnnotation, PreserveAnnotation, maxRequeueInterval) so assertions and setup map directly to the original tests.janitor/main.go (1)
494-519: Consider collapsing repeated TTL setup code into a small helper.The three near-identical option/setup blocks are good candidates for a tiny helper to reduce duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@janitor/main.go` around lines 494 - 519, The three TTL setup blocks duplicate the same pattern; create a small helper like setupTTL[T any](mgr ctrl.Manager, name string, ttl time.Duration) that builds opts using ttl.WithDefaultTTL[T](ttl) and ttl.WithMetrics[T](janitormetrics.GlobalMetrics.IncTTLDeletion) and then calls ttl.Setup[T](mgr, name, opts...); replace the current calls for RebootNode, GPUReset and TerminateNode with succinct calls to setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode](mgr, "rebootnode-ttl", defaultTTL) etc., and propagate/return any error from the helper so existing error logging/formatting (slog.Error and fmt.Errorf) can remain unchanged where you call it.distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yaml (1)
62-62: Avoid duplicating the default TTL in template logic.
ttl.defaultTTLis already defined in chart values, so keepingdefault "336h"here creates two sources of truth.♻️ Suggested simplification
- - "--default-ttl={{ .Values.ttl.defaultTTL | default "336h" }}" + - "--default-ttl={{ .Values.ttl.defaultTTL }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yaml` at line 62, The template currently uses a fallback default inside the TTL interpolation ("--default-ttl={{ .Values.ttl.defaultTTL | default "336h" }}"), creating a second source of truth; remove the inline default so the flag directly uses .Values.ttl.defaultTTL (i.e., "--default-ttl={{ .Values.ttl.defaultTTL }}") and rely on the chart values file to define the default TTL instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@janitor/main.go`:
- Around line 340-344: The flag DurationVar allowing negative values should be
validated after flags are parsed: check rf.defaultTTL (the variable set by
flag.DurationVar for --default-ttl) and if it is negative (< 0) exit with a
clear error (fail fast) and message stating that --default-ttl must be >= 0; add
this guard early in main() (after flag.Parse) and use the same logging/exit
pattern used elsewhere in janitor/main.go to report the invalid value.
In `@janitor/pkg/ttl/reconciler_test.go`:
- Around line 44-50: The tests use newFakeClient (which wraps
fake.NewClientBuilder) and must be migrated to an envtest-based client that
talks to a real API server; remove or stop using newFakeClient and instead
initialize an envtest.Environment in test setup (e.g., TestMain or a
setupTestEnv helper), start it to obtain a rest.Config, build the scheme with
newScheme(), then create a real controller-runtime client via client.New(cfg,
client.Options{Scheme: scheme}) and use that client in the reconciler tests;
also update any test setup/teardown to Start/Stop the envtest environment and
remove fake-client-specific workarounds around ResourceVersion and subresource
behavior (adjust assertions that expect API-server semantics to use the
envtest-backed client).
In `@janitor/pkg/ttl/reconciler.go`:
- Around line 42-56: The option setters WithClock[T] and WithMetrics[T] can
overwrite safe defaults with nil and cause panics; update WithClock to ignore
nil inputs (do nothing if c == nil) and update WithMetrics to ignore nil
functions (do nothing if fn == nil) so Reconciler's r.clock and r.onDeleted
remain the initialized defaults in NewReconciler; touch the Option[T] closures
for WithClock and WithMetrics to early-return when their parameter is nil,
referencing the Reconciler type and fields r.clock and r.onDeleted to locate the
changes.
---
Nitpick comments:
In `@distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yaml`:
- Line 62: The template currently uses a fallback default inside the TTL
interpolation ("--default-ttl={{ .Values.ttl.defaultTTL | default "336h" }}"),
creating a second source of truth; remove the inline default so the flag
directly uses .Values.ttl.defaultTTL (i.e., "--default-ttl={{
.Values.ttl.defaultTTL }}") and rely on the chart values file to define the
default TTL instead.
In `@janitor/main.go`:
- Around line 494-519: The three TTL setup blocks duplicate the same pattern;
create a small helper like setupTTL[T any](mgr ctrl.Manager, name string, ttl
time.Duration) that builds opts using ttl.WithDefaultTTL[T](ttl) and
ttl.WithMetrics[T](janitormetrics.GlobalMetrics.IncTTLDeletion) and then calls
ttl.Setup[T](mgr, name, opts...); replace the current calls for RebootNode,
GPUReset and TerminateNode with succinct calls to
setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode](mgr, "rebootnode-ttl",
defaultTTL) etc., and propagate/return any error from the helper so existing
error logging/formatting (slog.Error and fmt.Errorf) can remain unchanged where
you call it.
In `@janitor/pkg/ttl/ttl_test.go`:
- Around line 48-214: Collapse the many similar TestProcess_* cases into a
single table-driven test (e.g. TestProcess_TableDriven) that iterates over cases
describing name, initialAnnotations, defaultTTL, clockAdvance (duration to
advance the newClock/clk), and expected results (expected Action:
ActionRequeue/ActionNoop/ActionExpired, expected Changed bool, expected
RequeueAfter duration, expected annotation assertions for
TTLAnnotation/ExpiryAnnotation/PreserveAnnotation). For each case call
Process(ctx, rn, defaultTTL, clk) using newRebootNode and newClock (and use
clk.advance where needed), then assert result fields and rn.Annotations per
case; include a case for initializing nil annotation map and for
maxRequeueInterval behavior using maxRequeueInterval, and cover invalid
TTL/Expiry strings and zero TTL; keep existing helper names (Process,
newRebootNode, newClock, clk.advance, TTLAnnotation, ExpiryAnnotation,
PreserveAnnotation, maxRequeueInterval) so assertions and setup map directly to
the original tests.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3eea23b7-1c16-45d0-abe1-b2b6ca4f9c12
📒 Files selected for processing (12)
distros/kubernetes/nvsentinel/charts/fault-remediation/values.yamldistros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yamldistros/kubernetes/nvsentinel/charts/janitor/values.yamldistros/kubernetes/nvsentinel/values-full.yamldistros/kubernetes/nvsentinel/values-tilt.yamljanitor/main.gojanitor/pkg/metrics/metrics.gojanitor/pkg/ttl/reconciler.gojanitor/pkg/ttl/reconciler_test.gojanitor/pkg/ttl/setup.gojanitor/pkg/ttl/ttl.gojanitor/pkg/ttl/ttl_test.go
Signed-off-by: Ajay Mishra <ajmishra@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
fault-remediation/pkg/config/config.go (1)
203-204: Avoid repeated sorting inside nested superseding-group checks.Line 203 re-sorts the same action-name set for each group check, adding avoidable
O(n log n)work per call. Consider passing precomputed sorted names fromValidateinto downstream validators.♻️ Refactor sketch
func (c *TomlConfig) Validate() error { ... actionNames := sortedActionNames(c.RemediationActions) ... for _, actionName := range actionNames { - if err := c.validateSupersedingGroups(actionName, c.RemediationActions[actionName]); err != nil { + if err := c.validateSupersedingGroups(actionName, c.RemediationActions[actionName], actionNames); err != nil { return err } } return nil } -func (c *TomlConfig) validateSupersedingGroups(actionName string, resource MaintenanceResource) error { +func (c *TomlConfig) validateSupersedingGroups( + actionName string, + resource MaintenanceResource, + actionNames []string, +) error { for _, group := range resource.SupersedingEquivalenceGroups { - if err := validateOneSupersedingGroup(actionName, group, resource, c.RemediationActions); err != nil { + if err := validateOneSupersedingGroup(actionName, group, resource, c.RemediationActions, actionNames); err != nil { return err } } return nil } func validateOneSupersedingGroup( actionName, group string, resource MaintenanceResource, remediationActions map[string]MaintenanceResource, + actionNames []string, ) error { - for _, name := range sortedActionNames(remediationActions) { + for _, name := range actionNames { maintenanceResource := remediationActions[name] ... } ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fault-remediation/pkg/config/config.go` around lines 203 - 204, The loop in Validate repeatedly calls sortedActionNames(remediationActions) causing redundant O(n log n) sorts for each superseding-group check; instead compute sorted := sortedActionNames(remediationActions) once in Validate and pass that slice into downstream validators/loops (referencing Validate, sortedActionNames, remediationActions, and the for ... range over sortedActionNames) so all subsequent checks iterate the precomputed sorted slice rather than re-sorting each time.janitor/pkg/ttl/ttl_test.go (1)
53-258: Align test names with the repository test naming format.A few names (
TestProcess,TestKindFromType) don’t fully match theTestFunctionName_Scenario_ExpectedBehaviorconvention. Renaming will improve consistency and discoverability.As per coding guidelines, "Name Go tests descriptively using format:
TestFunctionName_Scenario_ExpectedBehavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@janitor/pkg/ttl/ttl_test.go` around lines 53 - 258, Rename the tests to follow the repository convention (TestFunctionName_Scenario_ExpectedBehavior): change TestProcess to a descriptive name like TestProcess_TableDriven_Scenarios_HandleTTLAndExpiry and change TestKindFromType to TestKindFromType_CRDAndCoreTypes_ReturnsCorrectKind; update any references to these symbols (test function names) and run `go test` to ensure there are no remaining name collisions or references (also apply similar renames to the related helper/subtests if they rely on the old names).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@fault-remediation/pkg/config/config.go`:
- Around line 203-204: The loop in Validate repeatedly calls
sortedActionNames(remediationActions) causing redundant O(n log n) sorts for
each superseding-group check; instead compute sorted :=
sortedActionNames(remediationActions) once in Validate and pass that slice into
downstream validators/loops (referencing Validate, sortedActionNames,
remediationActions, and the for ... range over sortedActionNames) so all
subsequent checks iterate the precomputed sorted slice rather than re-sorting
each time.
In `@janitor/pkg/ttl/ttl_test.go`:
- Around line 53-258: Rename the tests to follow the repository convention
(TestFunctionName_Scenario_ExpectedBehavior): change TestProcess to a
descriptive name like TestProcess_TableDriven_Scenarios_HandleTTLAndExpiry and
change TestKindFromType to TestKindFromType_CRDAndCoreTypes_ReturnsCorrectKind;
update any references to these symbols (test function names) and run `go test`
to ensure there are no remaining name collisions or references (also apply
similar renames to the related helper/subtests if they rely on the old names).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56c41796-57bc-4d66-b780-f9d4926bf475
📒 Files selected for processing (6)
distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yamlfault-remediation/pkg/config/config.gojanitor/main.gojanitor/pkg/ttl/reconciler.gojanitor/pkg/ttl/reconciler_test.gojanitor/pkg/ttl/ttl_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yaml
- janitor/pkg/ttl/reconciler_test.go
Contributor
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
The "minor changes" commit (3697d32) unintentionally bundled a deterministic validation refactor of fault-remediation/pkg/config/config.go (sortedActionNames helper, split Validate into two passes) alongside the legitimate TTL review-fix changes. That refactor is unrelated to the janitor TTL work and belongs in its own PR. Revert fault-remediation/pkg/config/config.go to its pre-3697d32 state so this PR stays focused on ADR-037 (TTL-based cleanup of janitor CRs). Signed-off-by: Ajay Mishra <ajmishra@nvidia.com> Made-with: Cursor
Follow the project convention established in janitor/pkg/controller/suite_test.go and plugins/slinky-drainer/pkg/controller/drainrequest_controller_test.go: reconciler tests use envtest with the real janitor CRDs registered, not controller-runtime's fake client. - Replace newFakeClient helper with a TestMain-managed envtest.Environment pointing at distros/.../charts/janitor/crds for the CRDs. - Share a package-level k8sClient across all reconciler tests. - Switch tests from fake.NewClientBuilder().WithObjects(...) to k8sClient.Create(...) with per-test cleanup hooks to avoid cross-test leakage (CRs are cluster-scoped, so unique names via t.Name() are used). - Adjust the deletion-in-progress test to trigger deletion through the real API server (Create with finalizer + Delete) rather than setting DeletionTimestamp directly. - Discard controller-runtime logger during tests. Pure-unit tests that don't interact with the API server (TestProcess, TestReconciler_NilOptionsIgnored, TestKindFromType, TestNewZeroRef_*) keep their in-memory structure; envtest is transparently available to them via the shared setup. Signed-off-by: Ajay Mishra <ajmishra@nvidia.com> Made-with: Cursor
Contributor
Merging this branch will increase overall coverage
Coverage by fileChanged unit test files
|
TestJanitorNodeLocking's "different nodes should overlap" assertion used a strict time.Time.Before() comparison, which rejects the boundary case where the second RebootNode starts at the exact same RFC3339 second that the first finishes. On fast KWOK/kind runs this intermittently fails — not because the janitor serialized operations across nodes (the behavior under test), but because the two intervals happened to touch at a one-second boundary. Relax the assertion to !After on both bounds so touching intervals count as "not serialized". The strict Before is kept on the same-node pair, where exact-touch is still a legitimate "did not overlap" answer. Signed-off-by: Ajay Mishra <ajmishra@nvidia.com> Made-with: Cursor
1 task done
Signed-off-by: Ajay Mishra <ajmishra@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
janitor/main.go (1)
518-531: Optional: table-drive the per-kind TTL setup to reduce repetition.Current logic is correct; a small descriptor slice would make future CR-kind additions less error-prone.
♻️ Example refactor
func registerTTLReconcilers(mgr ctrl.Manager, enabled bool, defaultTTL time.Duration) error { if !enabled { slog.Info("TTL reconcilers disabled; maintenance CRs will not be auto-deleted") return nil } - if err := setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode]( - mgr, "rebootnode-ttl", "RebootNode", defaultTTL); err != nil { - return err - } - - if err := setupTTL[*janitordgxcnvidiacomv1alpha1.GPUReset]( - mgr, "gpureset-ttl", "GPUReset", defaultTTL); err != nil { - return err - } - - if err := setupTTL[*janitordgxcnvidiacomv1alpha1.TerminateNode]( - mgr, "terminatenode-ttl", "TerminateNode", defaultTTL); err != nil { - return err - } + if err := setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode](mgr, "rebootnode-ttl", "RebootNode", defaultTTL); err != nil { + return err + } + if err := setupTTL[*janitordgxcnvidiacomv1alpha1.GPUReset](mgr, "gpureset-ttl", "GPUReset", defaultTTL); err != nil { + return err + } + if err := setupTTL[*janitordgxcnvidiacomv1alpha1.TerminateNode](mgr, "terminatenode-ttl", "TerminateNode", defaultTTL); err != nil { + return err + } slog.Info("TTL reconcilers registered for RebootNode, GPUReset, TerminateNode", "default-ttl", defaultTTL) return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@janitor/main.go` around lines 518 - 531, Replace the three repetitive setupTTL calls with a single loop-driven construct: define a small slice of descriptors (each holding the display name string, the TTL key string, and the resource type identifier) for RebootNode, GPUReset, and TerminateNode and iterate it to call setupTTL for each entry; keep using mgr and defaultTTL and invoke setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode] / setupTTL[*janitordgxcnvidiacomv1alpha1.GPUReset] / setupTTL[*janitordgxcnvidiacomv1alpha1.TerminateNode] appropriately when constructing the slice entries so the existing setupTTL, mgr, and defaultTTL usages remain unchanged while eliminating repeated code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@janitor/main.go`:
- Around line 518-531: Replace the three repetitive setupTTL calls with a single
loop-driven construct: define a small slice of descriptors (each holding the
display name string, the TTL key string, and the resource type identifier) for
RebootNode, GPUReset, and TerminateNode and iterate it to call setupTTL for each
entry; keep using mgr and defaultTTL and invoke
setupTTL[*janitordgxcnvidiacomv1alpha1.RebootNode] /
setupTTL[*janitordgxcnvidiacomv1alpha1.GPUReset] /
setupTTL[*janitordgxcnvidiacomv1alpha1.TerminateNode] appropriately when
constructing the slice entries so the existing setupTTL, mgr, and defaultTTL
usages remain unchanged while eliminating repeated code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ef5e8046-843b-4461-822e-e4ea5e55ff83
📒 Files selected for processing (4)
distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yamldistros/kubernetes/nvsentinel/charts/janitor/values.yamldocs/designs/037-janitor-cr-ttl-cleanup.mdjanitor/main.go
✅ Files skipped from review due to trivial changes (2)
- distros/kubernetes/nvsentinel/charts/janitor/templates/deployment.yaml
- docs/designs/037-janitor-cr-ttl-cleanup.md
🚧 Files skipped from review as they are similar to previous changes (1)
- distros/kubernetes/nvsentinel/charts/janitor/values.yaml
Signed-off-by: Ajay Mishra <ajmishra@nvidia.com>
Contributor
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
lalitadithya
approved these changes
Apr 20, 2026
4 tasks done
Contributor
|
This pull request has been automatically locked since it has been closed for 90 days with no further activity. Please open a new pull request for related changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implementation based on: ADR-037: Janitor — TTL-Based Cleanup of Maintenance CRs
Testing
Cleanup logic test:
As expected the CR was deleted after ~30 seconds.
From the logs of janitor:
{"time":"2026-04-20T06:39:51.536921132Z","level":"INFO","msg":"Successfully created node lock lease and acquired lock","module":"janitor","version":"dev","maintenanceResource":"ttl-quick-test","nodeLockName":"nvsentinel-worker2"} {"time":"2026-04-20T06:39:51.63894852Z","level":"INFO","msg":"Sending reboot signal to node","module":"janitor","version":"dev","node":"nvsentinel-worker2"} {"time":"2026-04-20T06:39:56.839000591Z","level":"INFO","msg":"RebootNode status updated","module":"janitor","version":"dev","node":"nvsentinel-worker2"} {"time":"2026-04-20T06:40:21.000748196Z","level":"INFO","msg":"resource has expired","module":"janitor","version":"dev","name":"ttl-quick-test","expiry":"2026-04-20T06:40:21Z"} {"time":"2026-04-20T06:40:21.000761951Z","level":"INFO","msg":"deleting expired resource","module":"janitor","version":"dev","kind":"RebootNode","name":"ttl-quick-test","expiry":"2026-04-20T06:40:21Z"} {"time":"2026-04-20T06:40:21.0152205Z","level":"INFO","msg":"Validation for Janitor CR upon deletion","module":"janitor","version":"dev","logger":"janitor-webhook","type":"RebootNode","name":"ttl-quick-test"}Annotation addition on resource if not present:
Type of Change
Component(s) Affected
Testing
Checklist
Summary by CodeRabbit
New Features
Metrics
Tests
Documentation