-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post Stop tasks are never killed on the client node (also at GC time) resulting in task events loss. #17971
Comments
Is it ok if we ignore the
However if I use your jobspec so that the poststop task, and therefore the whole alloc, is terminal ("dead" and "complete" respectively), I can reproduce your corrupted task events state. So there's definitely a bug here, I'm just not sure it's related to killing poststop tasks. For anyone lazy like me if you create a file
You can use that with |
Thank you for taking a look!
I agree that this isn't the core of the problem and I think it is quite OK for us to ignore it as a stand-alone problem. The primary cause for me mentioning the killing is what I believe to be the root of the issue within the implementation -- I hope that the description and PR make it somewhat more clear, but please do let me know if you think this is wrong or otherwise confusing. Philosophically speaking, unbounded run-times of post-stop tasks are somewhat bothersome in that the task spec itself may specify values which imply that they are meant to be stopped (for instance, kill_timeout), whereas in reality they will never be stopped without a "special" event (such as an OOM kill). This may of course be circumvented by using
The primary reason for this assertion is |
This is a great feature request and similar to this issue despite being for batch jobs: #1782. I think making timeouts at the task level makes the most sense precisely for things like post-stop tasks. Another way to frame this is that |
I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. |
Nomad version
Tip.
Operating system and Environment details
Linux.
Issue
When the client Nomad node garbage collects an allocation, any post-stop lifecycle tasks:
While one may argue that #1 is intended, #2 is most likely not and seems to be a bug.
Reproduction steps
First, start up a single server and a single client. It is important that the client has a low
max_alloc_gc
so that we can easily evoke the garbage collection.Then start up a simple job that has a post start job configured.
If we now let this run, we can observe the correct output of the job state. In particular, we will notice that the task events are populated and give us plenty of information:
Now, let's start anything else. It doesn't really matter what happens to this second job -- all we care about is that the garbage collection of the allocation on the client occurs and this is guaranteed by
max_alloc_gc = 1
setting on the client. Once the new job is ran, we will see that the exact same query to the above endpoint will now lack metadata:Or in json form:
Which contains illegal, uninitialized values.
Expected Result
The post stop task is stopped and the metadata is preserved.
Actual Result
It is not stopped and the metadata is not preserved.
Rootcause
The client GC logic is quite simple. One looks through allocs that may be GCed, pick one and then run the Destroy logic on it:
https://github.com/hashicorp/nomad/blob/v1.5.5/client/gc.go#L183
This destroy logic is also simple:
https://github.com/hashicorp/nomad/blob/v1.5.5/client/allocrunner/alloc_runner.go#L661-L736
This function returns task states with the added kill states. Except, since poststop, prestart and poststart tasks never got proper treatment these states are missing entirely and the task events get overwritten which causes them to disappear. I think the fix for this would be to either
ts.IsPoststopTask
so that they are killed alongsidesidecar
tasks which is what the comments claim was the intended behavior. (https://github.com/hashicorp/nomad/blob/v1.5.5/client/allocrunner/alloc_runner.go#L714)I believe that the first treatment would be less invasive and less "different" from what is happening today. However, the second treatment seems to be the more correct one since right now these tasks simply do not get killed, but this assumes that we need to split the regular stop behavior from that which occurs at GC time
The text was updated successfully, but these errors were encountered: