-
Notifications
You must be signed in to change notification settings - Fork 161
Fix image based build to address for upstream changes #417
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.
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
Conversation
Upstream moby/moby#40180 removed the auto-generated code, replacing it with build-time variables (-X). This patch updates the Dockerfiles to account for this change. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
ping @arkodg @zelahi @StefanScherer PTAL 🤗 |
| RUN go build -o /dockerd \ | ||
| -tags 'autogen apparmor seccomp selinux journald' \ | ||
| RUN source ./hack/make/.go-autogen; \ | ||
| go build -o /sbin/dockerd \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This /sbin/dockerd will cause a problem in the last stage. There's a COPY --from in line 80 that uses /dockerd as source.
This lead to a next question, why are there two relatively similar Dockerfile.engine* files that we have to maintain?
At least we should use the same pathes in both Dockerfiles, so the COPY instruction must be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂ I worked on one file, then copied to the other file, but forgot they had different paths.
| ARG PLATFORM | ||
| ARG PRODUCT | ||
| ARG DEFAULT_PRODUCT_LICENSE | ||
| ARG TINI_COMMIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to pass in that argument somewhere with --build-arg TINI_COMMIT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm.. good one; let me check if it's passed somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even wondering how this worked at all, because the binaries-commits file was removed in moby/moby@b529d1b
seemethere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These files don’t matter anymore since docker engine activate was deprecated and should be removed.
|
Yeah, perhaps we should just go for #411, which removes this stuff. I started with this because I thought the problem was in build in general, but it was only in the image-based build. |
|
superseded by #411 |
depends on moby/moby#40393
Upstream moby/moby#40180 removed the auto-generated code, replacing
it with build-time variables (-X).
This patch updates the Dockerfiles to account for this change.