Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate goreleaser to produce draft releases when new tags are pushed #2365

Merged

Conversation

timflannagan
Copy link
Member

Description of the change:
Integrate goreleaser to the OLM repository for building multi-arch
binaries and manifestlist container images, and help automate the
release process for OLM.

Add a GHA release workflow that's triggered on tags that's responsible
for building and pushing multi-arch (i.e. manifestlist) OLM container
images using goreleaser.

Goreleaser will also create a draft release, and generate a changelog
since the previous tag.

Rendering the release quickstart manifests is done after goreleaser has
created this draft release as there's no easy way to hook this
functionality into goreleaser after the docker images/manifestlists have
been pushed but before release artifacts are generated.

Use the 'softprops/action-gh-release' to update the newly created draft
release with these quickstart manifests as assets.

Update the release target and avoid hardcoding the
quay.io/operator-framework/olm quay repository and instead use the
existing IMAGE_REPO variable defined further up in the Makefile.

Update how the $(ver) variable is handled and avoid hardcoding the 'v'
prefix when overriding that 'ver' variable in the environment.

Motivation for the change:
Alleviate the need for a multi-stage release process.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive

@timflannagan
Copy link
Member Author

This still needs a couple of prerequisites added to the repository.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 17, 2021
@timflannagan timflannagan changed the title Integrate goreleaser [skip-ci] Integrate goreleaser Sep 17, 2021
@timflannagan timflannagan changed the title [skip-ci] Integrate goreleaser [skip ci] Integrate goreleaser Sep 17, 2021
.goreleaser.yml Outdated
Comment on lines 87 to 101
- name_template: quay.io/{{ .Env.IMAGE_REPO }}:v{{ .Major }}.{{ .Minor }}
image_templates:
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-amd64
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-arm64
- name_template: quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}
image_templates:
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-amd64
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-arm64

- name_template: quay.io/{{ .Env.IMAGE_REPO }}:latest
image_templates:
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-amd64
- quay.io/{{ .Env.IMAGE_REPO }}:{{ .Tag }}-arm64
create_flags:
- --amend
Copy link
Member Author

Choose a reason for hiding this comment

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

This produces manifestlist images for v.., v., and latest which seems like the right behavior since OLM isn't v1 and thus a major-only tag seems strange.

Something to note, is that docker images are pushed with amd64/arm64 suffixes using a v.. format (e.g. whatever tag triggered this action) so I'm not sure whether we're comfortable with that format.

Copy link
Member

Choose a reason for hiding this comment

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

We have to be careful with the way latest works here. If we push tag v0.21.0 and then later push tag v0.20.1, latest will point to v0.20.1 when we probably want it to stay on v0.21.0.

operator-registry solves this with some clever envvar value empty string shenanigans.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'm leaning towards omitting the handlement around latest images entirely for the time being and trying to tackle that in a follow-up while we still work out what we'd like to see for tag structure in this repository. WDYT?

- name: Run GoReleaser
uses: goreleaser/goreleaser-action@v2
with:
version: latest
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest pinning this. Its always a bummer when you push a tag and then the release fails to build/push artifacts because the tool version broke your config.

Also, more nitpicky: it can be nice to have this setup as a make target so that it can be executed locally in case something needs to be reproduced or debugged.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I strayed away from housing a Makefile target for this is because I wanted to avoid also setting up qemu-static and buildx actions but I realize that this goreleaser-action likely isn't handling that under-the-covers either, or this ubuntu VM already has all of that configured?

Comment on lines 48 to 56
- name: Update release artifacts with rendered Kubernetes manifests
uses: softprops/action-gh-release@v1
with:
name: ${{ env.IMAGE_TAG }}
files: |
deploy/upstream/quickstart/crds.yaml
deploy/upstream/quickstart/olm.yaml
draft: true
token: ${{ secrets.GORELEASER_GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

GoReleaser has some ability to create GH releases and upload artifacts there. You may be able to move the release manifest generation above (or maybe included in) the goreleaser configuration and then have goreleaser handle creating the release and uploading artifacts.

Side note: only reason we're not doing this in operator-registry is because GoReleaser can't build a macOS binary alongside linux binaries without a cross-compiler running, and Apple doesn't permit that in general.

Copy link
Member Author

@timflannagan timflannagan Sep 17, 2021

Choose a reason for hiding this comment

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

Unfortunately, I've already tried rendering these release manifests before running the goreleaser action, but it dirtied the local environment (which I assume maps to git diff produces output) and failed the release process. I might be able to instead migrate towards generating these manifests, uploading using the archiving action, restore the git environment, and somehow attach these files using the extra_files stanza in the release step, but it's not immediately clear how that'd work if those files are achived and not present locally. Maybe a .gitignore could work?

Another implementation that I played around with trying to global after hooks but that's a pro feature and there wasn't a step between building and pushing the manifestlist images and building out the release.

Copy link
Member Author

@timflannagan timflannagan Sep 17, 2021

Choose a reason for hiding this comment

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

Maybe the cleaner option would be to create a draft release first using the create-release action, then run the goreleaser acition, then upload any post-goreleaser artifacts that need to be attached so it's clear we're creating the release outside of goreleaser (and thus increase readability)? That's assuming goreleaser has support for working with existing draft releases, which playing around in my own fork seems to be a safe assumption.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah perhaps separating them is cleaner in this case. It seems like getting things just so for goreleaser would just add complexity and obfuscation for no real gain.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ended up adding an action that's responsible for creating a draft release and it looks like goreleaser plays well with updating an existing draft release. So the overall workflow would be create an empty draft releaser -> run goreleaser to produce multi-arch container images and manifestlist images, and a generated changelog -> add post-goreleaser releaser artifacts by generating the quickstart manifests and using an action to attach those artifacts to this existing draft release.

The first step obviously isn't needed, but I think it improves the readability of the workflow file quite a bit as goreleaser abstracts quite a bit, and removes the multi-stage release process and removes the need for housing rendered YAML release manifests entirely. This means the new release process is maintainer creates a new tag locally and pushes that upstream -> the release action runs and creates a draft release -> maintainer publishes that draft release.

uses: goreleaser/goreleaser-action@v2
with:
version: latest
args: release --rm-dist
Copy link
Member

Choose a reason for hiding this comment

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

We probably need to add the --snapshot flag anytime we're not pushing images.

FYI - opm also pushes a master image on every push to master for folks that want to live on the edge :) It can be nice to have that, but definitely non-blocking.

Update the release target and avoid hardcoding the
quay.io/operator-framework/olm quay repository and instead use the
existing IMAGE_REPO variable defined further up in the Makefile.

Update how the $(ver) variable is handled and avoid hardcoding the 'v'
prefix when overriding that 'ver' variable in the environment.

Signed-off-by: timflannagan <[email protected]>
@timflannagan timflannagan force-pushed the integrate-goreleaser branch 2 times, most recently from ab3625e to 569a720 Compare September 21, 2021 14:24
@timflannagan timflannagan changed the title [skip ci] Integrate goreleaser Integrate goreleaser to produce draft releases when new tags are pushed Sep 21, 2021
@timflannagan
Copy link
Member Author

I think this largely is read to go.

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 21, 2021
Add a GHA release workflow that's triggered on tags that's responsible
for building and pushing multi-arch (i.e. manifestlist) OLM container
images using goreleaser.

Goreleaser will also create a draft release, and generate a changelog
since the previous tag.

Rendering the release quickstart manifests is done after goreleaser has
created this draft release as there's no easy way to hook this
functionality into goreleaser after the docker images/manifestlists have
been pushed but before release artifacts are generated.

Use the 'softprops/action-gh-release' to update the newly created draft
release with these quickstart manifests as assets.

Signed-off-by: timflannagan <[email protected]>
@kevinrizza
Copy link
Member

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 24, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kevinrizza, timflannagan

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2021
@joelanford
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2021
@timflannagan timflannagan merged commit 41261fc into operator-framework:master Sep 24, 2021
@timflannagan timflannagan deleted the integrate-goreleaser branch September 24, 2021 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants