stage0,rkt: don't require the pod to be running to remove apps #3799
Conversation
The kubelet tries to remove apps from pods that are stopped and there's no reason to prevent the removal of apps from not running pods, actually it was considered in some parts of stage0.RmApp(). Let's not require pods to be running to remove apps. If the pod is running, we'll still stop the app units and so on. If not, we just remove the app.
| stderr.PrintE(fmt.Sprintf("unable to determine the pid for pod %q", p.UUID), err) | ||
| return 254 | ||
| var podPID int | ||
| if p.State() == pkgPod.Running { |
There was a problem hiding this comment.
I don't think you need this. If you are removing an app from a stopped pod, it means it previously reached both pid-file writing and supervisor-ready file writing.
There was a problem hiding this comment.
I'm using podPID == 0 to mean the pod is not running and hence we don't need to call the stop and rm entrypoints. I guess I was confused by the comment in
Line 65 in 5e83d91
There was a problem hiding this comment.
Which is actually partially true if we use ppid because we'll then try to find the pid by calling getChildPID which looks in /proc/PPID/task/PPID/children or over /proc. So it will return a -1.
However, when we use pid (KVM flavor), the file will be present and have a > 0 value.
There was a problem hiding this comment.
I could move this test to stage0.RmApp to decide whether to call the entrypoints or not.
There was a problem hiding this comment.
We have a p.isRunning() inside p.ContainerPid1(). I think I'd rather leave this like it is now.
There was a problem hiding this comment.
Ack, I'm fine with that, and thanks for the additional info.
|
Part of me wonders if removing an app from a stopped pod should be a no-op. However, I bet the kubelet will keep trying to remove the app over-and-over, so I suspect it's needed. |
|
Thanks! |
The kubelet tries to remove apps from pods that are stopped and there's
no reason to prevent the removal of apps from not running pods, actually
it was considered in some parts of stage0.RmApp().
Let's not require pods to be running to remove apps. If the pod is
running, we'll still stop the app units and so on. If not, we just
remove the app.