Skip to content

upload-oscontainer: Support oscontainer.yaml and extensions.yaml#1916

Merged
openshift-merge-robot merged 1 commit intocoreos:masterfrom
cgwalters:extensions
Dec 3, 2020
Merged

upload-oscontainer: Support oscontainer.yaml and extensions.yaml#1916
openshift-merge-robot merged 1 commit intocoreos:masterfrom
cgwalters:extensions

Conversation

@cgwalters
Copy link
Copy Markdown
Member

@cgwalters cgwalters commented Nov 30, 2020

Pairs with openshift/os#455

Add an oscontainer.yaml which allows configuring (currently) just
the FROM line equivalent. This helps keep things declarative
in the config instead of part of the pipeline.

And since extensions are now a core part of OpenShift 4 (particularly RT kernel)
which also uses the oscontainer, let's lift the logic to generate
those bits of the oscontainer into this repo and out of the config git.

Both of these are part of the general philosophy we've had that:

  • config git is declarative config
  • coreos-assembler is the mechanism
  • a pipeline just scripts coreos-assembler in a way that's
    easy to reproduce with plain podman too

Comment thread src/download-extensions
Copy link
Copy Markdown
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

One comment, LGTM otherwise! Also had some feedback in openshift/os#455 which could affect the implementation here.

Comment thread src/download-extensions Outdated
Pairs with openshift/os#455

Add an `oscontainer.yaml` which allows configuring (currently) just
the `FROM` line equivalent.  This helps keep things declarative
in the config instead of part of the pipeline.

And since extensions are now a core part of OpenShift 4 (particularly RT kernel)
which also uses the oscontainer, let's lift the logic to generate
those bits of the oscontainer into this repo and out of the config git.

Both of these are part of the general philosophy we've had that:

 - config git is declarative config
 - coreos-assembler is the mechanism
 - a pipeline just scripts coreos-assembler in a way that's
   easy to reproduce with plain podman too
@cgwalters cgwalters changed the title upload-oscontainer: Support extensions.yaml in config dir automatically upload-oscontainer: Support oscontainer.yaml and extensions.yaml Dec 2, 2020
@cgwalters
Copy link
Copy Markdown
Member Author

OK reworked this to add an oscontainer.yaml to handle the FROM bit too, so it's really just cosa upload-oscontainer --name=registry.svc.ci.openshift.org/cgwalters/test-rpmostree-4.7-os. (And making the target declarative...a lot trickier)

Comment thread src/cmd-upload-oscontainer
Comment thread src/download-extensions
# 1 = extension name
# 2 = package string/glob
# 3 = OPTIONAL: dependencies string/glob
def createext(extname, pkgs):
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.

The previous script this is intending to replace used to link deps at the top-level for backcompat. Is it safe to drop that behaviour now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure yes; the code which was doing regexp.MustCompile("kernel-rt*") to find packages went away as part of unifying that code with extensions in openshift/machine-config-operator@6757381 which is in 4.6.

(But, I didn't actually test this in a cluster yet)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok tested now and works!

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Dec 3, 2020

/lgtm

@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

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:

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 84d2f6a into coreos:master Dec 3, 2020
cgwalters added a commit to cgwalters/os that referenced this pull request Dec 3, 2020
There's some logic I didn't currently preserve from this
in the move into coreos-assembler around kernel/kernel-headers
matching.
coreos/coreos-assembler#1916

I will look at re-adding that.  But short term we don't
actually have that problem right now and I'd like to
get the 4.7 pipeline switched over to
the new model so we don't have duplication.

The goal here is to make it very "cosa like" to upload the
oscontainer because it needs to be trivial for other people
to do outside of a pipeline.

Because this is git, when we need some of this code again we
can find it in the history.
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.

5 participants