Commonize filtering of Pods by Owner with all orphans in namespace #132615
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mimowo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/sig apps |
|
LGTM. |
|
/cc @wojtek-t |
|
/lgtm Thanks @mimowo |
|
LGTM label has been added. Git tree hash: 1c94a333d2b70694ed6187acec26169e0641cba1
|
| @@ -1112,6 +1113,29 @@ func AddPodControllerUIDIndexer(podInformer cache.SharedIndexInformer) error { | |||
| }) | |||
| } | |||
|
|
|||
| // FilterPodsByOwner gets the Pods managed by an owner or orphan Pods in the owner's namespace | |||
| func FilterPodsByOwner(podIndexer cache.Indexer, owner *metav1.ObjectMeta) ([]*v1.Pod, error) { | |||
There was a problem hiding this comment.
Can we take this refactoring as an opportunity to introduce a simple unit test?
There was a problem hiding this comment.
I believe we already have coverage at the integration test level, but indeed the unit test would be great.
Let me explore adding one as part of the PR.
There was a problem hiding this comment.
Added in the last commit e8e178e, ptal
|
/triage accepted |
|
New changes are detected. LGTM label has been removed. |
|
/test pull-kubernetes-unit |
7ead42e to
e8e178e
Compare
e8e178e to
6d5e0bf
Compare
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
DRY principle, as part of #130767
Which issue(s) this PR is related to:
Special notes for your reviewer:
The duplication was observed when solving subtle issues in previous PR
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: