Delete a Workload if the related object is absent - #779
Conversation
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
WalkthroughThe changes add a new controller, Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant Manager
participant WorkloadReconciler
participant Workload
Main->>Manager: Initialize controller
Manager->>WorkloadReconciler: SetupWithManager()
WorkloadReconciler-->>Manager: Registered
Note over Manager: On workload event
Manager->>WorkloadReconciler: Invoke Reconcile(request)
WorkloadReconciler->>Workload: Fetch Workload Object
alt Workload Found
Workload-->>WorkloadReconciler: Return object details
WorkloadReconciler->>Manager: Return success result
else Workload Not Found / Error
Workload-->>WorkloadReconciler: NotFound/Error
WorkloadReconciler->>Manager: Log error / Delete workload
end
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/controller/workload_controller.go (2)
62-68: Update comment to reflect “Workload” rather than “WorkloadMonitor.”The controller registers watches for
Workloadobjects, but the comment still referencesWorkloadMonitor. Adjusting the comment ensures consistency.- // Watch WorkloadMonitor objects + // Watch Workload objects
70-87: Consider a more robust approach for identifying monitored object types.Relying on name prefixes works but can be brittle if naming conflicts arise. As an optional enhancement, storing a resource type in
Workloadspec or using labels/annotations could improve clarity and avoid collisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/cozystack-controller/main.go(1 hunks)internal/controller/workload_controller.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
internal/controller/workload_controller.go (2)
pkg/apiserver/apiserver.go (1)
Scheme(41-41)api/v1alpha1/workload_types.go (1)
Workload(52-57)
cmd/cozystack-controller/main.go (2)
internal/controller/workload_controller.go (1)
WorkloadReconciler(19-22)pkg/apiserver/apiserver.go (1)
Scheme(41-41)
🔇 Additional comments (3)
cmd/cozystack-controller/main.go (1)
181-188: Ensure proper test coverage and confirm alignment with existing reconcilers.This code block correctly initializes the new
WorkloadReconcilerand logs any setup failure before exiting. Please confirm that:
- Required unit or integration tests exist for this new controller setup.
- Logging categories and error messages remain consistent with your existing
WorkloadMonitorReconcilerlogic.internal/controller/workload_controller.go (2)
18-22: No immediate issues; consider verifying concurrency tolerances.Defining the
WorkloadReconcilerstruct embeddingclient.Clientwith aSchemeis a standard controller-runtime approach. Ensure any concurrent updates to shared fields (if introduced later) are handled properly.
24-60: Verify deletion logic to avoid accidental resource race conditions.This reconciler removes the
Workloadif the dependent object (PVC, Service, or Pod) does not exist. While it meets the PR objective, confirm you won’t accidentally delete theWorkloadif the monitored object is briefly unavailable or still being created. You may want to:
- Re-check the object after a short delay, or
- Add a finalizer approach for more controlled cleanup.
Summary by CodeRabbit