Skip to content

feat: add macOS (darwin) support to the worker agent installer - #1012

Open
andychoquette wants to merge 30 commits into
aws-deadline:mainlinefrom
andychoquette:macos-installer
Open

feat: add macOS (darwin) support to the worker agent installer#1012
andychoquette wants to merge 30 commits into
aws-deadline:mainlinefrom
andychoquette:macos-installer

Conversation

@andychoquette

@andychoquette andychoquette commented Jul 16, 2026

Copy link
Copy Markdown

What was the problem/requirement? (What/Why)

install-deadline-worker rejected every platform except Linux and Windows, so
macOS hosts could not be configured as workers in a customer-managed fleet —
even though the CMF fleet OS enum already accepts MACOS and the worker-agent
runtime otherwise runs on macOS. Customers want to bring Mac render hosts into their fleets.

What was the solution? (How)

Add darwin as a first-class platform in the installer, mirroring the existing
Linux structure:

  • installer/__init__.py

    • Allow darwin through the platform gate.
    • Add a darwin entry to INSTALLER_PATH so install() dispatches through
      the existing sudo <script> ... path to a new install_darwin.sh.
    • Reject --vfs-install-path on macOS (the Deadline VFS is Linux-only),
      surfacing the error before shelling out.
  • installer/install_darwin.sh (new) — the macOS port of install.sh:

    • Creates the agent user and job group via Directory Services
      (dscl / dseditgroup) instead of useradd/groupadd, preserving the
      same jobRunAsUser isolation model (agent user's primary group is never the
      job group; job group is a secondary membership only).
    • Provisions the same directories with the same ownership/permissions
      (credentials 700, worker.toml 640, etc.), writes worker.toml via the
      same deadline_worker_agent.config module, and installs a launchd
      LaunchDaemon
      instead of a systemd unit.
    • Does not write a jobRunAsUser sudoers rule. An earlier revision of this
      PR did; it was removed on review feedback so that macOS matches the Linux
      installer, where granting the agent passwordless sudo to the queue's job user
      is the operator's documented manual step. See
      jobRunAsUser sudo below.
    • Two macOS-specific deviations from a naive port:
      1. Argument parsing uses a portable while/case loop rather than
        getopt --longoptions, because macOS ships BSD getopt (which silently
        drops long options rather than erroring).
      2. The default session root is /var/lib/deadline/sessions, not
        /sessions, because the macOS root volume is sealed read-only
        (macOS 10.15+) and a top-level directory cannot be created there.
  • config/settings.py — add DEFAULT_MACOS_SESSION_ROOT_DIR
    (/var/lib/deadline/sessions) and select it on darwin in the settings model
    and the installer arg-parser default, so a stock install-deadline-worker
    (no --session-root-dir) works out of the box on macOS.

jobRunAsUser sudo: left to the operator, matching Linux

openjd-sessions runs a Session's actions as the queue's jobRunAsUser via
sudo -u <job-user> -i ..., which needs the agent user to become a job user
without a password. This installer does not configure that, and neither does
install.sh — on Linux it is a documented manual step in the
developer guide,
where the operator writes:

deadline-worker-agent ALL=(jobRunAsUser) NOPASSWD:ALL

macOS now behaves the same way. An earlier revision of this PR wrote a
group-scoped rule (deadline-worker ALL=(%deadline-job-users) NOPASSWD: ALL)
automatically, on the reasoning that the failure is silent on macOS — a
LaunchDaemon has no TTY, so every impersonated action dies with sudo: a terminal is required to read the password and the task loops READY ↔ ASSIGNED with
nothing on the host explaining why. That rule has been removed following
review feedback: the right move is to match the Linux install process first, and
treat automating this (with or without an installer flag) as a separate
quality-of-life change that lands for both platforms together rather than
diverging them here.

Consequences worth stating plainly:

  • A stock macOS install cannot run jobs on a queue using
    runAs = QUEUE_CONFIGURED_USER
    until the operator adds the sudoers rule by
    hand. Same as Linux, but the macOS failure mode is quieter, so this needs to be
    called out in the macOS worker-host documentation (tracked as the docs
    follow-up below).
  • Queues using runAs = WORKER_AGENT_USER are unaffected and work out of the
    box: the agent passes no user to openjd-sessions,
    PosixSessionUser.is_process_user() short-circuits the sudo branch, and
    actions run as the agent user directly.
  • Removing the rule also removes the privilege question it raised. There is no
    longer a stock install that grants the agent user authority over every current
    and future member of the job group, so the WORKER_AGENT_USER concern raised
    earlier in this review no longer applies to this PR.

The installer still writes the --allow-shutdown rule
(/etc/sudoers.d/deadline-worker-shutdown, 440 root:wheel, granting exactly
/sbin/shutdown -h now as root) — that one has a direct Linux counterpart and is
flag-gated. It is installed through a install_sudoers_file helper that writes to
a temp path, runs visudo -cf, and only then moves the file into place, so a file
sudo cannot parse never appears in /etc/sudoers.d (where it would break sudo
host-wide). The Linux installer validates nothing here, so this is a small
improvement on the baseline.

What is the impact of this change?

macOS hosts can be installed as customer-managed-fleet workers. Linux and
Windows behavior is unchanged (the Linux install.sh and Windows installer
paths are untouched; only a new darwin branch and a new script are added).

Dependency note: running a job as a jobRunAsUser on macOS also requires the
companion openjd-sessions changes (macOS lacks setsid(1), which that PR replaces
with a pure-Python session-leader shim); see
OpenJobDescription/openjd-sessions-for-python#335, now merged.
This PR covers host installation; that PR covers cross-user job execution, and neither
is useful without the other — without the shim the macOS cross-user spawn fails at
setsid regardless of how sudo is configured on the host.

That shim ships in openjd-sessions 0.10.14 (published), so this PR bumps the
pin from == 0.10.13 — which predates the merge and resolves to a build without the
shim — to == 0.10.14. Verified against the published sdist that it contains
_MACOS_SETSID_SHIM, is_macos(), and the pgrep retry fix.

Prerequisite: macOS worker hosts require the Xcode Command Line Tools (for
the OS-provided /usr/bin/python3 used by the openjd-sessions cross-user path).

Not covered here: neither this installer nor the Linux one creates the
jobRunAsUser accounts, enforces their job-group membership, or grants the agent
sudo to them. All three are operator steps. On macOS each one fails quietly if
missed (no TTY for sudo to prompt on), which is why the user-guide follow-up below
matters — there is currently no macOS worker-host setup page in the public docs at
all.

Pre-existing issues surfaced while reviewing this (not introduced here, not
fixed here)
— filing separately unless reviewers want them in scope:

  • --scripts-path is only checked for existence, never ownership, yet the
    LaunchDaemon execs ${scripts_path}/deadline-worker-agent at every boot. A
    group-writable venv (common on macOS, e.g. /opt/homebrew/bin is
    drwxrwxr-x root:admin) means a local admin user can replace the binary and
    run code as the agent user on next boot. Same gap in install.sh.
  • worker.toml.example is copied from $SCRIPT_DIR, so the same writable-path
    precondition allows config poisoning on a fresh install. Same in install.sh.
  • queue_boto3_session.py:367-372 chmods each queue's cached credentials 0640
    and chowns them to the queue user's group. If two queues resolve to the same
    posix group, one queue's job user can read the other's AWS credentials. Purely
    inherited (identical code path and directory modes on Linux), but it is the
    highest-impact credential exposure in this area and the installer's default
    group naming makes the misconfiguration easy to reach.

How was this change tested?

  • Added unit tests (test/unit/install/test_install.py): INSTALLER_PATH has a
    darwin entry, install() builds the expected sudo command on darwin, and
    --vfs-install-path is rejected before dispatch. Removed darwin from
    test_unsupported_platform_raises (it is now supported). Updated the
    session_root_dir settings-field test to expect the platform-correct default.
  • Full test/unit install + config suites pass (hatch run test): 726 passed.
  • Added macOS installer integration tests (test/integ/macos/test_installer.py)
    that run the installer for real as root and assert the invariants it must
    establish: hidden service account, the primary-group/job-group isolation
    invariant, file modes, LaunchDaemon plist contents, and the sudoers rules.
    They mutate host state, so they are gated behind RUN_INSTALLER_TESTS=true
    and run on throwaway macOS runners via the new macos_installer_test.yml
    workflow. No AWS access needed — the farm/fleet ids are fakes.
    • Sudoers coverage specifically: the jobRunAsUser rule exists with mode 440 root, parses under visudo -cf, and contains exactly the one
      group-scoped rule (so an added rule or a widened runas list fails the
      test); the shutdown rule is asserted separately and is revoked on a re-run
      without --allow-shutdown.
  • Validated end-to-end on macOS 26.5 (arm64) against a live customer-managed
    fleet: install-deadline-worker → the worker registers and reaches IDLE → a
    job runs as the queue's jobRunAsUser (uid 498, gid 501) → the job user
    cannot read the agent's cached credentials → the job is cancelled and its
    process group is reaped with no orphaned processes. Installer re-runs are
    idempotent.

