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: Add release workflow #166

Merged

Conversation

Tomasz-Smelcerz-SAP
Copy link
Member

Description

Add release workflow to the project.

Changes proposed in this pull request:

  • Adds release workflow to the project
  • Adds a document describing steps to use the new release workflow

Related issue(s)
#134

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP requested review from a team as code owners November 16, 2023 07:26
@kyma-bot kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 16, 2023
@jeremyharisch jeremyharisch self-assigned this Nov 16, 2023
Comment on lines 7 to 12
workflow_dispatch:
inputs:
name:
description: 'Existing tag for the new release'
default: ""
required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not let it automatically trigger it when a new tag get pushed like:

name: CI
on:
  push:
    tags:
      - 'v[0-9]+.[0-9]+.[0-9]+'  # matches on tags like v1.2.3

And the instead Check if docker image exists we would wait until this image is build with a certain timeout like 15min. So not fail directly when image is not found, but retry with final timeout.

What do you think? I think this should also work and thus we do not need a manual trigger for each release and no check if the tag which was provided exists.

Copy link
Member Author

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP Nov 16, 2023

Choose a reason for hiding this comment

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

The reason I did not went into "full automation" is exactly what you mentioned: Waiting for the image to be built, error handling, etc.
If a new tag is created and prowjob fails to build the image, or builts it AFTER the timeout configured here, then what? We have to remove the tag and re-create it? This should also be described in "How to do the release". And so on.
I decided this way (I implemented it now) is most robust. Yes, it's not fully automatic. But there's nothing that can go wrong, it's either success or failure and you don't have to "rollback" anything.
Let's discuss this further, I am not saying my approach is the best one - It just reflects my personal bias towards "foolproof" solutions, even if they require a bit manual effort.

Comment on lines 24 to 29
- name: Check if source code tag exists
run: |
if ! [ $(git tag -l ${{ inputs.name }}) ]; then
echo "::error ::Tag ${{ inputs.name }} doesn't exist"
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in other comment, I would appreciate if we could do everything automatically since this also only checks if the tags exists, but not if that is the actual wanted release. One more point of failure.

If the manual trigger is needed, then ok I think this step is not other possible :(

GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
LAST_RELEASE_TAG: ${{ needs.validate-release.outputs.last_release_tag }}
run: |
echo "Generating changelog from version: ${{ inputs.name }} to version: $LAST_RELEASE_TAG"
Copy link
Contributor

Choose a reason for hiding this comment

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

This sound confusing, I suggest something simpler like:

echo "Generating changelog for version: ${{ inputs.name }}"

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP changed the title Add release workflow feat: Add release workflow Nov 16, 2023
jeremyharisch
jeremyharisch previously approved these changes Nov 20, 2023
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 20, 2023
Copy link
Contributor

@mmitoraj mmitoraj left a comment

Choose a reason for hiding this comment

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

I would suggest:

  • moving the document to the /docs folder
  • renaming the file so that it is written in lower case (how_to_release.md)
  • introducing a link to the how_to_release.md file in the root README.md in the Read more section

HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
jeremyharisch
jeremyharisch previously approved these changes Nov 21, 2023
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 21, 2023
@kyma-bot kyma-bot added area/documentation Issues or PRs related to documentation and removed lgtm Looks good to me! labels Nov 21, 2023
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 21, 2023
@kyma-bot kyma-bot merged commit 0fbf0b8 into kyma-project:main Nov 21, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Issues or PRs related to documentation lgtm Looks good to me! size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants