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

feat(release workflows): Improve Release workflows to enable bugfix versions #1543

Merged

Conversation

rafaelmag110
Copy link
Contributor

WHAT

Implements the decisions made in the following DR: https://github.com/eclipse-tractusx/tractusx-edc/tree/main/docs/development/decision-records/2024-08-28-Release-Workflow-Refactor

  • The PR that triggers the release now has the main branch as base instead of releases. No need to be moving commits around from releases and then to main.
  • A new action publish-tag-and-release was created to enable a parametrized execution of the previous GH tag and release set of steps, depending if its a latest or bugfix release.
  • Release version is now resolved as the version in gradle.properties and not from branch names.
  • A manual workflow to trigger manual bugfix releases was introduced. It basically re-uses the release workflow.
  • Adds a validation step to skip the run if the tag to be created already exists.
  • Adds a gate that prevents unintentional workflow runs in forks.

WHY

  • Enable bugfix versions.
  • Refactor existing release workflows to remove the need for a releases/ branch and even release/* branches.
  • General CI improvements.

Closes #1448

@rafaelmag110
Copy link
Contributor Author

@paullatzelsperger
@wolf4ood
Please review.

Copy link
Contributor

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

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

minor remarks, looks good to me otherwise

.github/workflows/draft-release.yaml Outdated Show resolved Hide resolved
outputs:
branch_name: ${{ steps.release.outputs.branch_name || steps.bugfix.outputs.branch_name }}
steps:
- id: release
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I prefer a simple shell script over conditional steps or dedicated actions, because shell scripts can be easily replicated in a local environment and are generally more easily understandable.

Copy link
Contributor Author

@rafaelmag110 rafaelmag110 Sep 16, 2024

Choose a reason for hiding this comment

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

I refactored the validation jobs in 788baa1 since I also agree those could be simpler.

On a personal level, I prefer the conditionals on the jobs or actions. The workflow execution graph is calculated before it is sent to the runner and conditionals that evaluate to false are skipped. When everything is in a shell script you have to be more careful what behaviors should lead to job failure and explicitly have it fail.

I'm fine either way. Let me know if the refactor is ok.

Copy link
Contributor

@paullatzelsperger paullatzelsperger Sep 18, 2024

Choose a reason for hiding this comment

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

We've seen this numerous times, where we want to replicate the runner's behaviour and debug or test a workflow locally, e.g. after a refactoring or a maintenance change. If everything is a job with conditional execution, as opposed to a simple script in a single job, there is zero chance of doing that, and ACT only gets you so far. Maybe I'm just old and like bash scripting.

Let's get this merged as-is so as not to block the PR.

Generally speaking, I would be very cautious when adding GH actions, because we are effectively introducing undebuggable foreign code, that you have no control over.

.github/actions/publish-tag-and-release/action.yml Outdated Show resolved Hide resolved
.github/actions/publish-tag-and-release/action.yml Outdated Show resolved Hide resolved
.github/actions/publish-tag-and-release/action.yml Outdated Show resolved Hide resolved
check-head:
name: "Check if repository is not fork AND head is release OR base is bugfix"
runs-on: ubuntu-latest
if: ${{ github.repository == 'eclipse-tractusx/tractusx-edc' && (startsWith(github.ref_name, 'bugfix/') || startsWith(github.event.pull_request.head.ref, 'release/')) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks complicated plus it will be evaluated for every PR closed. given that we're following a sort-of gitflow approach, why don't we release on the release branch and at the end we merge it back to main later? this will simplify things a little.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will be evaluated for every merged PR on main but it won't run because of the condition. And since this is a job-level conditional, github will just skip its execution and not throw any error (btw, that is the reason why I prefer job/action level conditionals vs everything script).

Like such.

image

If we release from the release branch we can't create the tag from it, otherwise the release branch cannot be deleted.

I wouldn't say it looks complicated. Its actually very readable. It's just a veeeery long line 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

well, the tag can be created after the release branch has been merged into main. I think that's the typical gitflow approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I got your point then, sorry.
Your suggestion is to change the trigger of the release workflow to be the push on the release branch?

Copy link
Contributor

@ndr-brt ndr-brt Sep 16, 2024

Choose a reason for hiding this comment

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

Either having this workflow to run when a PR is closed on a release branch or making it manual as for the bugfix version.
At the end of the release we could merge back to main if ref_name starts with release and snapshot version can be bumped.
The more the release and bugfix processes are similar, the easier will be to maintain them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The workflow is being triggered when a PR is closed on a release branch. If we really want to avoid the workflow to be evalutated on every closed PR to main, we can make it manual only.

Copy link

sonarcloud bot commented Sep 18, 2024

Copy link
Contributor

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

let's try it out like this, still not really convinced about having the release workflow created for every pr merged but let's eventually change that later on

@paullatzelsperger paullatzelsperger merged commit 6dee261 into eclipse-tractusx:main Sep 19, 2024
33 checks passed
@rafaelmag110 rafaelmag110 deleted the improve_release_wfs branch September 19, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Release workflow: make base branch configurable
3 participants