Release process/workflow #1150

Boomatang · 2025-01-31T09:49:13Z

closes #1168
This PR targets the uses of a release.toml file for releases.

There are a number of changes that have being done in the hope to simplify future work.

The first main change is the adding of a release.toml file for the release. Below is a sample of what this file will look like. The file will be saved at the root of the project.

[kuadrant]
release = "1.0.1"

[kuadrant.olm]
channels = ["alpha"]
default_channel = "alpha"

[dependencies]
Authorino_bundle = "0.16.0"
Console_plugin = "0.0.14"
DNS_bundle = "0.12.0"
Limitador_bundle = "0.12.1"
Wasm_shim = "0.8.1"

The next major change is moving the moving the release code out of the Makefiles and into self contained scripts. Every thing from the tools required to validation. This is an attempt to allow the make the steps required to do a release simpler.

Validation

To validation that the process is still working as expected.

check out this branch
create a worktree for a past release. This sha was before cutting the release for v1.0.1

git worktree add /tmp/review 14d99004ab8dfda4bd4f8e50191da28d8bc85aa9

Create the release.toml file in the root of the project.

echo 'kuadrant:
  release: "1.0.1"

olm:
  channels: 
  - "alpha"
  default_channel: "alpha"

dependencies:
  Authorino: "0.16.0"
  Console_plugin: "0.0.14"
  DNS: "0.12.0"
  Limitador: "0.12.1"
  Wasm_shim: "0.8.1"' > /tmp/review/release.toml

Run the release script. NOTE: this will download any tools required that are missing from your path

ROOT=/tmp/review make prepare-release

compare the resulting changes with the commit that was created during the v1.0.1 release.

git -C /tmp/review diff 1e417b087bd96a5a1f5fc0c3e53e2237b324c1f8

The expected changes are as follows

The createdAT timestamp being different
The make/release.mk file being missing.

eguzki · 2025-01-31T15:25:20Z

Before jumpling to the review of the implementation:

The location that the tools are being installed to has also being changed to ~/.local/bin -> this can break interoperability with other repos that are using same tools (yq, helm, operator-sdk, kustomize) and require specific version.
I do not see as an issue the Golang dependency for installing tools. From the perspective of somebody releasing, it would be interacting with GH workflows, so no need to have golang installed. From the perspective of engineering, we all have. And to be honest, golang provides very nice/easy (not simple) mechanism to install tools locally.

What are your thoughts?

The release script has being design to use a toml file for the version and is to be run locally. Signed-off-by: Jim Fitzpatrick <[email protected]>

Boomatang · 2025-02-04T14:29:52Z

After the call today the implementation is changing and some of points to address your comments are not as valid.

The breaking of interoperability with other repos. So what if it does. Prior to the call today, these scripts were being designed to run in GitHub workflows. There should be no other repos using the tools. The argument over requiring specific version for the different repos, ya I can see that, but that problem also exists today. I don't add the project bin to my PATH every time I enter the project. So if I call any of those tools out side the make targets it is highly likely the version don't match. Even if the I was to use one in the project bin there is no guarantee that version would be the correct version. This has happened to me more than once were a version of a tool was updated and the only way I found out was by a failing build. Think we high to much of the complicity around tooling in the make targets. I do like the idea of nix for the management of tools, but I think it better to warn if a version we don't expect is being used.

On the Golang dependency, that is a hard one. Someone doing a release that is all in the GitHub workflows, they should not need it locally. We can agree on that. When I don't think we will agree is on the perspective of engineering. You say we all have, but that is not true. Last week I found my self using a machine that didn't have Golang install. In fairness, I was only wanting to explore the operator running, and do much edits. But I couldn't run make local-setup as it requires Golang even tho it builds the operator in container files. That machine still does not have Golang installed, and I still have not explored what I was going to look at. If there was no way to build the operator but only using go build locally, or all the dev tools could be installed using Golang, then ya I would have a lesser issue with it. We know the arguments between easy and simple, and I don't want to go there.

Remove the reference to bundles from the format. It is suspected that the released version of the operator will never be different from the bundles. There is also an overlap with helm which does not have the concept of bundles. Signed-off-by: Jim Fitzpatrick <[email protected]>

The release workflow has being modified to only be triggered when a pull request is merged to branches that match `release-vX.Y`. The workflow will only make the GitHub release objects. Signed-off-by: Jim Fitzpatrick <[email protected]>

Set up a make target for verifying that the prepare-release target was ran, and there is no uncommitted changes. There has also being a workflow set up to run this check when a Pull Request is made against the release branches. Signed-off-by: Jim Fitzpatrick <[email protected]>

Slight change to the form that is being used. Signed-off-by: Jim Fitzpatrick <[email protected]>

didierofrivia · 2025-02-07T10:56:18Z

.github/workflows/release.yaml

@@ -1,88 +1,34 @@
 name: Release Operator

 on:
-  workflow_dispatch:


I strongly believe this should not be removed. The new release process that is proposed adds many extra steps, both local and remote, reducing the profile of people that can release the operator and even if it adds some validation step to avoid human error, it doesn't guarantee mistakes won't be made. The release button could be enhanced to have an intermediate step that could add some extra validation.

Far I can add that back in.

Will add it in my following PR

eguzki

Some comments left

.github/workflows/release-changes.yaml

eguzki · 2025-02-07T12:21:15Z

.github/workflows/release.yaml

-        required: false
-        type: boolean
+  pull_request:
+    types:


no sure we want to generate a release every time a PR against release branches are merged. Thinking about backporting features/fixes from main branch

Good point, I will look at it. What is our process for doing backports? Do we have that documented anyway?

Currently, we cherry pick commits to the release branch from main.

What version of the kuadrant operator gets released after the backporting?

a patch release. Let's say you release v1.1.0. And continue adding features in main. Then we need to create v1.1.1 with some fixes. Those fixes are first added in main and then backported to v1.1.1 and v1.1.1 can be released. Makes sense?

Maybe we got this differently. From the RELEASE.md, when you are ready to release the existing content in kuadrant-vX.Y.Z, you create a new ephemeral branch named kuadrant-vX.Y.Z-rc(n) to hold the changes to be reviewed in the PR. After being merged the RC branch can be deleted. I do not see two long lived release branches.

Yes, less step back for a moment, I think we might be saying the same thing but in different ways.

We going about making the release for v1.1.0. The branch v1.1 is created which holds the patch version. A v1.1.0-rc1 branch is created from main. A PR is opened against v1.1 from v1.1.0-rc1. Once that PR is merged the GitHub release is created from the v1.1 branch for v1.1.0. After this the v1.1.0-rc1 branch can be deleted.

If a patch v1.1.1 is required, and main has moved on. The v1.1.1-rc1 branch is created. The required commits are cherry picked from main to v1.1.1-rc1. Then a PR is opened against v1.1, once merged the GitHub release is created from v1.1 for v1.1.1. Once this is done the v1.1.1-rc1 branch can be deleted.

At no point should changes land in the the v1.1 branch that didn't come from a PR. We may need logic to ensure that PR is coming from a patch rc branch for that version stream, but that can be an improvement.

Does my description align with your understand?

Yes, in general terms we are aligned. But, there is one fact that I see differently.

If a patch v1.1.1 is required, and main has moved on. The v1.1.1-rc1 branch is created. The required commits are cherry picked from main to v1.1.1-rc1

For me the v1.1.1-rc1 branch (and related PR) only includes the releasing updates, like versioning, manifests and so on.. The patches cherry-picked from main can be added to v1.1 branch in separate PRs, or directly pushed to v1.1 as we have been doing. For me v1.1 can be updated just like we do with main. Minor release branches, vX.Y, serves as base branch to create patch releases. In the same way, main branch serves as base branch to minor releases.

I also think it is a typo, but just in case, to clarity:

the branch v1.1 is created which holds the patch version. A v1.1.0-rc1 branch is created from main

v1.1.0-rc1 branch is created from v1.1 at the point we want to release.

Sorry if this process looks tedious. For me it is totally worthy. We need to be aligned and in the same page. And, like anything else in real life, there are multiple ways of doing it, all valid. We need to go through this helpful discussion in order to reach agreement we all feel comfortable with.

I agree this is the one thing that we do need to be in alignment about. And to that point I opened #1166 to update the release document.

I have yet to open a PR for, but I think that might be the correct place to continue the is discussion. Hopefully I will have that up in the next few days. I will ping here when it is up.

Opened #1169 to get the release doc update with the expected steps for bring changes into a release branch.

eguzki · 2025-02-07T12:25:15Z

.github/workflows/release-changes.yaml

+on:
+  pull_request:
+    types:
+    - opened


I think that we want to verify that releases are correct for all branches, including release branches. And not only opening a PR. This test should be in place with multiple verification checks, like running make bundle and then checking all changes are git committed.

The check that is being done here is ensuring the versions in the release.toml are set in the bundles & charts. It might not be good to enable this on all branches just yet as it would fail for any PR that is currently opened with out the release.toml file.

If it should be enabled for every branch long term is a different question. And I am not a 100% sure it should, but feel it would natural end up there. Which would be good as it would mean the verify-bundle and verify-helm` checks could be removed.

The check that is being done here is ensuring the versions in the release.toml are set in the bundles & charts. It might not be good to enable this on all branches just yet as it would fail for any PR that is currently opened with out the release.toml file.

Currently we are checking on every PR and main that bundles and charts are consistent with versions. If release.toml is missing, versions should have a default value and that default value is the one set on bundles and charts. That is what I would do, just food for thought.

Should the jobs verify-bundle and verify-helm be updated to use release.toml instead a new workflow? Task overlapping is quite clear.

Some what yes. The current verify-bundle and verify-helm check that there has being no changes to the CRD data. There is no checking of operand versions. Where as the verify-prepare-release will also check that the index can be built for olm.

For updating the current verify-bundle and verify-helm to use the release.toml. I though about this, and decided against it. I would be inclined to deprecate those targets. They both do the same modifications to the initial manifest, which can mean they can fallout of sync. They both are required to run together. I don't see any reason why you may want to run one but not the other.

There is also the last part where it is hard to know where people has set up automation with them targets. In this case I am thinking of a developers local workflow. Once the release.toml file is everywhere those targets could be removed.

The current verify-bundle and verify-helm check that there has being no changes to the CRD data. There is no checking of operand versions

If I am not mistaken (I implemented those originally) they are doing far more than check changes to the CRD data. They have dependency targets to build entirely OLM bundle and Helm charts. So all the manifests for OLM bundle and Helm Charts are built from scratch from Makefile and release.mk versions. Then it is asked to git if there is any change comparing to what has been checked in.

.github/workflows/release.yaml

eguzki · 2025-02-07T12:30:29Z

.github/workflows/release.yaml

-          create_branch: true
-          tagging_message: v${{ inputs.kuadrantOperatorVersion }}
-          commit_options: '--signoff'
+          bash ./hack/release/load_envvar.sh


You're not going to be able to modify the caller's shell because it's in a different process context.

You sure? I am almost sure I got that from GitHubs examples. Will check.

So I checked this in a test repo that I have, and there is no issues with calling it in this way. What I might do is rename the file to load_github_envvar.sh as it pipes into the $GITHUB_ENV which is only in the workflow.

Oh.. that is then different. It is populating github environment which is a data structure and it is not shell env var. Bear in mind that this approach does not work locally, only on github workflows. That can be a constraint and inconvenience to test locally the workflows.

Yes I hit that issue. The only step in that uses the data structure values is the softprops/action-gh-release@v1 which is creating the github release. There is no other scripts using that data.

And of local checking I have add a --echo flag that will echo out what would be added to the the structure. So I think we are okay.

hack/release/operator/make_helm.sh

eguzki · 2025-02-07T12:36:12Z

release.toml

+release = "1.0.1"
+
+[dependencies]
+Authorino = "0.16.0"


I am much in favor of dependency pinning here, but if I recall it correctly, we were going to address this in a following up work item and needs to be agreed (we might find some resistance). In main this file should not exist with well known defaults or exist with 0.0.0

This file can exist on any branch currently, there are only two make targets that are making use of it. The prepare-release and verify-prepare-release will use the file. If the file is not on main, then every time a release is created the releaser would need to create the file, this I could see as being error prone. It would be better to update a existing file.

I am ok having the release.toml in every branch, even main. This change is targeting main indirectly, so the values of this file should be those in main. Maybe 0.0.0 (which can be interpreted as latest for docker tags)

release.toml

Signed-off-by: Jim Fitzpatrick <[email protected]>

hack/release/load_github_envvar.sh

hack/release/operator/make_chart.sh

.github/workflows/release-operator.yaml

release.toml

Signed-off-by: Jim Fitzpatrick <[email protected]>

didierofrivia · 2025-02-14T13:14:46Z

release.yaml

@@ -0,0 +1,14 @@
+kuadrant:
+  release: "1.0.1"


Once this goes to main branch, all the versions will be 0.0.0 or something else was decided?

Nothing has being decided yet.

In that case I'm in favour of main --> 0.0.0 as it's reflected currently on our manifests (OLM, Helm)

didierofrivia · 2025-02-19T09:35:28Z

.github/workflows/release-operator.yaml

+    types:
+    - closed
+    branches:
+      - 'release-v[0-9]+.[0-9]+'


Was thinking that since we are targeting this workflow to be run once the candidate release merge into the release branch, we could do the following:

Suggested change

- 'release-v[0-9]+.[0-9]+'

on:

push:

branches:

- 'release-v[0-9]+.[0-9]+'

push since we'll be only merging once the candidate PR is approved, no?
Thus the conditional on line 13 won't be needed

didierofrivia

Given that everything regarding the automated process will be included in my following PR, matching the versions on main to 0.0.0 and reviewing the event that triggers the Release Operator are the only changes requested from my side. Maybe @eguzki can mark as resolve or expand on his review?

Good work!

didierofrivia · 2025-02-20T14:41:04Z

release.yaml

+kuadrant:
+  release: "1.0.1"
+
+olm:
+  channels: 
+  - "alpha"
+  default_channel: "alpha"
+
+dependencies:
+  Authorino: "0.16.0"
+  Console_plugin: "0.0.14"
+  DNS: "0.12.0"
+  Limitador: "0.12.1"
+  Wasm_shim: "0.8.1"


Suggested change

kuadrant:

release: "1.0.1"

olm:

channels:

- "alpha"

default_channel: "alpha"

dependencies:

Authorino: "0.16.0"

Console_plugin: "0.0.14"

DNS: "0.12.0"

Limitador: "0.12.1"

Wasm_shim: "0.8.1"

version: "1.0.1"

dependencies:

authorino-operator: "0.16.0"

dns-operator: "0.12.0"

limitador-operator: "0.12.1"

console-plugin: "0.0.14"

wasm-shim: "0.8.1"

olm:

channels:

- "alpha"

default_channel: "alpha"

The suggestion above makes it much leaner, and also matches the real dependency names.

didierofrivia · 2025-02-20T15:45:03Z

.github/workflows/release-validation.yaml

+    types:
+    - opened
+    branches:
+      - 'release-v[0-9]+.[0-9]+'


If this is meant for validating the RC branch, maybe the following will target better the event:

Suggested change

- 'release-v[0-9]+.[0-9]+'

- 'release-v[0-9]+.[0-9]+-?**'

Boomatang requested a review from a team as a code owner January 31, 2025 09:49

UPDATE: RELEASE SCRIPT

5e68f1a

The release script has being design to use a toml file for the version and is to be run locally. Signed-off-by: Jim Fitzpatrick <[email protected]>

Boomatang force-pushed the release_process/workflow branch from 1ed3d58 to 5e68f1a Compare February 4, 2025 14:01

Boomatang added 4 commits February 5, 2025 11:07

UPDATE: release workflow

e3755aa

The release workflow has being modified to only be triggered when a pull request is merged to branches that match `release-vX.Y`. The workflow will only make the GitHub release objects. Signed-off-by: Jim Fitzpatrick <[email protected]>

UPDATE: Format release.toml

44d9def

Slight change to the form that is being used. Signed-off-by: Jim Fitzpatrick <[email protected]>

didierofrivia requested changes Feb 7, 2025

View reviewed changes

eguzki requested changes Feb 7, 2025

View reviewed changes

eguzki reviewed Feb 7, 2025

View reviewed changes

release.toml Outdated Show resolved Hide resolved

UPDATE: PR Comments

3b1dfb0

Signed-off-by: Jim Fitzpatrick <[email protected]>