Skip to content

drop supermin for upload-oscontainer, use digestfile (not skopeo)#1502

Merged
openshift-merge-robot merged 2 commits intocoreos:masterfrom
cgwalters:oscontainer-buildah-vfs
Jun 3, 2020
Merged

drop supermin for upload-oscontainer, use digestfile (not skopeo)#1502
openshift-merge-robot merged 2 commits intocoreos:masterfrom
cgwalters:oscontainer-buildah-vfs

Conversation

@cgwalters
Copy link
Copy Markdown
Member

In general we should avoid doing any nontrivial networking in
supermin because usermode networking is slow. Further, as of
OpenShift 4 it's documented how to use buildah inside OpenShift,
so let's do that rather than supermin.

Motivated by debugging why the oscontainer push in the RHCOS
pipeline is 2 hours.

Closes: #1500

@cgwalters
Copy link
Copy Markdown
Member Author

Tested in a dev cosa shell in PSI, and this works quickly. I also ensured we clean up our temporary storage after we're done.

@cgwalters
Copy link
Copy Markdown
Member Author

Tangentially: Also I investigated building the oscontainer using a proper BuildConfig. There would be a lot of advantages to that obviously, but one wrinkle there is that we'd need to make a separate BC per version because currently BCs don't support having the build process itself inject labels AFAICS.

In general we should avoid doing any nontrivial networking in
supermin because usermode networking is slow.  Further, as of
OpenShift 4 it's documented how to use `buildah` inside OpenShift,
so let's do that rather than supermin.

Motivated by debugging why the oscontainer push in the RHCOS
pipeline is 2 hours.

Closes: coreos#1500
@cgwalters cgwalters force-pushed the oscontainer-buildah-vfs branch from d031f16 to a2c4f40 Compare June 2, 2020 22:43
@miabbott
Copy link
Copy Markdown
Member

miabbott commented Jun 2, 2020

This is only used by RHCOS right?

So let’s get this in and make our pipeline sane again.

Leaving /lgtm to someone not on mobile 😉

/approve

A while ago, we were using `skopeo inspect` as part of `pivot`
for OpenShift 4 OS upgrades, and it turns out that enumerates
all tags, which is expensive and rate limited by quay.io:
openshift/pivot#51

The RHCOS oscontainer builds have been failing in `skopeo inspect`
and I suspect it's related to this.

It looks like nowadays podman has `--digestfile` which is
exactly what we want here, so let's use it and drop the `skopeo`.
This simplifies the code too.
@cgwalters cgwalters changed the title Remove oscontainer command, drop supermin for upload-oscontainer drop supermin for upload-oscontainer, use digestfile (not skopeo) Jun 3, 2020
@cgwalters
Copy link
Copy Markdown
Member Author

Folded in #1503 for simplicity of merging - while they're logically independent let's get both in at once rather than test intermediate states.

@ashcrow ashcrow requested review from ashcrow and dustymabe and removed request for miabbott and mike-nguyen June 3, 2020 14:03
Copy link
Copy Markdown
Member

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

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

The code changes look good but one question, should cmd-upload-oscontainer be updated since we're dropping cosa oscontainer here?

@cgwalters
Copy link
Copy Markdown
Member Author

The code changes look good but one question, should cmd-upload-oscontainer be updated since we're dropping cosa oscontainer here?

You linked to code in git master, which this PR is changing right?

@ashcrow
Copy link
Copy Markdown
Member

ashcrow commented Jun 3, 2020

The code changes look good but one question, should cmd-upload-oscontainer be updated since we're dropping cosa oscontainer here?

You linked to code in git master, which this PR is changing right?

🤦 yes.

@ashcrow
Copy link
Copy Markdown
Member

ashcrow commented Jun 3, 2020

/lgtm

@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, cgwalters, miabbott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [ashcrow,cgwalters,miabbott]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit b574572 into coreos:master Jun 3, 2020
Sign up for free to 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.

stop using supermin entirely for container builds

5 participants