images/installer: Rewrite tectonic-installer for openshift-install #343
Conversation
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
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/M
wking
force-pushed
the
next-gen-dockerfile
branch
from
c7ea759 to
7404847
Compare
openshift-ci-robot
added
size/S
The golang-1.10 image has everything we need to build now. I run the build in a golang-1.10 container using 'FROM ... AS ...' [1] like we used to. But now I no longer install packages with yum. And I use a recursive COPY [2] to bring the built/fetched binaries over into the output container, which is based on scratch. Once we switch openshift/release over to this Dockerfile, we can drop images/tectonic-installer. [1]: https://docs.docker.com/engine/reference/builder/#from [2]: https://docs.docker.com/engine/reference/builder/#copy
wking
force-pushed
the
next-gen-dockerfile
branch
from
7404847 to
29e4d10
Compare
Member
Author
|
Dropped the |
Contributor
|
/lgtm |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
|
/test images |
Member
Author
|
The images failure was: |
Member
Author
|
/retest |
Contributor
|
These seem like errors from |
Member
Author
|
For a similar error earlier, @smarterclayton pointed at this indent issue. Looks like that's currently still in master, but it could also be another busted JSON/YAML file causing the "operator "catalog" failed to map images: yaml: line 16: mapping values are not allowed in this context". |
Member
Author
|
More work on that indent issue ongoing in operator-framework/operator-lifecycle-manager#486. |
Contributor
|
Goal is to have more gating in place in the next few days to mitigate the impact here - everything landing at once etc |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Member
Author
|
operator-framework/operator-lifecycle-manager#486 landed. /retest |
Member
Author
|
/retest |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Member
Author
|
The images presubmit is new today with openshift/release#1704. Maybe we shoulfd roll it back temporarily? |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
wking
added a commit
to wking/openshift-release
that referenced
this pull request
Sep 27, 2018
openshift-install is he next-gen installer. With the new installer, we only need the installer binary and terraform to launch and destroy clusters. I've removed the smoke tests for now, because I haven't looked into building them without Bazel. Hopefully we'll get expanded e2e testing *and* other OpenShift projects using our installer soon to keep us honest. Also missing from the new installer is a way to set expirationDate. But we'll just not leak until we regain the ability to set that, right? ;) The new installer dumps less cruft into the output directory (most of the generated output goes into a temporary directory), so I've adjusted the openshift-install calls to just use an artifacts subdir for their state storage. A bonus of this approach is that if the installer hangs up, we'll capture anything it wrote to disk without needing an explicit cp call. A drawback is that we'll leak any secrets that get put into the kubeconfig, so don't put anything in there that needs to stay private after the cluster is reaped. Using the base image for the installer's 'from' is quite a bit different from openshift/installer@29e4d10e (origin/pr/343) images/installer: Rewrite tectonic-installer for openshift-install, 2018-09-26, openshift/installer#343), where the Dockerfile is 'FROM scratch'. Including the OpenShift base currently adds ~230 MB to the installer layer's 110 MB for the two binaries, although both of those are uncompressed sizes. Gzipping layers reduces the sizes to around 84 MB and 26 MB respectively. So the added base cruft is not huge, but it's still hefty. The upside of using the base image is that we have a standard POSIX-ish system for executing the cleanup script. To demonstrate the 'FROM scratch' approach, I've adjusted the setup container to call the installer directly (with no wrapping shell script). And to support that, I've shifted some waiting code over into the test container (which also saves us from having to copy 'oc' around). I've also dropped the 3.11 config, since the installer is 4.0-only.
wking
commented
Sep 27, 2018
| FROM openshift/origin-release:golang-1.10 AS build | ||
| WORKDIR /go/src/github.com/openshift/installer | ||
| COPY . . | ||
| RUN hack/build.sh && hack/get-terraform.sh |
Member
Author
There was a problem hiding this comment.
We'll have to replace the curl-based get-terraform.sh here with a yum call or some such to comply with Red Hat's OCP Automated Release Policies. But that's probably not too much of a rush. I think get-terraform.sh should stay curl-based to be package-manager agnostic.
Contributor
There was a problem hiding this comment.
Vendoring a copy of terraform into a repo is probably the best option if you need a specific tagged commit. If you have to change X at a high rate, the pain you'll feel from RPM iteration will outweigh any ugliness of vendoring.
wking
added a commit
to wking/origin
that referenced
this pull request
Apr 13, 2019
The installer Dockerfile has put the binary in /bin since openshift/install@29e4d10eb7 (images/installer: Rewrite tectonic-installer for openshift-install, 2018-09-26, openshift/installer#343). Fixes: $ oc adm release extract --command=openshift-install registry.svc.ci.openshift.org/openshift/origin-release:v4.0 error: image did not contain usr/bin/openshift-install
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
The golang-1.10 image has everything we need to build now. I run the build in a golang-1.10 container using
FROM ... AS ...like we used to. But now I no longer install packages withyum. And I use a recursiveCOPYto bring the built/fetched binaries over into the output container, which is based onscratch./assign @abhinavdahiya