fix(service)!: make DOCKER_HOST module-owned; require uid#32
Merged
Conversation
DOCKER_HOST for the managed runner service is now derived from runner_account.uid and rendered alongside any runner_service.environment vars. A DOCKER_HOST line in runner_service.environment is rejected at compile time, and runner_service.manage now requires runner_account.uid. The manager's connection, the healthcheck, and socket_mount all read the one derived socket, so they can no longer diverge; an emptied environment can no longer leave the manager without DOCKER_HOST. BREAKING CHANGE: runner_account.uid is required whenever runner_service.manage is on, and DOCKER_HOST can no longer be set via runner_service.environment.
Update the runner_service narrative and the host-requirements DOCKER_HOST note: the managed service derives DOCKER_HOST from runner_account.uid; runner_service.environment adds other variables and a DOCKER_HOST line there fails the compile; an external daemon is a per-runner host concern, not a supported manager override.
Describe the runners `host` key as GitLab Runner's native `[runners.docker]` host passthrough, independent of the module-owned DOCKER_HOST, instead of framing it as support for an external daemon.
jcardozo-eth
added
breaking
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
1 participant
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.
Closes #26.
Changes
Makes
DOCKER_HOSTfor the managed runner service module-owned: derived fromrunner_account.uidand always rendered into the drop-in, so the manager, the healthcheck, andsocket_mountall use the one rootless socket and can no longer diverge.runner_service.environmentis now a passthrough for additionalEnvironment=lines only; aDOCKER_HOSTline there fails the compile, and a runner whose jobs need a different daemon uses the per-runnerhostkey.runner_service.managenow requiresrunner_account.uid.This resolves the reported bug: an empty or emptied
runner_service.environment(including one reduced to[]by the--knockout prefix) can no longer leave the manager withoutDOCKER_HOST, because the derived line is always present.Breaking:
runner_account.uidis required wheneverrunner_service.manageis on, andDOCKER_HOSTcan no longer be set throughrunner_service.environment.Verification
just check: 120 tests, plus lint, validation, and doc generation).DOCKER_HOST, requiring a uid when the service is managed, and rejecting a hand-setDOCKER_HOST. For each one, the matching test was checked to fail when that safeguard is removed, so the tests genuinely cover the behaviour rather than passing by accident.runner_service.environmentline that its shared (fleet-wide) config had set, using the--removal marker, which leaves the environment list empty. The test confirms the service still gets the correctDOCKER_HOST(derived from the uid) instead of none — the exact situation that used to break.