This is an automated email from the ASF dual-hosted git repository.
pbacsko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/master by this push:
new 8e680afe [YUNIKORN-2637] finalizePods should ignore pods like
registerPods (#847)
8e680afe is described below
commit 8e680afee341c57e4b33a5432c4abd186946bb9e
Author: Wilfred Spiegelenburg <[email protected]>
AuthorDate: Fri Jun 7 15:24:20 2024 +0200
[YUNIKORN-2637] finalizePods should ignore pods like registerPods (#847)
If a pod was in a terminal state during registration it is skipped. The
same principal should apply to finalisePods:
* only check pods that were added in registerPods.
* remove finished pods in the finalizePods call if they were registered.
* new pods are not added in finalizePods
Closes: #847
Signed-off-by: Peter Bacsko <[email protected]>
---
pkg/cache/context.go | 20 +++++++++++----
pkg/cache/context_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 5 deletions(-)
diff --git a/pkg/cache/context.go b/pkg/cache/context.go
index 87bbbfeb..45e74017 100644
--- a/pkg/cache/context.go
+++ b/pkg/cache/context.go
@@ -1689,7 +1689,7 @@ func (ctx *Context) finalizeNodes(existingNodes
[]*v1.Node) error {
}
func (ctx *Context) registerPods() ([]*v1.Pod, error) {
- log.Log(log.ShimContext).Info("Starting node registration...")
+ log.Log(log.ShimContext).Info("Starting pod registration...")
// list all pods via the informer
pods, err :=
ctx.apiProvider.GetAPIs().PodInformer.Lister().List(labels.Everything())
@@ -1704,14 +1704,16 @@ func (ctx *Context) registerPods() ([]*v1.Pod, error) {
})
// add all pods to the context
- for _, pod := range pods {
- // skip terminated pods
+ for i, pod := range pods {
+ // skip terminated pods: we do not add or finalise them later
if utils.IsPodTerminated(pod) {
+ pods[i] = nil
continue
}
ctx.AddPod(pod)
}
+ log.Log(log.ShimContext).Info("Finished pod registration...")
return pods, nil
}
@@ -1726,13 +1728,21 @@ func (ctx *Context) finalizePods(existingPods
[]*v1.Pod) error {
// convert the pod list into a map
podMap := make(map[types.UID]*v1.Pod)
for _, pod := range pods {
+ // if the pod is terminated finalising should remove it if it
was running in register
+ if utils.IsPodTerminated(pod) {
+ continue
+ }
podMap[pod.UID] = pod
}
- // find any existing nodes that no longer exist
+ // find any existing pods that no longer exist
for _, pod := range existingPods {
+ // skip if the pod was already terminated during register
+ if pod == nil {
+ continue
+ }
if _, ok := podMap[pod.UID]; !ok {
- // node no longer exists, delete it
+ // pod no longer exists, delete it
log.Log(log.ShimContext).Info("Removing pod which went
away during initialization",
zap.String("namespace", pod.Namespace),
zap.String("name", pod.Name),
diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go
index 3a157fc1..67df1d47 100644
--- a/pkg/cache/context_test.go
+++ b/pkg/cache/context_test.go
@@ -2416,3 +2416,67 @@ func foreignPod(podName, memory, cpu string) *v1.Pod {
},
}
}
+
+func TestRegisterPods(t *testing.T) {
+ context := initContextForTest()
+
+ pods, err := context.registerPods()
+ assert.NilError(t, err, "register pods with empty setup should not
fail")
+ assert.Equal(t, len(pods), 0, "should have returned an empty pod list")
+
+ var api *client.MockedAPIProvider
+ switch v := context.apiProvider.(type) {
+ case *client.MockedAPIProvider:
+ api = v
+ default:
+ t.Fatalf("api type not recognized")
+ }
+ pod1 := newPodHelper("yunikorn-test-00001", namespace, "UID-00001",
"node-1", "yunikorn-test-00001", v1.PodRunning)
+ pod2 := newPodHelper("yunikorn-test-00002", namespace, "UID-00002",
"node-1", "yunikorn-test-00002", v1.PodRunning)
+ pod3 := newPodHelper("yunikorn-test-00003", namespace, "UID-00003",
"node-1", "yunikorn-test-00003", v1.PodSucceeded)
+ pod4 := newPodHelper("yunikorn-test-00004", namespace, "UID-00004",
"node-1", "yunikorn-test-00004", v1.PodRunning)
+
+ api.GetPodListerMock().AddPod(pod1)
+ api.GetPodListerMock().AddPod(pod2)
+ api.GetPodListerMock().AddPod(pod3)
+ api.GetPodListerMock().AddPod(pod4)
+
+ pods, err = context.registerPods()
+ assert.NilError(t, err, "register pods should not have failed")
+ assert.Assert(t, assertListerPods(pods, 3), "should have returned 3
running pods in the list")
+
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod1.UID)) !=
nil, "expected to find pod 1 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod2.UID)) !=
nil, "expected to find pod 2 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod3.UID)) ==
nil, "not expected to find pod 3 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod4.UID)) !=
nil, "expected to find pod 4 in cache")
+
+ // prep for finalising the pods
+ // new pod added (should be ignored)
+ pod5 := newPodHelper("yunikorn-test-00005", namespace, "UID-00005",
"node-1", "yunikorn-test-00005", v1.PodRunning)
+ api.GetPodListerMock().AddPod(pod5)
+ // update pod 1 is now marked as terminated (will be removed)
+ pod1.Status = v1.PodStatus{
+ Phase: v1.PodSucceeded,
+ }
+ api.GetPodListerMock().AddPod(pod1)
+ // remove pod 4 as if it was removed from K8s (will be removed)
+ api.GetPodListerMock().DeletePod(pod4)
+
+ err = context.finalizePods(pods)
+ assert.NilError(t, err, "finalize pods should not have failed")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod1.UID)) ==
nil, "not expected to find pod 1 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod2.UID)) !=
nil, "expected to find pod 2 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod3.UID)) ==
nil, "not expected to find pod 3 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod4.UID)) ==
nil, "not expected to find pod 4 in cache")
+ assert.Assert(t, context.schedulerCache.GetPod(string(pod5.UID)) ==
nil, "not expected to find pod 5 in cache")
+}
+
+func assertListerPods(pods []*v1.Pod, count int) bool {
+ counted := 0
+ for _, pod := range pods {
+ if pod != nil {
+ counted++
+ }
+ }
+ return count == counted
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
