Conversation
|
hey @skrech overall the strategy looks solid :) |
| ref: main | ||
| ssh-key: ${{ secrets.release_key }} | ||
|
|
||
| - name: Setup git |
There was a problem hiding this comment.
comment: totally out of scope, probably, but I wonder why we aren't signing the commits. See here how we are mixing up verified and non-verified commits: https://github.com/trento-project/ansible/commits/1.0.0
I guess this is simply because we don't have it elsewhere in the project... but perhaps it could be a good idea since we are revisiting some flows.
|
Thank you for the reviews, guys. :) |
|
Sorry, I have to re-request reviews, because of CHANGELOG handling work that might raise some concerns. PR info is updated with new section for CHANGELOG handling behaviour. |
| git config --global user.name "${GIT_USER}" | ||
| git config --global user.email "${GIT_AUTHOR_EMAIL}" | ||
|
|
||
| - name: Merge `release` into `main` |
There was a problem hiding this comment.
comment: Not sure if it's worth it, but... what if we check if this merge has any conflicts in advance? Something like a dry-run git merge --no-commit --no-ff origin/release; git status; git merge --abort
Perhaps it's too much, we may not need it, but wanted to mention it just in case.
There was a problem hiding this comment.
If there are any conflicts, I think I'd like them to be auto-resolved if possible.
My thought: If we're only back-porting fixes from main into release, then merging back release into main should be clean operation -- every change into release should be already in main.
-X ours is handling the case where for example I made a change A on main, then backport it into release and before releasing I made change B on the same place in main in the context of a bigger change that should be release yet. Then, when I actually release the hotfix, I don't want that to revert change B but rather preserve it.
| default: patch | ||
| exclude-labels: | ||
| - skip-release-notes | ||
| - released-as-hotfix |
There was a problem hiding this comment.
The take-aways are quite simple, after all:
- Nothing changes if we're going to release only full releases, as before.
- If you need to release a hotfix, you need to introduce the hotfix change with a PR marked with released-as-hotfix and also find the equivalent PR on main and retro-actively mark it with the same label.
- When making full release after hotfix, you don't need to do anything.
comment: Thank you for the explanation and summary. I'm not used to the release workflows in this project, so just dropping my two cents:
Relying on a manual two-step process seems really error-prone. At the very least, we would need a piece of documentation with step-by-step instructions on how to do it.
Don't know if it is an option, but, what about creating a kind of "backport PR workflow"?
For example, assuming we have an existing PR (against main) for a hotfix, a maintainer could just comment with /backport. This way, the bot could:
- cherry-pick the merged PR onto release and open a backport PR.
- apply
released-as-hotfixto both the backport PR and the original PR automatically. - cross-link the PRs in body/comments.
This way, we keep the release label-based dedupe logic, but we remove the manual work. WDYT?
There was a problem hiding this comment.
This actually sound amazing!
But I guess this would need developing a bot. I'm actually eager to work on that if others also thing it's worth it!
There was a problem hiding this comment.
Thanks :P
Not really, I meant the existing shapbot (like our hakube-bot) for the commits as the current PR does, but just using a workflow with a "comment" trigger that simply detects the / command.
Alternatively, we could even avoid the GHA action for the changelog, and build manually using external tools, like Goreleaser changelog or github-changelog-generator
There was a problem hiding this comment.
Ah, issue_comment trigger would be piece of cake! Thanks, @antgamdia !
I really don't like this gh action but our current release was relying on it and I wanted as little changes as possible. Anyway, it works with PRs, not commits, which gives us easy grouping based on PR labels. Of course, there could be other variants relying on naming or commit labeling, but we'll have to explore them in the future. This would also allow us to potentially de-couple from GH and make the process more universal.
/backport command seems to be a quick win, for now. Then we can expand on switching components further.
arbulu89
left a comment
There was a problem hiding this comment.
Hey @skrech ,
Really appreciated the long description about the process and the issue to solve.
Without having been thinking that much about it, I was wondering why would we do
- Fix issue in
main - Cherry pick to
release - Merge
releaseback tomain(maybe fixing conflicts)
Instead of simply:
- Fix issue in
release - Merge
releaseback tomain(maybe fixing conflicts)
At the end, the issue we want to fix is certainly present in release branch, while main could have had already some ongoing changes in the affected piece of code. In this case, backporting the fix looks maybe harder, as it cannot be cherry picked without resolving issues (this could happen in the other way as well). I find it more natural to work in the code that has exactly the "bug".
Other scenario or example could be as well that the "bug" in main was already solved during a new feature implementation, so cherry-picking a commit again is not feasible without including new features.
|
Hey @arbulu89, However, if possible, it's preferable (more cleaner maybe) to fix everything on main, then back-port/cherry-pick the fix in a hotfix. Of course, if that couldn't happen, then directly crafting a fix on |
|
Added documentation for the process in this PR: I consider this PR as complete in terms of concept. I intend to open new PRs for the good ideas mentioned in the comments:
Let me know if you wouldn't want to merge this without adding some of these items in this PR. |
|
LGTM; deferring the actual approval to someone more experienced in the release process here. But for now, it is an improvement from what we had, +1 to go ahead and add improvements next. Thanks for editing the docs! |
gagandeepb
left a comment
There was a problem hiding this comment.
Question: Something not very clear to me: how would we handle hotfixes to old major versions. For example, if current major version is 3.x.y and we need to support version 2.a.b for some users/deployers for whom there is no/limited upgrade path due to some reason to do a major version update for the near term.
Comment: Additionally, it would be interesting (in the future, not in this PR) to have a matrix of trento components' cross compatibility across different released versions. (not sure if this is even automate-able)
|
Hey @gagandeepb, this is in the docs: trento-project/docs#153 TL;DR: It's not possible right now to support hotfixes for older version if there is a major version after it. This would be possible if we handle tag sorting ourselves, but then tools like |
gagandeepb
left a comment
There was a problem hiding this comment.
Thanks @skrech. Looks good for now, let's talk about evolving the branching strategy as and when a need of sort I mentioned in my previous comment arises (i.e. if we really need to release a hotfix for an old major version).
Extended the CI to support "hotfix" releases.
Here is the explanation of how it works:
Overall, the process is the same as before -- to create a release we need to modify VERSION file and merge it, which would trigger the
release.yamlworkflow. However, there would be two branches now --main(with the meaning of development branch) andrelease(branch where we keep all the released stuff).releasebranch would be auto-created if a specific repository doesn't have it already.So, when we modify VERSION on
main, this would create a "cross-merge" betweenmainandreleasebranches.This means that:
mainis merged intorelease, soreleasewould get all the latest developments. Merge conflicts are auto-resolved inmains favor.releaseis merged back intomain. This is actually afast-forwardmerge (since we've just integrated all of main intoreleasein the previous step).mainandreleaseare pointing to it, at that moment.This workflow is most likely used for releasing major changes.
The other case is when modifying VERSION on
releasebranch, this is the case for "hotfix" releases. In this scenario:main) intorelease.release), and tag that commit with the new version tag.releasebranch is merged intomain. This would allow forgit describefrommainto track how many commits are done since last releasing. The merge auto-resolves conflicts, again, inmains favor. Please note, the tag for this release is made on a commit that is not the merge commit, but rather the one before it, made on thereleasebranch (this is the changelog-updating commit).NOTES:
mainand then "cherry-picked" ontorelease. The same approach as OBS's factory-first policy.git-release,obs-syncandpublish-containersworkflows intotrento-project/.githubto make them common re-usable blocks for all the rest of the trento projects. Following that, I'll remove them from this repo.I've used my personal fork to try this out, and you can see how
git log --graphlooks like here:https://github.com/skrech/ansible
Changelog handling caveat
Although this branching strategy looks sound and integrates well with container builds/publishes and obs sync, there is a slight problem with CHANGELOG handling. To understand the problem I need to describe how the changelog handling is done in our repos. It's based on a github action called
release-drafterand works like the following:1a.
release-draftergets all the releases in a repo (using the GH API);2a. sorts them in an ascending order and gets the latest one;
3a. creates an entry for every Pull Requests targeted at the branch that would be the base for the new release;
4a. the PRs considered are these that are made in the time window between discovered latest release (in 2.) and the top of the target branch.
This GH actions, thus, implies two constraints:
1b. Every change in a changelog should have a corresponding PR, the action can't extract changelog entries from commits only.
2b.
release-drafteraction only considers very simple use-cases where releases are happening in strictly chronological order and on a same branch.This effectively means that if we make hotfix release and there is work done on
mainalready, on the subsequent release of main, changes happened before the hotfix won't have changelog entries. This is obviously, non-acceptable.To solve this problem, I'll tackle the constraints enumerated above in reverse order:
2b. In my opinion
release-drafterhas very limited support for various git branching strategies. It uses the GH graphql API to get the commits with their associated PRs usingsincehistory lookup. However, we have the case where two branches share a common base commit but at the time of release they contain different commits. So, when we make release 4.0.0 but there were already two hotfix releaes 3.0.1 and 3.0.2, we want to include all the changes onmainsince 3.0.0 but not the changes introduced in 3.0.1 and 3.0.2. This is trivial to express in git:git log 3.0.2..4.0.0. But it's not howrelease-drafterworks.So, a workaround is introduced by me to handle that:
release-draftersupports a config options calledfilter-by-commitish, which takes part in step 2a -- in addition to sorting the releases it also filters them by the value of fieldtarget_commitish, eg. by the branch on which the release has been made. So, when making full releases, we consider only version 4.0.0, 3.0.0, and so on. When making hotfix releases we consider every release 3.0.2, 3.0.1, 3.0.0, etc.This has implications that we have to pay attention to tag the full release on
mainbranch even though both branch headsmainandreleasepoint to the same commit.target_commitishis GH API fields only. Hotfix releases are tagged onreleasebranch as expected, no special attention needed for them.Unfortunately,
filter-by-commitishis only available viarelease-drafter.yaml, so I had to make two version of this config file, one for full releases and one for hotfix releases, depending on which branch is triggering the release. These files are also needed in connection to 1b. as well.1b. As already mentioned,
release-drafterincludes only PRs in the changelog, not commits. This is actually not a big deal for us but it implies we create PRs toreleasebranch when introducing the hotfix changes. This is a bit clumsy because for a hotfix branch, one usually usescherry-pickoperation.There is more to this though, since this is not a cherry-pick operation, there is no way for git/gh to understand that a hotfix change was already released as part of hotfix release. Thus, there would be duplicate changelog entries when releasing full releases. To solve this, we have to mark every change from main that is released in a hotfix with newly-introduced
released-as-hotfixlabel. When this label is set,release-drafterwould skip inclusion of that PRs message as changelog entry for the release.Again, because
release-drafterdoesn't support the equivalent ofgit log a..b(orgit log a..b --cherry-pick), it would consider both changes onmainandreleasebranch when releasing full releases. Thus, PRs on both branches should be marked asreleased-as-hotfix.Anyway, don't despair. This is a lot of text to explain how things work and what were the decisions made. The take-aways are quite simple, after all:
released-as-hotfixand also find the equivalent PR on main and retro-actively mark it with the same label.