Add linux-amd64 binary to installer-artifacts #4891
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Per our discussions on adding this to oc |
| @@ -4,8 +4,8 @@ | |||
| FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.15-openshift-4.8 AS builder | |||
| WORKDIR /go/src/github.com/openshift/installer | |||
| COPY . . | |||
| RUN go generate ./data && \ | |||
| SKIP_GENERATION=y GOOS=darwin GOARCH=amd64 hack/build.sh | |||
| RUN make cross-build-darwin-amd64 cross-build-linux-amd64 | |||
There was a problem hiding this comment.
Why do you need to build for Linux here? Don't we get that via the FROM ...:installer line below?
There was a problem hiding this comment.
We want to have a linux/amd64 installer even in the other architecture's payloads. For remote deployments, just like we enable installing from mac regardless of cluster architecture, we also want to enable doing such from Linux on commodity hardware, which is still most commonly x86.
|
@abhinavdahiya From comments in past PRs that have tried to add a makefile, you seem to be the biggest advocate against adding a makefile. I would love your input here on whether adding a makefile to ease building for cross-platform architectures would alleviate your concerns that using make is no easier that using the build script. |
| # This Dockerfile builds an image containing the Mac version of the installer layered | ||
| # on top of the Linux installer image. |
There was a problem hiding this comment.
Is this comment still accurate?
|
|
||
| FROM registry.ci.openshift.org/ocp/4.8:installer | ||
| COPY --from=builder /go/src/github.com/openshift/installer/bin/openshift-install /usr/share/openshift/mac/openshift-install | ||
| COPY --from=macbuilder /go/src/github.com/openshift/installer/_output/bin/darwin_amd64/openshift-install /usr/share/openshift/mac/openshift-install | ||
| COPY --from=linuxbuilder /go/src/github.com/openshift/installer/_output/bin/linux_amd64/openshift-install /usr/share/openshift/linux_amd64/openshift-install |
There was a problem hiding this comment.
Help me better understand this. Is ART replacing registry.ci.openshift.org/ocp/4.8:installer with architecture-specific images based on the target architecture of the OpenShift release image?
Depending on the architecture of the release image, this binary may be a duplicate of the one from registry.ci.openshift.org/ocp/4.8:installer, right? But we want to include the duplicate
- so that
ochas a consistent way of extracting the installer binary for the user's desired architecture and - because there is no good way to use a link instead since the Dockerfile does not know which architecture ART is going to use when replacing
registry.ci.openshift.org/ocp/4.8:installer.
There was a problem hiding this comment.
That pretty well describes it.
This will be used by oc in the next release to allow extracting an x86_64 installer from another architecture's payload. Due to the size of the installer binary, building each in a separate stage minimizes resource requirements when using imagebuilder. This does require a matching change to ocp-build-data to add a second builder stage.
|
/test all |
|
/retest |
|
don't see how this affecths the upgrade test |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: staebler The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
This is necessary after openshift/installer#4891
No description provided.