adopt `buildmaster` and `alpha` tags for oscontainer #308

miabbott · 2018-09-21T15:11:26Z

This pile of commits attempts to implement some of the versioning scheme for the oscontainer discussed in #201.

Firstly, during the treecompose pipeline, the oscontainer is no longer tagged with latest, but with buildmaster. Additionally, it is tagged with the ostree commit that was used to generate the container image.

This ostree commit is passed to the aws-test pipeline after a successful cloud pipeline run. In the aws-test pipeline, the oscontainer is pulled by the ostree commit and if the AWS tests are successful, the oscontainer is tagged with alpha and pushed to the registry.

This is probably not the best way to do this kind of image promotion and will likely be obsoleted by coreos-assembler, but I figured I would still put this out there to see how it was received.

cgwalters · 2018-09-21T15:23:49Z

The primary downside of this is that today, the api.ci cluster will be garbage collecting untagged oscontainers. If we start pushing ostree-commits-as-tags, we'll need to implement some GC policy ourselves.

Which quickly gets into something like this: https://github.com/cgwalters/playground/blob/master/rust/imagestream-ostree-hash/src/imagestream-ostree-hash.rs

I could try to stick that code inside the coreos-assembler container?

cgwalters · 2018-09-21T15:25:49Z

The other big topic here is that if we go to the always-pivot approach from openshift/installer#281

We'd basically stop caring too much about the AMIs, and our AWS test would be "boot and pivot". This would significantly simplify a lot of things.

I mean more generally we would need to be uploading AMIs (we will have to ship fixes to e.g. Ignition) but they can be on a much less frequent basis.

cgwalters · 2018-09-21T15:27:43Z

At a high level...I didn't know you were working on this, I probably would have asked nicely for work to focus on the assembler 😄 But - improving the current pipeline is useful too! We just have to carefully weigh the risk/rewards.

Let's split out the two prep patches to the util API, they look clean and we can merge them now.

miabbott · 2018-09-21T15:35:19Z

The primary downside of this is that today, the api.ci cluster will be garbage collecting untagged oscontainers. If we start pushing ostree-commits-as-tags, we'll need to implement some GC policy ourselves.

I think we could implement this trivially by just using skopeo delete on the images tagged with an ostree commit that didn't pass AWS tests, no?

At a high level...I didn't know you were working on this, I probably would have asked nicely for work to focus on the assembler 😄 But - improving the current pipeline is useful too! We just have to carefully weigh the risk/rewards.

Yeah, I was just tinkering in this area and wanted to measure the reception. I could try to fit this into the assembler, but I guess I'm still not clear on everything the assembler would be responsible for and how to work in pieces like this.

I'm comfortable with sending the majority of this PR to /dev/null

Let's split out the two prep patches to the util API, they look clean and we can merge them now.

Will do.

cgwalters · 2018-09-21T15:50:20Z

I think we could implement this trivially by just using skopeo delete on the images tagged with an ostree commit that didn't pass AWS tests, no?

Yeah that'd help a lot.

Yeah, I was just tinkering in this area and wanted to measure the reception. I could try to fit this into the assembler, but I guess I'm still not clear on everything the assembler would be responsible for and how to work in pieces like this.

Right, that's a big topic. I'm still trying to land the oscontainer basics there along with some work around how the images are sync'd.

miabbott · 2018-09-21T15:52:30Z

Right, that's a big topic. I'm still trying to land the oscontainer basics there along with some work around how the images are sync'd.

So let's hold off on this PR for now, with the expectation that we'll be able to do more intelligent tagging/promotion once coreos-assembler is more mature.

It's not like there is any tooling here or on the OpenShift side that is waiting for a buildmaster or alpha tag.

miabbott · 2018-09-21T18:45:04Z

So let's hold off on this PR for now, with the expectation that we'll be able to do more intelligent tagging/promotion once coreos-assembler is more mature.

I chatted with @cgwalters and we decided to give this PR a shot, but first I need to try to implement the skopeo delete garbage collection mentioned above.

I've split out the utility functions in to a separate PR (#309)

miabbott · 2018-09-21T18:45:16Z

/hold

miabbott · 2018-09-26T20:17:50Z

The commits on this PR are kind of a mess right now because I rebased on top of #312 and force pushed.

However, I think the changes in #312 will simplify some of what I'm doing here, so I want to wait for that to land before trying to get this reviewed/merged again.

miabbott · 2018-09-27T15:01:23Z

/hold cancel

miabbott · 2018-09-27T15:02:03Z

I'm pretty sure this should work; though kind of hard to test since it involves manipulating the production registry.

miabbott · 2018-09-28T13:32:19Z

Rebased ⬆️

cgwalters · 2018-09-28T18:44:25Z

Jenkinsfile.treecompose

+            podman push ${OSCONTAINER_IMG}:buildmaster
+            podman push ${OSCONTAINER_IMG}:${composeMeta.commit}
+            skopeo inspect docker://${OSCONTAINER_IMG}:buildmaster | jq '.Digest' > imgid.txt
+            podman inspect --format='{{.Id}}' ${OSCONTAINER_IMG}:buildmaster > imgid.txt


Duplicate writes to imgid.txt?

Ugh, screwed up my rebase.

cgwalters · 2018-09-28T18:46:42Z

Jenkinsfile.aws-test

