Conversation
There was a problem hiding this comment.
Pull request overview
Extends the deployment-tracker controller to recognize and attribute pods to additional Kubernetes workload types beyond Deployments, including DaemonSets, StatefulSets, Jobs, and CronJobs (via Job ownership resolution).
Changes:
- Add workload resolution logic (Deployment/DaemonSet/StatefulSet/Job/CronJob) and start/sync the needed informers/listers.
- Enhance pod event handling to also catch very short-lived Job pods that reach terminal phase before Running is observed.
- Update RBAC manifests/charts and expand unit/integration test coverage for the new workload types.
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 |
|---|---|
| internal/controller/controller.go | Adds informers/listers for new workload types, resolves a “top-level workload” name for pod attribution, and adds Job terminal-phase handling. |
| internal/controller/controller_test.go | Updates recordContainer callsites and adds unit tests for new workload helper functions. |
| internal/controller/controller_integration_test.go | Adds integration coverage for Job/CronJob/DaemonSet/StatefulSet lifecycles and syncs added informers. |
| internal/metadata/metadata.go | Extends owner-metadata lookup to DaemonSet/StatefulSet/Job/CronJob resources. |
| internal/metadata/metadata_test.go | Adds aggregation tests for Job→CronJob ownership chain and DS/SS owners; registers new GVKs. |
| deploy/manifest.yaml | Expands ClusterRole permissions to cover daemonsets/statefulsets/jobs/cronjobs. |
| deploy/charts/deployment-tracker/templates/clusterrole.yaml | Mirrors RBAC expansion in Helm chart. |
| README.md | Documents that {{deploymentName}} now refers to the owning workload and updates required permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extend the controller to track pods owned by all standard Kubernetes workload types, not just Deployments. Includes short-lived Job detection via terminal phase handling, CronJob name resolution through Job ownership chain, and RBAC/docs updates.
ef2371b to
a596e6f
Compare
ajbeattie
left a comment
There was a problem hiding this comment.
**Note: ** the bulk of the new logic is Job/CronJob-specific, since those need some special handling.
I included DaemonSet and StatefulSet since they have very basic requirements. Since they also come with specific test cases, the size of the PR is inflated, but excluding those wouldn't actually reduce the complexity of the changes meaningfully.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Extends the controller to track pods from all standard Kubernetes workload types, not just Deployments.
New workload support
Short-lived Job handling
Job-owned pods that complete before the controller observes them in Running phase (e.g. sub-second tasks) are now caught via terminal phase (Succeeded/Failed) detection in the AddFunc and UpdateFunc handlers. Scoped to Job-owned pods only to avoid extra event volume for long-lived workloads. Dedup cache prevents duplicate API calls when a pod passes through both Running and terminal phases.
Other changes
apps/daemonsets,apps/statefulsets,batch/jobs,batch/cronjobs(get/list/watch)processEventresolvesworkloadRefexactly once (was called twice on delete path)deleteJobhelper usespropagationPolicy=BackgroundKnown limitations