pdist: run the post-update scripts in the client and run make-automaps
#575
+36
−14
Conversation
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
pmenzel
force-pushed
the
pdist-run-post-update-scripts
branch
from
579c705 to
946b43e
Compare
donald
reviewed
Aug 28, 2026
pdist/pdist
Outdated
| @@ -846,6 +847,38 @@ sub prog_update | |||
| } | |||
| } | |||
|
|
|||
| sub run_post_update_scripts | |||
Collaborator
There was a problem hiding this comment.
Please use new style for all new code, don't follow the obsolete style of the script.
- Indent with 4 spaces. No tabs.
- Put a space before and after binary operators (
=,==,+,.,
=>,&&,||,and,or, etc.). - Put a space after commas in argument lists and list literals.
- Put a space after
if,elsif,while,for,foreach,until
before the opening(. - No trailing whitespace at end of line.
- No blank line at end of file.
- Collapse runs of multiple consecutive blank lines to a single blank
line. Use a single blank line to separate subs and logical blocks. - Put a space after
#in comments (# comment, not#comment). - Braces follow K&R style: opening brace on the same line as the
keyword/sub, e.g.sub foo {,if (...) {.
| @@ -0,0 +1,17 @@ | |||
| #! /bin/bash | |||
Collaborator
There was a problem hiding this comment.
nit: I've just learned that is it more portable to avoid the space after the #!.
| # /etc/automount/auto.home, and that user cannot log in until somebody runs | ||
| # make-automaps by hand. | ||
| # | ||
| # https://github.molgen.mpg.de/mariux64/mxtools/issues/574 |
Collaborator
There was a problem hiding this comment.
Whole comment is not needed. For example, probably make-automaps.service is going away soon (it has no function, has it?) and then the comment will be out of date.
| # | ||
| # https://github.molgen.mpg.de/mariux64/mxtools/issues/574 | ||
|
|
||
| set -e |
Collaborator
There was a problem hiding this comment.
NIT: Not really needed for a single command script.
pdist/pdist
Outdated
| # | ||
| # the name must consist of digits, letters, '-', '.' and '_' only, so | ||
| # that editor backups like 0001-foo~ are not executed. A script which | ||
| # is not executable is an error and not silently skipped. |
Collaborator
There was a problem hiding this comment.
Really? -f $path would be true, system($path); would be $? == -1 and $! == 13, so that should die with "$path: Permission denied".
I think, the code is okay. The whole comment block is not needed, IMO. Takes too much vertical space.
Collaborator
|
Nice! |
Scripts to be run after an update live in a directory on the client and
are called by a wrapper, which the master has to name on every push:
ExecStart=… pdist push $host … --post-command /usr/libexec/post-pdist/post-pdist
$ cat /usr/libexec/post-pdist/post-pdist
set -e
for f in /usr/libexec/post-pdist/[0-9]*; do
"$f"
done
The client knows on its own when an update has finished, so let it walk
the directory itself. That removes the wrapper, the argument which has
to be threaded from the unit file through ssh to the client, and the
[0-9] glob, which only exists to keep the wrapper from calling itself.
A push without `--post-command` now runs the scripts as well, which is
what one wants for an ad-hoc `pdist push somehost`.
The scripts run before `--post-command`, which is kept for anything a
single push wants to add.
The first failing script stops the run, as `set -e` did in the wrapper,
so a script may rely on its predecessors having run. Unlike the wrapper,
the failure now also fails the update: the client dies, ssh exits
non-zero and `push_single()` picks that up. A post-update script which
fixes up the installed system is not something to fail silently.
Names are restricted to digits, letters, '-', '.' and '_' so that an
editor backup left in the directory is not executed – the old glob would
have run `0001-foo~`. A script which is not executable stays an error
and is not silently skipped.
Tested with a copy of the script pointed at a scratch directory:
case 1: directory missing → no output, exit 0
case 2: 0001, 0002, 0010 → run in that order, exit 0
README, 0001-first.orig~, subdirectory 0005-a-directory
→ ignored
case 3: 0002 exits 3
.../0002-second: exit status 3 → 0010 not run, exit 3
case 4: 0002 kills itself with SIGTERM
.../0002-second: killed by signal 15 → 0010 not run, exit 255
case 5: 0002 not executable
.../0002-second: Permission denied → 0010 not run, exit 13
case 6: --quiet → per-script line gone, failure still reported
The directory is empty until the scripts move there, so this commit on
its own is a no-op and the wrapper keeps doing the work.
Assisted-by: Claude Opus 5 (claude-opus-5)
The client walks `/usr/libexec/pdist/post-update.d` now, so move the
scripts there and drop the wrapper:
/usr/libexec/post-pdist/post-pdist gone
/usr/libexec/post-pdist/0001-… → /usr/libexec/pdist/post-update.d/0001-…
/usr/libexec/post-pdist/0002-… → /usr/libexec/pdist/post-update.d/0002-…
They sit next to the pdist client which runs them, under the name of the
program owning the directory rather than under the name of the wrapper
which used to. `.d` says a script can be dropped in without editing
anything else, which `install.sh` has done since commit 692fa93.
`/usr/libexec`, not `/etc`: these are executables shipped with the
distribution and not host configuration.
`pdistd@.service` no longer has to name the wrapper. The old directory
disappears from the clients on the next push, because pdist deletes
what the master does not have.
Installed into a `DESTDIR` to check the layout:
$ DESTDIR=/scratch/... ./install.sh
$ find $DESTDIR -path '*post-update*'
…/usr/libexec/pdist/post-update.d
…/usr/libexec/pdist/post-update.d/0001-convert-default-target
…/usr/libexec/pdist/post-update.d/0002-reload-systemd-260.2
$ ls -l .../usr/libexec/pdist/post-update.d/
-rwxr-xr-x 1 … 0001-convert-default-target
-rwxr-xr-x 1 … 0002-reload-systemd-260.2
Assisted-by: Claude Opus 5 (claude-opus-5)
A user created while a host is switched off cannot log in on that host
after it comes back, although `/etc/passwd` has the entry:
$ sudo journalctl -b -u make-automaps.service
make-automaps.service skipped, unmet condition check ConditionPathExists=!/etc/automount/auto.scratch
pdist distributes `/etc/amd`, but not the autofs maps generated from it:
`/etc/automount` is excluded in `/root/Distfile` and built on the client
by `make-automaps`. The only thing calling it on a running system is
`make-automaps.service`, and its
`ConditionPathExists=!/etc/automount/auto.scratch` holds exactly once,
on a host which has never built the maps. So the host receives the new
`/etc/passwd` entry with the next update, while
`/etc/automount/auto.home` keeps the state of its last boot as an
installed system, and the home directory of the new user cannot be
mounted.
`clusterd push --post make-automaps` covers the case where the host is
up when the user is created; the host which was down is picked up by
whatever update comes later, and that is the pdist update. So rebuild
the maps from there.
Verified that the client picks the script up, with make-automaps
replaced by a stub so nothing outside the scratch directory is touched:
…: executing …/post-update.d/0001-convert-default-target
…: executing …/post-update.d/0002-reload-systemd-260.2
…: executing …/post-update.d/0005-make-automaps
make-automaps stub: rebuilding maps
exit=0
and that a failing `make-automaps` fails the update:
…/post-update.d/0005-make-automaps: exit status 2
exit=2
Not tested against a real distmaster and a real client.
Resolves: #574
Assisted-by: Claude Opus 5 (claude-opus-5)
Both update paths call make-automaps right after asking the distmaster
for an update:
-- bash -c 'netcat $(distmaster) 237 ; make-automaps'
netcat $(distmaster) 237
make-automaps
That netcat reaches pdistd.socket on the distmaster, which pushes back
to this host, and the push runs 0005-make-automaps on the client now.
So the maps are already rebuilt when netcat returns.
Assisted-by: Claude Opus 5 (claude-opus-5)
pmenzel
force-pushed
the
pdist-run-post-update-scripts
branch
from
946b43e to
ff280e8
Compare
donald
approved these changes
Sep 1, 2026
Sign in
to join this conversation on GitHub.
2 participants
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.
Resolves: #574