[
https://issues.apache.org/jira/browse/YUNIKORN-2637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17848435#comment-17848435
]
Wilfred Spiegelenburg commented on YUNIKORN-2637:
-------------------------------------------------
The list of pods we iterate over in {{finalizePods()}} are the *original* pods
that were processed and returned by the {{registerPods()}} call.
In {{registerPods()}} we skip pods in a terminal state. Those pods do not get
added to the context. {{AddPod()}} is never called for them.
In {{finalizePods()}} we list the pods again and build a map. We then iterate
over all pods returned from {{registerPods()}} and see if they are in the in
new list we just pulled from K8s. If we have a pod from the registered list
that does not show up in the newly pulled list we remove the pod from the
context.
That last step, removing from the context only makes sense if the pod was added
to the context to start with. Pods that were in a terminal state during the
{{registerPods()}} processing are not added and thus do not need to be removed
as they cannot exist in the context. It does not matter what state they are in
in the newly pulled list. A pod in a terminal state cannot return to a running
state ever.
{quote}I don't think this is safe. The pod may have moved into a terminal state
between registerPods() and finalizePods(). In that case, we may lose the
transition and end up with a phantom pod still in the system.
{quote}
This is not the case that needs to be optimised as this does not happen at all
with the current code. That is a bug in the code by itself I did not even
notice before.
The newly pulled list of pods is not status checked. A pod that was running,
during {{{}registerPods(){}}}, and now shows as terminated, in
{{{}finalizePods(){}}}, shows up in the both the map as well as the iteration
and will thus not be removed. Just the existence check in the register list
compared to the finalise list is not enough.
For that case to work we either need:
# filtering of the pods that we put in the finalised map (i.e. skip terminated
pods)
or
# a comparison of the state of the pods during the iteration
Option 1 is simplest as it just gives us a map of still running pods and
anything that does not exist should be removed.
I'll put up a PR that fixes both issues.
> finalizePods should ignore pods like registerPods does
> ------------------------------------------------------
>
> Key: YUNIKORN-2637
> URL: https://issues.apache.org/jira/browse/YUNIKORN-2637
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: shim - kubernetes
> Reporter: Wilfred Spiegelenburg
> Priority: Major
>
> The initialisation code is a two step process for pods: first list all pods
> and add them to the system in registerPods(). This returns a list of pods
> processed.
> The second step happens after event handlers are turned on and nodes have
> been cleaned up etc. During the second step pods from the first step are
> checked and removed. However pods that were already in a terminated state in
> step 1 get removed again. Although the step should be idempotent this is
> unneeded. When iterating over the existing pods any pod in a terminal state
> should be skipped.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org
