[TASK] DPL-204: runTests.sh follow-ups - #72
Merged
Conversation
"${USERSET}" passes "--user ${HOST_UID}" without a group, so a docker
container runs as "uid=${HOST_UID} gid=0(root)". A "--tmpfs" is created
"root:root" and inherits the mode of its host mountpoint, which is 0755
at a CI umask of 0022 - group 0 gets "r-x" only, and the functional
sqlite suite fails with "unable to open database file".
The docker adoption worked around this by mounting the sqlite tmpfs
"mode=1777". That is correct and umask independent, but it treats the
one mount rather than the missing group.
The mount options move into "TMPFS_MOUNT_OPTIONS", assigned next to the
container parameters they belong to, so the two container binaries can
state what they actually need: docker adds "uid" and "gid" and keeps
"mode=1777", rootless podman maps the container root to the host user
and needs neither, keeping "mode=1777" for the rootful case.
"${USERSET}" is deliberately left alone. It is evaluated for both
container binaries, and appending a group there would change which host
group rootless podman maps the container to, which is a different
question from who owns a tmpfs.
The comment moves along with the options and now names the umask that
makes this visible: at the 0002 of a typical workstation the mountpoint
comes up 0775 and the defect cannot be reproduced at all.
Two variables were referenced or built but never took effect, and both
misled anyone reading the script.
"CI_PARAMS" is expanded into the podman container parameters and into
the mock server container - four sites, and the mock server one is
unconditional, so it also applies under docker - but was never
assigned. It is not a leftover though: the harnesses this one is
modelled on assign it as "CI_PARAMS="${CI_PARAMS:-}"" and treat it as
an escape hatch a caller can export to inject additional container
flags. Only that assignment was missing here, so it is added rather
than the references removed.
"DOCUMENTATION_COMMON_PARAMS" was assigned for both container binaries
and then never used. "renderDocumentation" built its own line instead,
inlining "${CONTAINER_INTERACTIVE}" and repeating the bind mount, and
so ran without "--rm" and, under docker, as root rather than as the
host user - which is why rendering can leave root owned files behind
in "Documentation-GENERATED-temp".
It now uses the parameters that were built for it. They carry the same
"${CONTAINER_INTERACTIVE}" the line had, add the "--rm" and the
"${USERSET}" it was missing, and provide the "/project" mount, so the
mount is dropped from the line. "--pull always" and the image
arguments are unchanged. This also brings the call in line with
"main", where the same variable is wired up.
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 work left over from the CI docker adoption (WVP-106) for
branch
1. Everything here is pre-existing — none of it was caused bythat adoption.
Addresses DPL-204 (T3, T4). DPL-214 has no work on this branch —
see below. The companion pull request for
maincarries T1 and the 8label edits.
This branch touches
Build/Scripts/runTests.shonly. No workflow fileis changed at all.
Commits
[TASK] DPL-204: Own the sqlite tmpfs from the hostHOST_GID,TMPFS_MOUNT_OPTIONSper container binary, used on the sqlite--tmpfs[TASK] DPL-204: Revive two unused harness variablesCI_PARAMSassigned;renderDocumentationnow usesDOCUMENTATION_COMMON_PARAMSWhich tasks applied
The 4 mislabelled steps exist on this branch too (2 in
testcore12.yml, 2 intestcore13.yml), but the entiretestsuite:job is commented out in both files — job header, matrix, permissions
and every step. Parsing either file yields only the
code-qualityjob, so this branch runs zero functional steps. The defect is
dormant, not live. Un-commenting the block to fix it would re-arm a
whole test job, which is exactly what the "no job added, removed or
reordered" constraint rules out. Fix it there if the job is ever
revived.
-itinrenderDocumentation: does not apply, asthe issue already states. The only
-iton this branch is thelegitimate
CONTAINER_INTERACTIVE="-it --init", which is emptied whenCI=true. Left alone.--userwithout a gid: applied, byte-identical tomain.${USERSET}is deliberately untouched; the mount options moved intoTMPFS_MOUNT_OPTIONS, where docker addsuid/gidand podman doesnot need them. The existing
mode=1777is kept in both.CI_PARAMSwasreferenced but never assigned,
DOCUMENTATION_COMMON_PARAMSwas builtbut never read.
Label changes (DPL-214)
None. DPL-214 is scoped to
mainonly, and this branch has no livefunctional step to relabel. The dormant steps inside the commented-out
testsuitejob were left exactly as they are — editing commented lineswould only make a future revival look reviewed when it has not been.
The
renderDocumentationdifference frommainWorth a reviewer's eye, because this branch's line is not the one the
issue describes. On
main,renderDocumentationusedCONTAINER_COMMON_PARAMSand therefore picked up--network,--add-hostand a stray-w. Here it built its own line instead:so it ran without
--rmand, under docker, as root rather than as thehost user — which is how root-owned files end up in
Documentation-GENERATED-temp. Pointing it atDOCUMENTATION_COMMON_PARAMSkeeps the same${CONTAINER_INTERACTIVE}, adds the missing--rmand${USERSET},and supplies the
/projectmount, so the repeated-vis dropped fromthe line.
--pull alwaysand the image arguments are unchanged.publish.ymlrenders the documentation into a release asset on thisbranch, so this path is exercised.
Corrections to the issue text
renderDocumentationusesCONTAINER_COMMON_PARAMSand inherits
--network,--add-hostand a stray-w. True onmain, not true on this branch — see above. The dead-variableconclusion holds; the stated consequence does not.
CI_PARAMSis referenced in the podman branch. It isreferenced at 4 sites, and the mock-server run line is
unconditional, so the missing assignment also affected the docker path.
1are inside a commented-out block.Verification
Static plus CI. The functional suites were not run locally — this is one
of 24 branches in the sweep, and each pull request's own CI exercises
them. On this branch there is in any case no functional CI job to run.
Structural comparison of every workflow against
origin/1: all 3 filesidentical in structure and byte-identical —
git diff origin/1..HEAD -- .github/is empty, which is the strongest possible statement that nojob, step, matrix entry, trigger or commented line moved.
Substantive property checks, deliberately not grepping for strings that
also occur in the inserted comments:
grep -E '^\s*TMPFS_MOUNT_OPTIONS="'→ the two expected assignments…functional-sqlite-dbs/:${TMPFS_MOUNT_OPTIONS}, noliteral option list left on that line
grep -E '^CI_PARAMS="'→ 1,grep -E '^HOST_GID='→ 1renderDocumentationrun line uses${DOCUMENTATION_COMMON_PARAMS}runTests.sh(thewaitForcap of 60 with itscleanUp; exit 1abort, andhandleDbmsOptions) byte-identical toorigin/1Acceptance
maincompanion
Build/Scripts/runTests.shreordered; no commented block un-commented
[TAG] <ISSUE>: Subject≤ 52 characters, bodieswrapped at 72
-b dockerflags, the WVP-106 workflowheader comment, the
waitForcap of 60 and its abort,CONTAINER_INTERACTIVE="-it --init", and the sqlitemode=1777