Was this change documented?

Code comments in install_darwin.sh explain the macOS-specific choices (BSD
getopt, read-only root volume, Directory Services user/group creation,
launchd mappings) and flag items that warrant host-level verification (TCC /
Full Disk Access, code-signing / notarization for a headless daemon, the exact
shutdown argv for --allow-shutdown). The sudoers block documents which queue
configurations the rule affects, why Linux needs an equivalent rule as a manual
step, and that job group membership is the security boundary.

Public user-guide documentation (AWSBeaLineDocs) is a separate follow-up. It is
load-bearing here: the fleet-types table already lists macOS for CMF, but there
is no macOS worker-host setup page, and the worker agent README still states
that install-deadline-worker "does not support MacOS at this time" — that line
needs updating alongside this change.

Is this a breaking change?

No. It adds a new supported platform; Linux and Windows install behavior is
unchanged and no public interface is modified.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@andychoquette
andychoquette requested a review from a team as a code owner July 16, 2026 23:25
@github-actions github-actions Bot added the waiting-on-maintainers Waiting on the maintainers to review. label Jul 16, 2026
@andychoquette
andychoquette force-pushed the macos-installer branch 3 times, most recently from 4bc3cd0 to 6fc0a39 Compare July 16, 2026 23:35
@andychoquette
andychoquette marked this pull request as draft July 16, 2026 23:36
Comment thread src/deadline_worker_agent/installer/install_macos.sh
@andychoquette
andychoquette marked this pull request as ready for review July 16, 2026 23:57
Comment thread src/deadline_worker_agent/installer/install_darwin.sh
crowecawcaw
crowecawcaw previously approved these changes Jul 17, 2026
@crowecawcaw

Copy link
Copy Markdown
Contributor

LGTM. I think we should open an issue to convert the linux and mac install scripts to Python so code is cleaner, we're not building structures strings in bash, and so we can test them better.

Also do we have an E2E test we'll run? Is it possible to run one in GitHub actions?

@andychoquette

Copy link
Copy Markdown
Author

I'll get that issue added and add an e2e test before we merge this.

Comment thread src/deadline_worker_agent/installer/install_darwin.sh Outdated
@andychoquette andychoquette changed the title feature: add macos installer feat: add macOS (darwin) support to the worker agent installer Jul 17, 2026
@andychoquette

Copy link
Copy Markdown
Author

added e2e test in 3853d23

Comment thread src/deadline_worker_agent/installer/install_macos.sh
install-deadline-worker rejected every platform except Linux and Windows, so
macOS hosts could not be configured as workers in a customer-managed fleet even
though the CMF fleet OS enum already accepts MACOS.

- __init__.py: allow 'darwin' through the platform gate; add a 'darwin' entry to
  INSTALLER_PATH so install() dispatches through the existing sudo path to a new
  install_darwin.sh; reject --vfs-install-path on macOS (VFS is Linux-only).
- install_darwin.sh (new): macOS port of install.sh. Creates the agent user and
  job group via Directory Services (dscl/dseditgroup) instead of
  useradd/groupadd, preserving the jobRunAsUser isolation model; provisions the
  same directories/permissions; writes worker.toml via the same config module;
  installs a launchd LaunchDaemon instead of a systemd unit. Uses a portable
  while-loop for argument parsing (macOS ships BSD getopt, which lacks
  --longoptions).
- settings.py + arg-parser default: add DEFAULT_MACOS_SESSION_ROOT_DIR
  (/var/lib/deadline/sessions) and select it on darwin, because the macOS root
  volume is sealed read-only and /sessions cannot be created there.
- Tests: cover the darwin dispatch path, --vfs-install-path rejection, and the
  platform-correct session-root default; drop darwin from the
  unsupported-platform test.

Linux and Windows install paths are unchanged. Running a job as a jobRunAsUser
on macOS additionally requires the companion openjd-sessions changes.

Validated end-to-end on macOS 26.5 (arm64) against a live customer-managed
fleet: install, worker registration, running a job as the jobRunAsUser, and
cancellation.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
The plist previously combined RunAtLoad=true with an unconditional
`launchctl bootstrap`, so the agent started as soon as the installer
ran even without --start. An intermediate fix gated RunAtLoad instead,
but that broke start-on-boot for non---start installs (launchd loads
/Library/LaunchDaemons plists at every boot, and with RunAtLoad=false
plus no other trigger the daemon would never run at all). launchd has
no separate start-now/start-on-boot controls the way systemd separates
`systemctl start` from `systemctl enable`, so the two must be split
differently:

- The plist is always written boot-ready (RunAtLoad=true with
  KeepAlive/SuccessfulExit for restart-on-failure); installing it into
  /Library/LaunchDaemons is the systemctl-enable analog: launchd loads
  and starts it on the next boot.
- `launchctl bootstrap` (load now, which per RunAtLoad also starts
  now) only runs with --start -- the systemctl-start analog.

Caught by the macOS installer CI workflow asserting on the process
table and launchd registration state after installs with and without
--start.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Adds test/integ/macos/test_installer.py, following the pattern of the
Windows installer integration tests: pytest tests that run the real
installer (via sudo) and assert the invariants it must establish:

- agent user hidden with no interactive shell; its PRIMARY group is a
  dedicated per-user group and the job group is secondary-only (the
  credential-isolation boundary)
- credentials dir 700 and denied to unprivileged users, config
  dir/worker.toml 750/640 root-owned and well-formed TOML with the
  configured farm/fleet ids, session root under /var, logs 750
- LaunchDaemon plist 644 root:wheel, parses, runs as the agent user,
  boot-ready (RunAtLoad=true + KeepAlive/SuccessfulExit); without
  --start the service is not registered with launchd and no agent
  process runs; with --start it is registered (which also proves
  launchd accepts the plist), then booted out
- sudoers file 440, validates with visudo, grants exactly
  '/sbin/shutdown -h now', and is revoked on a re-run without
  --allow-shutdown
- --vfs-install-path is rejected before any system mutation
- a second run is idempotent

The tests mutate host state (users, groups, LaunchDaemon, sudoers) so
they are gated behind RUN_INSTALLER_TESTS=true and skip everywhere
else, including 'hatch run integ-test' on a developer Mac. The
macos_installer_test.yml workflow sets the gate and runs them on
macOS runners, with the agent installed from the checkout into a venv
and never successfully starting (fake farm/fleet ids, no AWS access).
Actions are hash-pinned per the repository's zizmor blanket policy.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Comment thread src/deadline_worker_agent/installer/install_darwin.sh Outdated
…he bootout/bootstrap race

Two related re-install problems in the launchd load logic:

1. A config-only re-run (no --start) over a loaded service booted the
   service out to pick up the new plist and never put it back, leaving
   the worker offline until the next reboot. On Linux the equivalent
   re-run leaves a running service running. Track whether the service
   was loaded before the bootout and re-bootstrap it afterward even
   without --start. A re-run over an unloaded service still leaves it
   unloaded.

2. launchctl bootout is asynchronous: bootstrap immediately after it
   can fail transiently while the old instance is still unloading,
   aborting the installer under set -e. Retry bootstrap for up to ten
   seconds, then run it unguarded once more so a persistent failure
   still surfaces its real error message.

Both caught by review on the macOS installer PR; covered by new
integration tests (reinstall-with---start race, config-only re-run
restore, and unloaded-stays-unloaded).

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
deadline-cloud-test-fixtures >= 0.18.16 (2026-07-17) added a numpy
dependency, and mypy reaches numpy's stubs through pytest's approx
implementation when checking the test tree. numpy 2.5+ stubs use PEP
695 'type' statements, which mypy rejects with a syntax error while
python_version is pinned to 3.10, failing 'hatch run lint' on every
platform. numpy is not used by this package, so skip following imports
into it (follow_imports_for_stubs is required for the setting to apply
to .pyi files).

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
@andychoquette
andychoquette requested a review from baxeaz July 20, 2026 15:04
crowecawcaw
crowecawcaw previously approved these changes Jul 20, 2026
@crowecawcaw

Copy link
Copy Markdown
Contributor

New test looks good! For testing, I think we want an additional test that spins on a Mac instance, runs this install script, and verifies the worker registers to the real Deadline service and picks up a task. Even better if it also triggers the instance to restart and verifies it re-attaches successfully.

Comment thread src/deadline_worker_agent/installer/install_macos.sh
crowecawcaw
crowecawcaw previously approved these changes Aug 11, 2026
Comment thread src/deadline_worker_agent/installer/install_macos.sh
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh
Comment thread src/deadline_worker_agent/installer/install_macos.sh
The macOS cross-user path needs the pure-Python session-leader shim added in
openjd-sessions#335, because macOS ships no setsid(1). That merged as 2df2d99,
after the 0.10.13 release, so the previous `== 0.10.13` pin resolved to a version
without it -- a macOS install would have had the installer support from this PR
and no working impersonation, failing every action at `setsid` with exit 127.

0.10.14 is the first release containing it (see the changelog in
openjd-sessions-for-python#347).

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
…me, widen id search

Four review findings on install_macos.sh.

1. SECURITY: --user naming an existing normal account resolved wa_group to that
   account's primary group, which on macOS is `staff` (GID 20) for any
   Setup-Assistant or MDM-created user. wa_group is the group owner of
   /etc/amazon/deadline (worker.toml, mode 640) and /var/log/amazon/deadline, so
   that published the agent's config and logs to every local user. Verified on
   macOS 26.5: three unrelated local accounts share primary group staff.

   install.sh is safe doing the same thing only because Linux useradd guarantees a
   dedicated single-member primary group; that guarantee does not exist here. The
   installer now refuses staff/admin/everyone/wheel/_unknown/nogroup with an error
   naming the fix. The existing invariant check did not catch this: it compares the
   primary group against job_group only, so staff passed silently.

2. find_unused_system_id searched a single namespace, so the group and the user
   could be assigned the same number: on a stock image 499 is free as both a UID
   and a GID, so the first install took gid=499 then uid=499. It now searches the
   union of both namespaces. The group record is written before the UID lookup, so
   the second call sees the first allocation and the two cannot collide.

3. worker_agent_homedir was hardcoded even when --user named an existing account
   with a different NFSHomeDirectory. launchd derives the daemon's HOME from the
   user record, not from the plist's WorkingDirectory, so HOME and the CWD pointed
   at different directories: anything resolving ~ (botocore's ~/.aws, its caches)
   landed somewhere never provisioned or chowned, while the directory the installer
   did provision went unused. An existing account's home is now adopted.

4. Corrected the Password '*' comment, which claimed Directory Services returns
   eDSAuthMethodNotSupported because no AuthenticationAuthority is present. That
   is not documented and not safe to assert. Kept the mechanism -- `dscl . -read
   /Users/daemon` shows Apple's own service accounts use exactly Password '*' with
   no AuthenticationAuthority and UserShell=/usr/bin/false -- but the comment now
   credits the account's non-interactivity to that combination rather than to '*'
   being interpreted as "never matches".

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh
…rsing

Three problems, all introduced by the NFSHomeDirectory adoption in 433ed0f.

1. Adopting the recorded home and then unconditionally chown/chmod-ing it could
   take over a SHARED system directory. Most of Apple's _-prefixed service
   accounts record /var/empty (root:wheel 0555, also sshd's privsep chroot) and
   `daemon` records /var/root, so a hand- or MDM-provisioned deadline-worker
   following that convention would have had /var/empty chowned to the agent user
   and narrowed to 750 -- a host-wide change well outside this installer's remit.
   Some service accounts use /dev/null, where `[[ ! -d ]]` passes and `mkdir -p`
   then fails with "File exists", aborting the install under set -e.

   The installer now only provisions a directory it creates itself, re-asserts
   ownership only when the directory is already owned by the agent user (the
   re-install case), and otherwise warns and leaves it alone. Verified against
   /var/empty (untouched, warned) and /dev/null (warned, no abort).

2. `awk '{print $2}'` truncated the home directory at the first space:
   "NFSHomeDirectory: /Users/Deadline Worker" yielded "/Users/Deadline", a
   different and probably nonexistent path that would then be provisioned and
   written into the plist while the account's real HOME stayed elsewhere. Uses
   sed to take the whole value.

   The same assumption in find_unused_system_id was worse: `dscl . -list` prints
   "name<padding>id", so a record name containing a space made $2 a fragment of
   the NAME, dropping that id from the used set and allowing it to be handed out
   again -- a duplicate-UID install rather than a clean failure. Now takes $NF,
   and skips lines with no attribute rather than emitting an empty field.

3. worker_agent_homedir went into the plist unescaped while ProgramArguments was
   escaped, even though it is now read from a Directory Services record rather
   than being a hard-coded constant. A home directory containing & < or > would
   produce a plist that is not well-formed XML, which launchd rejects opaquely.
   Escaped with the same xml_escape helper; verified paths containing "&" and
   "<>" now lint clean and round-trip through plistlib intact.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Comment thread src/deadline_worker_agent/installer/install_macos.sh
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Comment thread src/deadline_worker_agent/installer/install_macos.sh
Comment thread src/deadline_worker_agent/installer/install_macos.sh
…ucturally

Five review findings, most of them fallout from my own earlier changes.

1. SECURITY: the is_broad_group check only ran inside the `user_exists` branch, so
   `--user staff` bypassed it entirely -- the account does not exist, wa_group
   becomes "staff", and the group-creation block then adopts the existing staff
   record (GID 20) as the new account's primary group. Same exposure the check was
   added to prevent, by a different route. Hoisted out of the conditional so it
   runs on the resolved wa_group in both paths. Verified `--user admin` is also
   caught, since admin's primary group resolves to staff.

2. SECURITY: job_group had no such check at all, and it is group owner of
   /var/lib/deadline and queues/ (0750) and the session root (0755) -- modes
   deliberately loosened so job users can reach them. `--group staff` would let any
   local user list queues/ and traverse into session directories to read job
   attachment inputs and generated scripts. credentials/ stays 0700, so AWS
   credentials were never exposed. Now checked after its syntax validation.

3. `sed -n 's/^NFSHomeDirectory: //p'` failed for the exact case it was added for.
   `dscl -read` prints a whitespace-containing value on an indented CONTINUATION
   line, not inline -- confirmed: `RealName:` then " Choquette, Andy", versus
   "NFSHomeDirectory: /var/empty" inline. So a spaced home came back empty and the
   installer silently fell back to the default, provisioning a directory that is
   not the account's home. Replaced with a dscl_read_value helper using
   `dscl -plist` + `plutil -extract`, which is unambiguous for both shapes, and
   routed the PrimaryGroupID reads through it too so there is one parsing path.

4. An unusable WorkingDirectory is fatal, not cosmetic: launchd chdir()s before
   exec, so /dev/null (a real service-account home) makes every spawn fail with
   KeepAlive throttling the retry -- a service that never runs. It now falls back
   to /var/lib/deadline, which this installer provisions, and the warning says the
   real consequence instead of implying only ~ is affected.

5. Fail loudly when an adopted group record has no PrimaryGroupID, rather than
   running `dscl -create ... PrimaryGroupID ""`.

Also documents the session-root traversal divergence from Linux (id 3762317005):
nesting under 0750 /var/lib/deadline means traversal needs job_group membership,
which /sessions on Linux does not. Left as-is deliberately -- it is tighter than
Linux and matches the documented model where every jobRunAsUser is in the shared
job group -- but a jobRunAsUser outside that group gets EACCES, which the
worker-host docs need to state.

NOT changed: the claim that `set -e` aborts before the `-z "${wa_group}"` fallback
because user_primary_group_name returns 1. An `if` whose condition is false and
which has no `else` returns 0, so the function returns 0 and the fallback is
reachable; verified with a nonexistent user (reached the fallback, exit 0) and
with a gid matching no group record (`dscl -search` exits 0 on no match).

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
Two mypy errors, both surfacing in the E2E Lint job.

1. conftest.py:621 -- deadline-cloud-test-fixtures types OperatingSystem.name as
   Literal["AL2023", "WIN2022"], so `OperatingSystem(name="MACOS")` is rejected.
   That package needs macOS support before this can type-check properly (it also
   needs a MacInstanceWorker: the posix worker hardcodes an AL2023 AMI and
   provisions with useradd/groupadd, neither of which exists on macOS). Nothing
   sets OPERATING_SYSTEM=macos in CI yet, so the branch is unreachable today and is
   kept only so the plumbing is in place; narrow `# type: ignore[arg-type]` with a
   comment saying what removes it.

2. test_session_runtime.py:544 -- PRE-EXISTING, not from this branch: mainline
   alone fails this same check, in a file added by aws-deadline#1035 that this PR does not
   touch. `worker.worker_id` is `str | None` and `is_worker_started` wants `str`;
   the `assert ... is not None` above does not narrow inside the nested function,
   because an attribute could change between the assert and the call. Bound to a
   local so the narrowing holds. Fixed here rather than deferred because it blocks
   the E2E Lint job for every PR, this one included.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
Comment thread src/deadline_worker_agent/installer/install_macos.sh Outdated
…n it cannot be

Two findings on user_primary_group_name and its caller.

1. `dscl . -search` is documented as a SUBSTRING match on the attribute value, so
   searching for gid 20 can also match 120/200/2000, and `awk 'NR==1{print $1}'`
   then took whichever record dscl emitted first. Replaced with the conventional
   `dscl . -list` plus an exact `$NF == gid` comparison, using $NF rather than $2 so
   a group name containing a space cannot shift the fields, and gave the function an
   explicit `return 0` so its status no longer depends on the last command run.

   A misresolution here is not cosmetic: the result becomes the group owner of
   /etc/amazon/deadline (0750), worker.toml (0640) and the agent's logs, and it gates
   the job-group-is-not-the-primary-group invariant. is_broad_group cannot catch it,
   because it checks the resolved NAME and a wrongly-resolved name is not in
   broad_groups.

   Note the substring behaviour did not reproduce on macOS 26.5 -- `-search 20`
   returned only staff, though eight groups on this host have GIDs containing "20" --
   so this is hardening against documented behaviour rather than an observed
   misresolution. The `-list` form is deterministic either way, and the exit-status
   fix is real regardless.

2. The `[[ -z "${wa_group}" ]]` fallback named a group with no directory record: the
   block that creates /Groups/${wa_user} only runs on the create-USER path, so on
   this branch nothing ever creates it, and every later
   `chown "${wa_user}:${wa_group}"` would fail with "invalid group" -- aborting under
   set -e possibly after the job group, its membership, and the sudoers rule were
   already applied. Now a hard error before anything is mutated, naming the three
   ways out. Reachable without any resolver bug: a PrimaryGroupID pointing at a GID
   whose group record no longer exists resolves to nothing.

Verified: gid 20 -> staff, 204 -> _developer (not staff), 0 -> wheel, 99999 -> empty;
the full resolution flow survives set -e for a normal user, a service account and a
to-be-created user; and the dangling-PrimaryGroupID case now exits 1 up front.

Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com>
<false/>
</dict>
<key>RunAtLoad</key>
<true/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The plist sets no ExitTimeOut, so launchd applies its default of 20 seconds: on launchctl bootout / launchctl kickstart -k / system shutdown, launchd sends SIGTERM and then SIGKILLs the agent 20s later.

The Linux unit sets no TimeoutStopSec, so it gets systemd’s default of 90s. That is a ~4.5x reduction in drain time that exists only on macOS.

This matters because SIGTERM is exactly what starts the agent’s graceful drain (worker.py:139 registers _signal_handler, which drives Scheduler.shutdown), and that drain is deliberately unboundedscheduler.py:1471 passes gracetime = None with the comment "Let the cancels happen as defined in the Job Template". So the scheduler waits for each session’s cancelation as the Job Template defines it, which routinely exceeds 20s. Consequences on macOS:

  • SIGKILL at t=20s skips _agent_shutdown, so the worker never reports STOPPED and sits in STOPPING until the service-side timeout.
  • Session subprocesses (sudo -u <jobRunAsUser> ...) are orphaned rather than cancelled, leaving job processes and session directories behind.
  • --allow-shutdown autoscaling drains are truncated the same way, since _repeatedly_attempt_host_shutdown runs inside that same window.

Note also that test/e2e/test_job_submissions.py::test_worker_enters_stopping_state_while_draining uses launchctl bootout against a sleep 600 task and allows 120s. The 20s SIGKILL would land well inside that window, and because the test accepts STOPPING it would still pass while the process was being hard-killed.

Suggest setting ExitTimeOut explicitly so the macOS grace period matches Linux — an ExitTimeOut integer key of 90 alongside KeepAlive/RunAtLoad. (0 means "wait forever", also an option if a fully unbounded drain is intended, but a finite value matching systemd’s default is the closer port.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-maintainers Waiting on the maintainers to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants