Record the node a container is running on, not the container id - #322
Open
adrpo wants to merge 1 commit into
Open
Record the node a container is running on, not the container id#322adrpo wants to merge 1 commit into
adrpo wants to merge 1 commit into
Conversation
OpenModelica#320 stores socket.gethostname() as the machine that produced a library's results. That is right until the run is inside a container, where it is the container id: job_claim currently says the wasm-jit libraries were tested by "e9725d308091", which identifies no machine and is gone with the container. OpenModelica#295 wants every job in a Docker image, which would make that the answer everywhere and leave the new libversion.host column as useless as no column. Ask Jenkins instead. It names its agents and propagates NODE_NAME into a container it starts, so that is the machine name when there is a container in the way. LIBTEST_HOST overrides it, for the same reason LIBTEST_DB exists. NODE_NAME is only consulted inside a container - detected by /.dockerenv, or /run/.containerenv under podman. Outside one the kernel's answer is the right one and a NODE_NAME left in the environment must not override it, which is also what keeps this a no-op for the machines running the testing today. One helper in resultsdb, so the claim in job_claim and the host recorded with the results cannot disagree about which machine this is. Checked all four paths: plain host, NODE_NAME set but no container (unchanged, the real hostname wins), LIBTEST_HOST set, and inside a container with and without NODE_NAME. The container paths were exercised by faking the marker file rather than in a real container - Docker is not available where this was written. Follow-up to OpenModelica#320, part of OpenModelica#301, and needed before OpenModelica#295.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #320, which is merged.
#320 records
socket.gethostname()as the machine that produced a library's results. Thatis the machine right up until the run is inside a container, where it is the container id.
The database already shows the problem:
e9725d308091identifies no machine, and is gone with the container. Since #295 wantsevery job running from a Docker image, that would become the answer everywhere and leave
the new
libversion.hostcolumn as useless as having no column at all.Jenkins knows the machine — it names its agents — and propagates
NODE_NAMEinto acontainer it starts, which is something
socket.gethostname()inside that containercannot see. So:
NODE_NAMEis consulted only inside a container. Outside one the kernel's answer isthe right one and a
NODE_NAMEleft over in the environment must not override it — whichis also what makes this a no-op for the machines running the testing today.
LIBTEST_HOSToverrides both, for the same reason
LIBTEST_DBexists, and is the escape hatch if somejob turns out not to have
NODE_NAME.It lives in
resultsdbas one helper used by both callers, so the host injob_claimandthe host stored with the results cannot disagree about which machine this is. That also
fixes the wasm-jit claims above, not just the new columns.
Checked all four paths — plain host;
NODE_NAMEset but no container (unchanged, the realhostname wins);
LIBTEST_HOSTset; and inside a container with and withoutNODE_NAME.The container paths were exercised by faking the marker file rather than in a real
container, since Docker was not available where this was written; worth a real check on a
node before #295 leans on it.
Part of #301, and needed before #295's Docker task means anything for attribution.
generated by Claude Code