Skip to content

docker-engine: use systemd units conf from sources - #283

Merged
crazy-max merged 2 commits into
docker:mainfrom
crazy-max:fix-docker-engine-systemd
Oct 7, 2025
Merged

docker-engine: use systemd units conf from sources#283
crazy-max merged 2 commits into
docker:mainfrom
crazy-max:fix-docker-engine-systemd

Conversation

@crazy-max

Copy link
Copy Markdown
Member

No description provided.

@crazy-max

Copy link
Copy Markdown
MemberAuthor

build failure not related, see #284

@crazy-max
crazy-max marked this pull request as ready for review October 4, 2025 12:16
Comment threadpkg/docker-engine/deb/rules Outdated
Comment on lines +38 to +39
install -D -p -m 0644 /common/systemd/docker.service debian/docker-ce/lib/systemd/system/docker.service
install -D -p -m 0644 /common/systemd/docker.socket debian/docker-ce/lib/systemd/system/docker.socket
install -D -p -m 0644 engine/contrib/init/systemd/docker.service debian/docker-ce/lib/systemd/system/docker.service
install -D -p -m 0644 engine/contrib/init/systemd/docker.socket debian/docker-ce/lib/systemd/system/docker.socket

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ISTR that in docker-ce-packaging we changed the way to install these to be the more idiomatic way, which uses a dangling symlink in the deb/common directory, which gets satisfied by the source code; I think this was the commit doing so; docker/docker-ce-packaging@5c01d5b

And this PR;

There may be other similar changes (but we'd have to check for those); the TL;DR is that we were doing various things manually, which was not the "normal" way of doing things; it works, but basically with some of these overrides we were going against the stream of what the deb packages would normally expect.

@crazy-max
crazy-maxforce-pushed the fix-docker-engine-systemd branch from 125cd63 to 696d124CompareOctober 6, 2025 10:23
Comment threadpkg/docker-engine/Dockerfile Outdated
Comment on lines +176 to +177
--mount=type=bind,source=common/systemd/docker.service,target=/root/rpmbuild/SOURCES/docker.service \
--mount=type=bind,source=common/systemd/docker.socket,target=/root/rpmbuild/SOURCES/docker.socket \
--mount=type=bind,from=src-tgz,source=/out/engine.tgz,target=/root/rpmbuild/SOURCES/engine.tgz \
--mount=type=bind,from=src,source=/src/contrib/init/systemd/docker.service,target=/root/rpmbuild/SOURCES/docker.service \
--mount=type=bind,from=src,source=/src/contrib/init/systemd/docker.socket,target=/root/rpmbuild/SOURCES/docker.socket \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wondering if it's already extracting engine.tgz so if the files are already present ... somewhere and .. maybe not needed to be mounted separately, but if this works, we can look at that in a follow-up I guess 😅

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ah good catch!

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-maxforce-pushed the fix-docker-engine-systemd branch from 696d124 to d8b982cCompareOctober 6, 2025 14:25

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

... based on CI being green 😂 - I assume it verifies the files are there somehow?

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max

Copy link
Copy Markdown
MemberAuthor

LGTM

... based on CI being green 😂 - I assume it verifies the files are there somehow?

Pushed extra commit to validate systemd unit is there (md5sums) and taken into account (postinst).

@crazy-max
crazy-max merged commit bae8240 into docker:mainOct 7, 2025
61 checks passed
@crazy-max
crazy-max deleted the fix-docker-engine-systemd branch October 7, 2025 08:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@crazy-max@thaJeztah@vvoland