Skip to content

*: automate releases with goreleaser #4034

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

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

estroz
Copy link
Member

@estroz estroz commented Oct 14, 2020

Description of the change:

  • .goreleaser.yml: goreleaser config file
  • .ci/gpg: GPG public key, encrypted private subkey (used for signing), and script to decrypt private subkey in CI
  • website/scripts: scripts to update versions in release commits and branches at build time
  • website/content/en/docs: release and installation guide simplifications, related docs cleanup
  • *: script and Makefile updates to support automated releases

Motivation for the change: This PR adds goreleaser and tooling for automated releases in CI, agnostic of CI platform (albeit with some environment configuration). Any Github user with write capabilities to the SDK repo can start a release by doing git tag vX.Y.Z && git push --tags.

Closes #4114

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2020
@estroz estroz force-pushed the feature/goreleaser branch from 461b978 to 69c5e53 Compare October 14, 2020 20:03
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2020
@estroz estroz force-pushed the feature/goreleaser branch from 69c5e53 to 17e7412 Compare October 19, 2020 17:50
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2020
release/Makefile Outdated

.PHONY: changelog
changelog: check_tag ## Generate the changelog.
mkdir -p changelog/generated
Copy link
Member Author

Choose a reason for hiding this comment

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

@joelanford goreleaser needs a file containing release notes if we're going to use a custom changelog generator, so the idea is to create a new changelog file per release under changelog/generated, much like we do with migration guides. This file will be committed in the release commit prior to running goreleaser, much as we do with CHANGELOG.md now.

Copy link
Member

Choose a reason for hiding this comment

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

This file will be committed in the release commit prior to running goreleaser, much as we do with CHANGELOG.md now.

Sounds good while we have the need for a pre-release commit.

Thinking more in the future, if we're able to get rid of the pre-release commit would it be possible to generate it on the fly during the tag CI run just before running goreleaser? It might take some non-trivial changes to our current changelog fragment and generation process, but I think that would be on the table if we could get to the point of not needing pre-release commits.

Copy link
Member Author

Choose a reason for hiding this comment

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

if we're able to get rid of the pre-release commit would it be possible to generate it on the fly during the tag CI run just before running goreleaser

We could get rid of committing the changelog entirely, if that's desirable, right now (I think it is since we publish release notes).

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 take ^ back since we still need to remove fragments in the pre-release commit.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, maybe let's make a separate issue to investigate options for changelog/release notes generation without a pre-release commit.


# We don't use archives, so skip creating them.
archives:
- format: binary
Copy link
Member

Choose a reason for hiding this comment

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

I think we may need the following to make the binaries have the same naming scheme we've been using:

Suggested change
- format: binary
- format: binary
replacements:
amd64: x86_64
darwin: apple-darwin
linux: linux-gnu
arm64: aarch64
name_template: '{{.Binary}}-{{.Env.GIT_VERSION}}-{{.Arch}}-{{.Os}}'

Copy link
Member

Choose a reason for hiding this comment

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

One caveat: I noticed that our arm64 binary release on github use aarch64 and our images in quay use arm64. They have the same meaning, just a different name. But I think at some point we might want to align those if that's possible as a non-breaking change.

Copy link
Member Author

@estroz estroz Oct 21, 2020

Choose a reason for hiding this comment

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

Is it a breaking change to modify the released binary names? If not I'd prefer to use those defined by Golang going forward.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also removing version from binary names removes one more doc we have to change in the release commit (there are other ways around the latter issue, but this is the simplest).

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay using the Golang arch and os for the binary names.

@estroz estroz force-pushed the feature/goreleaser branch from ded4754 to b11186c Compare October 21, 2020 00:38
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2020
@estroz estroz force-pushed the feature/goreleaser branch from 1f7afbb to 9fb0e22 Compare October 22, 2020 02:28
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2020
@estroz estroz force-pushed the feature/goreleaser branch 3 times, most recently from 3cf5db6 to 337ef12 Compare October 22, 2020 03:02
@estroz
Copy link
Member Author

estroz commented Oct 22, 2020

I need to test a full release on my operator-sdk fork before merging this, and add a PR description.

netlify.toml Outdated
command = """
git submodule update -f --init themes/docsy && \
npm install postcss-cli autoprefixer@^9.0.0 && \
./scripts/set_version.sh && \
Copy link
Member Author

Choose a reason for hiding this comment

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

@asmacdo this script updates version_menu = "Releases" to version_menu = "vX.Y" at build time if the branch matches vX.Y.x. This should render the updated menu as it currently is now, but without us having to commit this change, right?

@estroz estroz force-pushed the feature/goreleaser branch 3 times, most recently from 15d249d to f94d600 Compare October 30, 2020 20:55
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2020
@estroz estroz force-pushed the feature/goreleaser branch from f94d600 to 33ece13 Compare October 30, 2020 20:58
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2020
@estroz estroz force-pushed the feature/goreleaser branch 2 times, most recently from 85d5cbc to 1280207 Compare October 31, 2020 00:45
@estroz estroz force-pushed the feature/goreleaser branch from 1280207 to 9e4d2dd Compare October 31, 2020 19:14
@estroz
Copy link
Member Author

estroz commented Nov 3, 2020

Successful release: https://travis-ci.org/github/estroz/operator-sdk/builds/741229073
Published release from job (using my Github key): https://github.com/estroz/operator-sdk/releases/tag/v0.33.0

@estroz estroz force-pushed the feature/goreleaser branch from 9e4d2dd to 423f6f7 Compare November 3, 2020 21:51
@estroz
Copy link
Member Author

estroz commented Nov 3, 2020

.travis.yml Outdated
@@ -3,7 +3,7 @@ dist: xenial

language: go
go:
- 1.15.x
- 1.15.2
Copy link
Contributor

Choose a reason for hiding this comment

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

why make it fixed to 1.15.2?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reproducible builds, see #4034 (comment). I am reconsidering this decision though.

Copy link
Member

Choose a reason for hiding this comment

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

For doing builds, I think pinning makes sense. CI should use the latest one so that if a specific one causes a build failure we catch it in the PR before we do a release. I would hate to see a release fail because of an updated go compiler (though I suspect it would be rare, not something I'd want to debug when it happens).

Copy link
Member

Choose a reason for hiding this comment

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

tl;dr +1 to pinning

Comment on lines +50 to +53
- name: check
if: type == pull_request
- name: test
if: type == pull_request
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we no longer run the check/test on master?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. We already run PR tests, so testing master post-merge is redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

I only think that we need to run the test/check when we push the release tag. I mean to generate the tag. Otherwise, +1 for me. However, in the latest release meeting, this idea was not accepted. @joelanford @jmrodri are you ok with as well

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with not running them on master anymore.

Comment on lines 38 to 41
[[ -z "$ver" ]] || (echo "cannot set goreleaser version" && exit 1)
ver="${DEST}/goreleaser"
ver_cmd="command -v ${DEST}/goreleaser"
fetch_cmd="curl -sfL https://install.goreleaser.com/github.com/goreleaser/goreleaser.sh | sh -s -- -b ${DEST}"
Copy link
Member

@joelanford joelanford Nov 5, 2020

Choose a reason for hiding this comment

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

The goreleaser install script allows specifying a tag:

Suggested change
[[ -z "$ver" ]] || (echo "cannot set goreleaser version" && exit 1)
ver="${DEST}/goreleaser"
ver_cmd="command -v ${DEST}/goreleaser"
fetch_cmd="curl -sfL https://install.goreleaser.com/github.com/goreleaser/goreleaser.sh | sh -s -- -b ${DEST}"
ver_cmd="${DEST}/goreleaser --version 2>/dev/null | grep version | cut -d' ' -f3"
fetch_cmd="curl -sSfL https://install.goreleaser.com/github.com/goreleaser/goreleaser.sh | sh -s -- -b \"${DEST}\" -d \"v${ver}\""

```sh
$ make cli-doc
```

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

The configuration for goreleaser and travis seem sane. Pinning golang makes sense. I like the GPG stuff, kudos for that. The Makefile looks good too.

My biggest request come with the release document. I think we should put the instructions directly in each step and not link to other parts of the doc which makes it hard to keep track of what step you're own. And I was wondering if we could add the git add step of the prerelease to the makefile which is one less command the release person has to run.

Comment on lines +19 to +28
echo -e "\nDecrypting secret key..."
{
# $GPG_PASSWORD is taken from the script's env (injected by Travis CI).
echo $GPG_PASSWORD | gpg --decrypt \
--pinentry-mode loopback --batch \
--passphrase-fd 0 \
--output "${SECRING_AUTO}" \
"${SECRING_AUTO}".gpg ; \
} || { err_exit "Failed to decrypt secret key." ; }
echo "Success!"
Copy link
Member

Choose a reason for hiding this comment

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

Exactly what we used to do in RHN back in the day when we stored our encrypted keys in CVS. I approve.

Comment on lines +43 to +46
asmflags: *build-asmflags
gcflags: *build-gcflags
ldflags: *build-ldflags
targets: *build-targets
Copy link
Member

Choose a reason for hiding this comment

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

I like this reuse of the flags and the fact that if we ever need to change one just for one of the builds, we still have the flexibility to update the flags in this section.


# We don't use archives, so skip creating them.
archives:
- format: binary
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay using the Golang arch and os for the binary names.

.travis.yml Outdated
@@ -3,7 +3,7 @@ dist: xenial

language: go
go:
- 1.15.x
- 1.15.2
Copy link
Member

Choose a reason for hiding this comment

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

For doing builds, I think pinning makes sense. CI should use the latest one so that if a specific one causes a build failure we catch it in the PR before we do a release. I would hate to see a release fail because of an updated go compiler (though I suspect it would be rare, not something I'd want to debug when it happens).

.travis.yml Outdated
@@ -3,7 +3,7 @@ dist: xenial

language: go
go:
- 1.15.x
- 1.15.2
Copy link
Member

Choose a reason for hiding this comment

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

tl;dr +1 to pinning

Comment on lines +168 to +175
git add --all
git commit -m "Release v1.3.1"
git push -u origin release-v1.3.1
Copy link
Member

Choose a reason for hiding this comment

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

Can we do the add as part of the prerelease target, and then printout the git status or git diff from the Makefile.
That way I just have to review, then commit and push.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add in a follow-up.

@estroz estroz force-pushed the feature/goreleaser branch from 423f6f7 to d5d7e7c Compare November 30, 2020 21:22
agnostic of CI platform (albeit with some environment configuration).
Any Github user with write capabilities to the SDK repo can start
a release.

.ci/gpg: GPG public key, encrypted private subkey (used for signing),
and script to decrypt private subkey in CI

website/scripts: scripts to update versions in release commits and
branches at build time

.goreleaser.yml: goreleaser config file

*: script and Makefile updates to support automated releases

docs: release and installation guide simplifications, related docs cleanup
@estroz
Copy link
Member Author

estroz commented Nov 30, 2020

We've effectively moved away from travis, so some of the CI hookup stuff is no longer valid. I plan on making a follow-up for porting the release job to GH actions.

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2020
@estroz estroz merged commit 7e490b4 into operator-framework:master Nov 30, 2020
@estroz estroz deleted the feature/goreleaser branch November 30, 2020 23:05
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
This commit adds tooling for automated releases in CI using goreleaser, 
agnostic of CI platform (albeit with some environment configuration).
Any Github user with write capabilities to the SDK repo can start
a release.

.ci/gpg: GPG public key, encrypted private subkey (used for signing),
and script to decrypt private subkey in CI

website/scripts: scripts to update versions in release commits and
branches at build time

.goreleaser.yml: goreleaser config file

*: script and Makefile updates to support automated releases

docs: release and installation guide simplifications, related docs cleanup
Signed-off-by: reinvantveer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate release binary signatures with a consistent GPG key
5 participants