WIP: add --coreos-url to `oc adm release new` #21998

cgwalters · 2019-02-08T20:37:56Z

For RHCOS we have two things:

The "bootimage" (AMI, qcow2, PXE env)
The "oscontainer", now represented as machine-os-content in the payload

For initial OpenShift releases (e.g. of the installer) ideally
these are the same (i.e. we don't upgrade OS on boot).

This PR aims to support injecting both data into the release payload.

More information on the "bootimage" and its consumption by the
installer as well as the Machine API Operator:
openshift/installer#987

More information on machine-os-content:
openshift/machine-config-operator#183

openshift-ci-robot · 2019-02-08T20:41:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cgwalters
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: enj

If they are not already assigned, you can assign the PR to them by writing /assign @enj in a comment when ready.

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:

pkg/oc/OWNERS

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

smarterclayton · 2019-02-08T20:46:09Z

As an alternative point - can we include the AMI info as content in the machine-os-content container when we build it? If we put that data in /manifests and that's a build side thing, we don't need to change it here.

I.e. have machine-os-content build lay down:

/manifests/0000_30_machine-os-content_configmap.yaml
kind: ConfigMap
...
data:
  metadata.json: |
      contents_of

? That's slightly increase in complexity in build time of os container. Or alternatively, embedded as label metadata.

I really don't like the external calls here and in the installer in general because it means we have two types of content, not one.

As a clarifying question, when would we change metadata.json but not the content we are shipping?

cgwalters · 2019-02-08T20:54:41Z

can we include the AMI info as content in the machine-os-content container when we build it?

The current RHCOS build system has some really funky dependencies between building the oscontainer and the bootimage. Because: the bootimage needs to know the oscontainer it came from - if you boot and run e.g.:

[root@osiris-master-0 ~]# rpm-ostree status
State: idle
AutomaticUpdates: disabled
Deployments:
● pivot://registry.svc.ci.openshift.org/rhcos/maipo@sha256:2b7ff5707a67234e6d3187a6b30f5382aa74cd98327228ec6a463dcfe8e02908
              CustomOrigin: Provisioned from oscontainer
                   Version: 47.307 (2019-02-07T16:07:32Z)

That's how the MCD knows whether or not it needs to update.

Another further wrinkle here is that we want to actually test the OS before we upload an oscontainer - which requires a bootimage (VM image).

So the build system today goes:

RPMs ➡️ ostree
ostree ➡️ bootimage
Test bootimage (kola sanity checks; no failing systemd units etc.)
ostree ➡️ oscontainer
push oscontainer to api.ci and get final digest (because docker images, compression...)
"scripts/prepivot" which injects the pull spec into the bootimage
Upload to AWS
Upload final build to S3

Now...I am not sure offhand how we could easily inject the AMIs into our oscontainer.

It might be easiest to have this be a separate container?

cgwalters · 2019-02-08T21:04:07Z

I really don't like the external calls here and in the installer in general because it means we have two types of content, not one.

Yep agreed - really the goal of this PR is to enable the magic to move from the installer to the release payload. If this lands and the installer implemented openshift/installer#987 then it could drop the code talking to the API - the source of truth for "what the installer will install" becomes just the release payload.

But bigger picture I don't think we're going to get away from the two fundamental types of content (bootimages/VM images, containers). A lot of work was put into the RHCOS build system to have a unified build scheme that encapsulates both - which is required if we want to have the "don't pivot on initial install" model.

In the end certainly from the user point of view we're streamlining this - they shouldn't have to care about how it's implemented. The coreos-assembler build scheme/metadata is fairly hidden here and we could definitely change how it works later.

Well...and we almost certainly will change it. It does seem likely at this point that we'll end up encapsulating a bootimage inside a container too. That isn't designed today though.

As a clarifying question, when would we change metadata.json but not the content we are shipping?

I assume here metadata.json == RHCOS build? I can't think of a case where we would change only that. But the goal here is to get the data in the release payload.

jlebon · 2019-02-08T21:05:26Z

"scripts/prepivot" which injects the pull spec into the bootimage

It's not ideal, but if breaking the oscontainer <-> bootimage coupling makes things much easier, we could pretty easily teach pivot/MCD to also consider the OSTree checksum rather than just the pullspec. Then we can add whatever metadata we want to the oscontainer about the built images.

cgwalters · 2019-02-08T21:06:52Z

pretty easily teach pivot/MCD to also consider the OSTree checksum rather than just the pullspec.

Yeah...or another idea here is to derive a layer from the oscontainer with the build metadata. Both would end up in the release payload but the "buildmeta" layer would be tiny.

EDIT: But the thing is, doing that will require nontrivial changes in the installer and I don't want to block on that. At least, not unless the installer folks think it will be easy to switch to extracting the AMI data from the release payload.

cgwalters · 2019-02-08T21:28:57Z

As a clarifying question, when would we change metadata.json but not the content we are shipping?

Actually, a possible scenario for this is "add EC2 region to existing release". But...we can probably ignore that for now.

Anyways we have a few choices. We could skip trying to get the bootimage data into the release payload (i.e. installer code stays unchanged), and simply add a cron job that pulls the latest rhcos oscontainer and pushes it into the machine-os-content IS which would unblock me for landing work to do OS updates in openshift/machine-config-operator#363

When the installer folks go to do a release they'd need to manually backreference from the machine-os-content to get the bootimage data (which BTW we should hardcode in the installer rather than being an API fetch). We could also in parallel try to implement machine-os-bootimage-meta or so which would just have metadata for now (all we need for AWS).

However, this would mean that in CI both would continue to float independently - leading to a situation where sometimes in CI one would boot a slightly older release and we'd upgrade during a run. The reason I was going for this model is it makes crystal clear that the source of truth is the release payload (which just happens to gather bootimage data using an external API at the moment at build time).

cgwalters · 2019-02-09T19:04:30Z

OK fixed various bugs; main thing is we now piggy back on o.Mappings which avoids touching a lot of code. This works for me:

$ ./_output/local/bin/linux/amd64/oc adm release new --coreos-url https://releases-rhcos.svc.ci.openshift.org/storage/releases/maipo --from-image-stream=origin-v4.0 -n openshift --to-image quay.io/cgwalters/oste
st:release
info: Found 111 images in image stream                                                                                                                                                                            
Using CoreOS build 47.310   
...
$ podman pull quay.io/cgwalters/ostest:release   # need a way to build to local image store
$ podman inspect quay.io/cgwalters/ostest:release | grep coreos | head -1
                "io.openshift.release.coreos-boot-image": "{\"amis\":[{\"hvm\":\"ami-02d655625b6c691e9\",\"name\":\"ap-northeast-1\"},{\"hvm\":\"ami-08f5ccc1a8b0f1fd4\",\"name\":\"ap-northeast-2\"},{\"hvm\":\"am
i-0147d72611333957b\",\"name\":\"ap-south-1\"},{\"hvm\":\"ami-089509f5194db0e3f\",\"name\":\"ap-southeast-1\"},{\"hvm\":\"ami-0737f6cd734a7d222\",\"name\":\"ap-southeast-2\"},{\"hvm\":\"ami-0b719049951b1fd21\",\
"name\":\"ca-central-1\"},{\"hvm\":\"ami-076f809a745f83d98\",\"name\":\"eu-central-1\"},{\"hvm\":\"ami-08d87f63698b5fd3d\",\"name\":\"eu-west-1\"},{\"hvm\":\"ami-0698a3aa0e4b52c8f\",\"name\":\"eu-west-2\"},{\"hv
m\":\"ami-04b893b85e5defb6d\",\"name\":\"eu-west-3\"},{\"hvm\":\"ami-021c3815a9866feb2\",\"name\":\"sa-east-1\"},{\"hvm\":\"ami-0fe81b1f471177095\",\"name\":\"us-east-1\"},{\"hvm\":\"ami-0013fbee46f77ca37\",\"na
me\":\"us-east-2\"},{\"hvm\":\"ami-06681b6266a5ded7c\",\"name\":\"us-west-1\"},{\"hvm\":\"ami-0e7126056bc16b40f\",\"name\":\"us-west-2\"}],\"buildid\":\"47.310\",\"images\":{\"qemu\":{\"path\":\"redhat-coreos-ma
ipo-47.310-qemu.qcow2.gz\",\"sha256\":\"0a6e060043713bd23617db3422e0863744f4163627eb8ff1237fc143d3eca6a5\"}},\"ostree-version\":\"47.310\",\"oscontainer\":{\"digest\":\"sha256:def5c8ef775f634d99e3f603da0db7591a0
113ad8307bc8d8545a170787a2825\",\"image\":\"registry.svc.ci.openshift.org/rhcos/maipo\"}}"           
$ podman run --rm -ti quay.io/cgwalters/ostest:release image machine-os-content
registry.svc.ci.openshift.org/rhcos/maipo@sha256:def5c8ef775f634d99e3f603da0db7591a0113ad8307bc8d8545a170787a2825

And notably machine-os-content is the oscontainer corresponding to 47.310.

For RHCOS we have two things: - The "bootimage" (AMI, qcow2, PXE env) - The "oscontainer", now represented as `machine-os-content` in the payload For initial OpenShift releases (e.g. of the installer) ideally these are the same (i.e. we don't upgrade OS on boot). This PR aims to support injecting both data into the release payload. More information on the "bootimage" and its consumption by the installer as well as the Machine API Operator: openshift/installer#987 More information on `machine-os-content`: openshift/machine-config-operator#183

cgwalters · 2019-02-11T19:35:20Z

Let me summarize. This PR is attempting to move the "RHCOS API consumption" from the installer to the release payload. It also helps ensure that the bootimage is the same as the oscontainer in the release payload which is something we want to support long term. I am not aware of a different approach to solve that.

If we want to wait on this we could just do manual updates of machine-os-content for now.

jlebon · 2019-02-11T19:20:05Z

pkg/oc/cli/admin/release/coreos.go

+	return builds.Builds[0], nil
+}
+
+// GetLatest returns the CoreOS build with target version.  If version


GetCoreOSBuild

jlebon · 2019-02-11T19:29:23Z

pkg/oc/cli/admin/release/coreos.go

+		Name string `json:"name"`
+	} `json:"amis"`
+	BuildID string `json:"buildid"`
+	Images struct {


I'd just drop this and include only the keys we need for now. Not that we're planning to break the schema, though there's no point in deserializing it if we're not using it.

Speaking of, I think we discussed adding schema versioning to the JSON output. Probably would be good to do this before teaching the origin about it?

jlebon · 2019-02-11T19:30:20Z

pkg/oc/cli/admin/release/new.go

+		fmt.Fprintf(o.Out, "Using CoreOS build %s\n", coreosBuild.BuildID)
+		digestedImage := fmt.Sprintf("%s@%s", coreosBuild.OSContainer.Image, coreosBuild.OSContainer.Digest)
+		if digestedImage == "@" {
+			return fmt.Errorf("No oscontainer in CoreOS build")


Why not do an explicit e.g. if coreosBuild.OSContainer.Image == "" || coreosBuild.OSContainer.Digest == "" { ... }? (This also catches the case where just one of the two is somehow missing.)

jlebon · 2019-02-11T19:56:11Z

pkg/oc/cli/admin/release/new.go

+			return err
+		}
+		// This is written as a label in the final image
+		is.Annotations[coreOSBootImageLabel] = string(serializedBuild)


Ahh OK, I see now why you wanted to also deserialize the qcow2s. Though... isn't there a way to have this data be a proper configmap instead of JSON-in-a-label? We were discussing this in openshift/machine-config-operator#183 right? (And including the pkglist, etc...).

Anyway, totally fine doing it this way for now too!

cgwalters · 2019-02-11T20:02:54Z

Clayton and I had a quick call on this. We agreed to for now just push a :maipo tag and get wire that up in CI to get into the payload as machine-os-content.

ashcrow · 2019-02-12T15:13:45Z

/retest

ashcrow · 2019-02-12T16:19:26Z

Looks like the new switches must be added to completions for ci/prow/verify

cgwalters · 2019-02-12T16:22:28Z

/hold

We are deferring this for now.

openshift-ci-robot · 2019-02-15T15:34:16Z

@cgwalters: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/verify	`38fc2fe`	link	`/test verify`
ci/prow/integration	`38fc2fe`	link	`/test integration`
ci/prow/e2e-aws	`38fc2fe`	link	`/test e2e-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2019-02-18T10:04:06Z

@cgwalters: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

abhinavdahiya · 2019-03-08T20:22:11Z

can we include the AMI info as content in the machine-os-content container when we build it?

The current RHCOS build system has some really funky dependencies between building the oscontainer and the bootimage. Because: the bootimage needs to know the oscontainer it came from - if you boot and run e.g.:
[root@osiris-master-0 ~]# rpm-ostree status
State: idle
AutomaticUpdates: disabled
Deployments:
● pivot://registry.svc.ci.openshift.org/rhcos/maipo@sha256:2b7ff5707a67234e6d3187a6b30f5382aa74cd98327228ec6a463dcfe8e02908
              CustomOrigin: Provisioned from oscontainer
                   Version: 47.307 (2019-02-07T16:07:32Z)
That's how the MCD knows whether or not it needs to update.

Another further wrinkle here is that we want to actually test the OS before we upload an oscontainer - which requires a bootimage (VM image).

So the build system today goes:
* RPMs arrow_right ostree

* ostree arrow_right bootimage

* Test bootimage (kola sanity checks; no failing systemd units etc.)

* ostree arrow_right oscontainer

* _push_ oscontainer to api.ci and get final digest (because docker images, compression...)

* "scripts/prepivot" which injects the pull spec into the bootimage

* Upload to AWS

* Upload final build to S3
Now...I am not sure offhand how we could easily inject the AMIs into our oscontainer.

It might be easiest to have this be a separate container?

what if we use machine-os-content as easy way for MC* to deal with ostree image.
and machine-os-config to store the ConfigMap that @smarterclayton suggested.

so the RHCOS build pipeline pushed 2 images and the release image creation process remains fairly same.

keeping the metadata.json in a configmap gives operators like machine-api-operator easy way to sync metadata.json contents as a consumer.

cgwalters · 2019-03-08T20:33:19Z

and machine-os-config to store the ConfigMap that @smarterclayton suggested. So the RHCOS build pipeline pushed 2 images and the release image creation process remains fairly same.

Yep, I think that should be fairly straightforward. An interesting wrinkle here though is that I think while we want to support drift, we'd also like the ability to "atomically" tag both of them together into the release payload. But eh, we can just deal with that in the release controller?

openshift-bot · 2019-06-07T01:40:21Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2019-07-07T03:11:56Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2019-08-06T04:45:08Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci-robot · 2019-08-06T04:45:11Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cgwalters · 2019-08-29T19:14:13Z

Been thinking about this again since bootimage updates would greatly help MCO kernel args.

We really have total flexibility in how we implement this. So here's a totally different proposal:
Create a new Prow job in api.ci which polls the RHCOS build endpoint for updated bootimages (one job per branch/stream), and generates a container which has a manifest that installs a configmap that has the meta.json. This would be machine-bootimage-metadata or something.

(And from there, either the MCO could update the machinesets, or the cluster API could read it directly, whichever)

Opinions?

cgwalters · 2019-08-29T19:18:59Z

Or perhaps to start, we don't poll the RHCOS build endpoint, we just inject the installer-pinned bootimage. Yeah...that way we avoid having to think about two separate bootimage versions per version.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 8, 2019

openshift-ci-robot requested review from enj and soltysh February 8, 2019 20:41

cgwalters force-pushed the release-new-rhcos branch from 3059a34 to 166036e Compare February 9, 2019 19:02

cgwalters force-pushed the release-new-rhcos branch from 166036e to 38fc2fe Compare February 9, 2019 19:05

This was referenced Feb 9, 2019

Embed RHCOS buildID in update payload, fetch it for installer openshift/installer#987

Closed

Design for osImageURL updates - integration with CVO/release payload openshift/machine-config-operator#183

Closed

jlebon reviewed Feb 11, 2019

View reviewed changes

cgwalters mentioned this pull request Feb 12, 2019

[RFE] add manifest subcommand containers/podman#713

Closed

cgwalters mentioned this pull request Feb 12, 2019

image-references: Update maipo oscontainer openshift/machine-config-operator#404

Merged

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2019

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 18, 2019

cgwalters mentioned this pull request Feb 18, 2019

server: Implement "early pivot" before the MCD comes up openshift/machine-config-operator#426

Merged

wking mentioned this pull request Feb 21, 2019

pkg/rhcos/release: Extract RHCOS build from release image openshift/installer#1286

Closed

wking mentioned this pull request Mar 11, 2019

Add one of the new style upgrade tests to ci-operator openshift/release#3030

Merged

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 7, 2019

openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 7, 2019

cgwalters mentioned this pull request Jul 8, 2019

Updating bootimages openshift/os#381

Closed

openshift-ci-robot closed this Aug 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: add --coreos-url to `oc adm release new` #21998

WIP: add --coreos-url to `oc adm release new` #21998

cgwalters commented Feb 8, 2019

openshift-ci-robot commented Feb 8, 2019

smarterclayton commented Feb 8, 2019

cgwalters commented Feb 8, 2019

cgwalters commented Feb 8, 2019

jlebon commented Feb 8, 2019

cgwalters commented Feb 8, 2019 •

edited

Loading

cgwalters commented Feb 8, 2019 •

edited

Loading

cgwalters commented Feb 9, 2019

cgwalters commented Feb 11, 2019

jlebon Feb 11, 2019

jlebon Feb 11, 2019

jlebon Feb 11, 2019

jlebon Feb 11, 2019

cgwalters commented Feb 11, 2019

ashcrow commented Feb 12, 2019

ashcrow commented Feb 12, 2019

cgwalters commented Feb 12, 2019

openshift-ci-robot commented Feb 15, 2019

openshift-ci-robot commented Feb 18, 2019

abhinavdahiya commented Mar 8, 2019

cgwalters commented Mar 8, 2019

openshift-bot commented Jun 7, 2019

openshift-bot commented Jul 7, 2019

openshift-bot commented Aug 6, 2019

openshift-ci-robot commented Aug 6, 2019

cgwalters commented Aug 29, 2019 •

edited

Loading

cgwalters commented Aug 29, 2019

WIP: add --coreos-url to oc adm release new #21998

WIP: add --coreos-url to oc adm release new #21998

Conversation

cgwalters commented Feb 8, 2019

openshift-ci-robot commented Feb 8, 2019

smarterclayton commented Feb 8, 2019

cgwalters commented Feb 8, 2019

cgwalters commented Feb 8, 2019

jlebon commented Feb 8, 2019

cgwalters commented Feb 8, 2019 • edited Loading

cgwalters commented Feb 8, 2019 • edited Loading

cgwalters commented Feb 9, 2019

cgwalters commented Feb 11, 2019

jlebon Feb 11, 2019

Choose a reason for hiding this comment

jlebon Feb 11, 2019

Choose a reason for hiding this comment

jlebon Feb 11, 2019

Choose a reason for hiding this comment

jlebon Feb 11, 2019

Choose a reason for hiding this comment

cgwalters commented Feb 11, 2019

ashcrow commented Feb 12, 2019

ashcrow commented Feb 12, 2019

cgwalters commented Feb 12, 2019

openshift-ci-robot commented Feb 15, 2019

openshift-ci-robot commented Feb 18, 2019

abhinavdahiya commented Mar 8, 2019

cgwalters commented Mar 8, 2019

openshift-bot commented Jun 7, 2019

openshift-bot commented Jul 7, 2019

openshift-bot commented Aug 6, 2019

openshift-ci-robot commented Aug 6, 2019

cgwalters commented Aug 29, 2019 • edited Loading

cgwalters commented Aug 29, 2019

WIP: add --coreos-url to `oc adm release new` #21998

WIP: add --coreos-url to `oc adm release new` #21998

cgwalters commented Feb 8, 2019 •

edited

Loading

cgwalters commented Feb 8, 2019 •

edited

Loading

cgwalters commented Aug 29, 2019 •

edited

Loading