@@ -32,6 +35,7 @@ node(NODE) {
            sshUserPrivateKey(credentialsId: params.ARTIFACT_SSH_CREDS_ID, keyFileVariable: 'KEY_FILE'),
        ]) {
            utils.rsync_file_in_dest(ARTIFACT_SERVER, KEY_FILE, "${images}/cloud/latest/meta.json", "${WORKSPACE}/meta.json")
+            ostree_commit = utils.sh_capture("jq -r '.[\"ostree-commit\"]' ${WORKSPACE}/meta.json")


This isn't new (you're building on code right below), but probably we should just use readJSON rather than calling out to jq - Groovy is a real programming language.

Okay, I'll include another commit to use that in this area.

cgwalters · 2018-09-28T18:47:29Z

Jenkinsfile.aws-test

+                    # Tag the container image to alpha, then GC the image tagged with the ostree commit
+                    podman pull ${OSCONTAINER_IMG}:${ostree_commit}
+                    podman tag ${OSCONTAINER_IMG}:${ostree_commit} ${OSCONTAINER_IMG}:alpha
+                    podman push ${OSCONTAINER_IMG}:alpha


We don't actually need to pull locally here, we can just skopeo copy right?

Ooh, fancy!

In an effort to get closer to what has been discussed in openshift#150, the tagging scheme has been changed so that the latest container image out of the pipeline is tagged with `buildmaster` and the commit ID. The usage of the commit ID allows for other parts of the piepline to easily refer to it for other operations.

In openshift#150, it was discussed that an `alpha` tag should be made for the oscontainer (and cloud image) after it passes the tests run in AWS. This change accomplishes this goal by pulling the oscontainer by commit ID, tagging it as `alpha` and pushing it to the registry. (After a successful test, obviously) If the AWS tests fail, the image tagged with the ostree commit is garbage collected and no alpha promotion happens.

To bootstrap the change to `buildmaster`, we need to do a quick test if the tag exists on the registry. If it exists, we can pull from it, otherwise we need to use `latest`

miabbott · 2018-09-28T21:54:32Z

New commits incorporating feedback from @cgwalters ⬆️

cgwalters · 2018-10-01T15:29:09Z

/lgtm

openshift-ci-robot requested review from cgwalters and sdemos September 21, 2018 15:11

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 21, 2018

miabbott mentioned this pull request Sep 21, 2018

pipeline-utils: add registry_login() and prep_container_storage() #309

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 Sep 21, 2018

miabbott force-pushed the version_tagging branch 4 times, most recently from 0b34a81 to 120d657 Compare September 26, 2018 20:16

miabbott force-pushed the version_tagging branch from 120d657 to 39de164 Compare September 27, 2018 15:00

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2018

miabbott force-pushed the version_tagging branch from 2ea99a6 to 5879c43 Compare September 28, 2018 13:31

cgwalters reviewed Sep 28, 2018

View reviewed changes

miabbott added 4 commits September 28, 2018 17:51

treecompose: detect if buildmaster exists on registry

ede4d52

To bootstrap the change to `buildmaster`, we need to do a quick test if the tag exists on the registry. If it exists, we can pull from it, otherwise we need to use `latest`

aws-test: use readJSON to extract values from JSON files

e4f784a

miabbott force-pushed the version_tagging branch from 5879c43 to e4f784a Compare September 28, 2018 21:53

openshift-ci-robot assigned cgwalters Oct 1, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2018

openshift-merge-robot merged commit 33efc9b into openshift:master Oct 1, 2018

wking mentioned this pull request Oct 5, 2018

Libvirt release channels? #354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adopt `buildmaster` and `alpha` tags for oscontainer #308

adopt `buildmaster` and `alpha` tags for oscontainer #308

miabbott commented Sep 21, 2018

cgwalters commented Sep 21, 2018

cgwalters commented Sep 21, 2018

cgwalters commented Sep 21, 2018

miabbott commented Sep 21, 2018

cgwalters commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 26, 2018

miabbott commented Sep 27, 2018

miabbott commented Sep 27, 2018

miabbott commented Sep 28, 2018

cgwalters Sep 28, 2018

miabbott Sep 28, 2018

cgwalters Sep 28, 2018

miabbott Sep 28, 2018

cgwalters Sep 28, 2018

miabbott Sep 28, 2018

miabbott commented Sep 28, 2018

cgwalters commented Oct 1, 2018

adopt buildmaster and alpha tags for oscontainer #308

adopt buildmaster and alpha tags for oscontainer #308

Conversation

miabbott commented Sep 21, 2018

cgwalters commented Sep 21, 2018

cgwalters commented Sep 21, 2018

cgwalters commented Sep 21, 2018

miabbott commented Sep 21, 2018

cgwalters commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 21, 2018

miabbott commented Sep 26, 2018

miabbott commented Sep 27, 2018

miabbott commented Sep 27, 2018

miabbott commented Sep 28, 2018

cgwalters Sep 28, 2018

Choose a reason for hiding this comment

miabbott Sep 28, 2018

Choose a reason for hiding this comment

cgwalters Sep 28, 2018

Choose a reason for hiding this comment

miabbott Sep 28, 2018

Choose a reason for hiding this comment

cgwalters Sep 28, 2018

Choose a reason for hiding this comment

miabbott Sep 28, 2018

Choose a reason for hiding this comment

miabbott commented Sep 28, 2018

cgwalters commented Oct 1, 2018

adopt `buildmaster` and `alpha` tags for oscontainer #308

adopt `buildmaster` and `alpha` tags for oscontainer #308