Skip to content
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

Add start time to .last_run_stats #288

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

urosgruber
Copy link
Contributor

I was trying something today, and this is to store the started time inside .last_run_stats for future reference when listing pots and finding the uptime of each pod.

Not sure if this is useful or not, and on the other hand .last_run_stats file was actually meant for things like that

@urosgruber
Copy link
Contributor Author

Any reason why there is no update on this @pizzamig ?

@grembo
Copy link
Collaborator

grembo commented Jul 28, 2024

Hi @urosgruber, sorry, I managed to miss your PR. I'll look into it.


echo "{ \"ExitCode\": $_exit_code }" > "$_confdir/.last_run_stats"
echo "{ \"ExitCode\": $_exit_code, \"StartedAt\": $_epoch }" > "$_confdir/.last_run_stats"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, last-run-stats is only used programmatically by the nomad pot driver, which extracts the exit code of the container from this file in case the container already exited (this is important, as restarting the nomad pot driver means it needs to re-attach and could miss the exit code otherwise). Therefore it contains not more information. In general, it could contain more things (including various timestamps), but in case of non-persistent containers (which is why the feature was implemented in the first place), it will be written when the container exits. That's also why it's called .last_run_stats and not .current_run_stats.

I admit that this is a bit confusing, since in all cases the file contains the exit code of the configured pot.cmd - with persistent jails that's usually (but not always an necessarily) the exit code of running rc (so that's usually written after "booting" the jail), while for non-persistent jails, that's always when the container exited, which makes no sense.

Long story short, writing StartedAt into this file is not universally useful, as it's not always written while the jail is running, but after it started. So for non-persistent jails, it could be interesting to have some statistics in there about for long it ran (which would hard to write for persistent ones, as it's written after rc started).

I hope the above makes sense. So basically, to be more useful, the last_run_stats feature would first need to be refined, so it means the same for persistent and non-persistent jails. When I introduced it, I had mostly non-persistent jails in mind. I didn't realize that for persistent jails the file is written much earlier in their lifetime and therefore has a different meaning.

What you still could do without making things worse:

  1. Rename the variable _epoch to _started_at
  2. Add it to local variables at the top of the _js_start function (this needs to happen with all local variables)
  3. Move _started_at=$(date +%s) to line 654

This way it reflects the real "started at" time of the jail and would be correct in all cases (even though having it for non-persistent jails would be "after the fact" and for persistent jails, it only becomes available after the rc script ran).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, I'll amend as instructed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants