Skip to content

Run the self-hosted incremental build without --privileged #5682

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.

Sign up for GitHub

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

Open
anchapin wants to merge 2 commits into
base: develop
Choose a base branch
from
+1 −1
Open

Run the self-hosted incremental build without --privileged #5682

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
893f851
Run the self-hosted incremental build container without --privileged
Nov 6, 2026
218ad11
Merge remote-tracking branch 'origin/develop' into incremental-unpriv…
Nov 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/incremental-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

container: # Define the Docker container for the job. All subsequent steps run inside it.
image: nrel/openstudio-cmake-tools:noble-main
options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Dropping --privileged doesn't close the host-takeover path by itself.

The runner adds -v "/var/run/docker.sock":"/var/run/docker.sock" to every Linux job container, though this workflow never asks for it. It's in the docker create line under "Initialize containers" in the last run, and it comes from ContainerInfo.cs in actions/runner. With -u root, any step can still reach the host Docker daemon and ask it for a new container with whatever privileges and host mounts it likes. The earlier --privileged runs started without --userns=host, so the daemon isn't using userns-remap: unless it runs rootless, container root is host root.

Nothing in options: or volumes: can remove that mount: a second mount at the same path fails docker create. Closing it takes a runner-side change, for example:

  • container hooks (ACTIONS_RUNNER_CONTAINER_HOOKS) that create the job container without the socket and as a non-root user (the image's default user is ubuntu; -u root is what makes the job uid 0)
  • a rootless or userns-remapped daemon
  • ephemeral runners

The change is still worth having: seccomp and AppArmor come back on, and host devices and /sys are no longer exposed. But the description overstates what it fixes.

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

os-4.0-dev still runs this job privileged on the same runner.

That branch has its own copy of this workflow: runs-on: linux-openstudio-2, options: --privileged -u root ..., and the jammy image. A pull_request run takes its workflow from the PR's merge ref. So after this merges, labeled PRs into os-4.0-dev will keep starting privileged containers here; for example, the open, already-labeled PR 5640 will on its next push.

develop hasn't been merged into os-4.0-dev since May, so the fix won't carry over on its own. Consider making the same one-line change on os-4.0-dev, or enforcing container options on the runner side so that no branch's copy can opt back in.

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Test plan: adding the label won't start a run.

on: pull_request (lines 7-8) has no types:, so only opened, synchronize and reopened start this workflow. Labeling fires labeled. Re-running the earlier skipped run reuses its original, unlabeled payload, so it is skipped again. Past PRs show the same pattern: PR 5626 ran only after a push that followed the label, and PR 5619 only after a close/reopen.

So you need a push, or a close/reopen, after adding the label. Until then build stays "skipped", and this could merge without the unprivileged container ever having run on the runner.

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On the fallback in the description ("the failing step will say which, and we can add that one capability"):

A denial in an unprivileged container is usually a bare Operation not permitted or Permission denied, and it doesn't name a capability. Also, several things --privileged lifts aren't capabilities at all:

  • seccomp blocks such as personality(ADDR_NO_RANDOMIZE) or keyctl
  • AppArmor's deny mount
  • masked /proc and /sys paths
  • missing /dev nodes

Those need --security-opt apparmor=..., seccomp=... or systempaths=unconfined, or --device, not --cap-add. Nothing in this job looks like it needs any of them, so this only matters for diagnosing a failure if one shows up.

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nit (optional, probably better as a follow-up): the image already sets LANG=en_US.UTF-8 and LC_ALL=en_US.UTF-8 (the openstudio-cmake-tools Dockerfile), so -e "LANG=en_US.UTF-8" is a no-op. full-build.yml passes the same flag in three matrix entries, so dropping it only here would add drift.

volumes: # envs don't work in volume definition for containers
- "/srv/data/jenkins/docker-volumes/conan-data-gha/.conan2:/github/home/.conan2" # Conan cache, separate from the Jenkins builds on this host (the conan cache is not safe for concurrent use)
- "/srv/data/jenkins/docker-volumes/ubuntu-2404:/github/home/Ubuntu"

@anchapin anchapin Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

"Every later job on it" still holds, with no privilege needed.

The job runs as root with read-write access to state that later runs reuse as-is:

  • the persistent source and build tree on this line
  • the Conan home on line 52
  • the runner's reused workspace (/__w)
  • the ccache store

Anything one run's build writes there carries into the next PR, push or dispatch run, and into the packages those runs upload. None of this depends on --privileged, so it isn't caused by this PR. Closing it would mean not sharing writable state between PR runs, or rebuilding that state from a trusted run.

Separately, the actions/cache step on ~/.ccache restores nothing. ccache 4 keeps its store in ~/.cache/ccache unless ~/.ccache already exists, and the last run logged "Path(s) specified in the action for caching do(es) not exist" while ccache started with a full 2 GB store.

Expand Down
Loading