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: Create release process #3485

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open

feat: Create release process #3485

wants to merge 55 commits into from

Conversation

dbadura
Copy link
Contributor

@dbadura dbadura commented Nov 18, 2024

Description

Changes proposed in this pull request:

  • Create release automation for busola

Related issue(s)

Definition of done

  • The PR's title starts with one of the following prefixes:
    • feat: A new feature
    • fix: A bug fix
    • docs: Documentation only changes
    • refactor: A code change that neither fixes a bug nor adds a feature
    • test: Adding tests
    • chore: Maintainance changes to the build process or auxiliary tools, libraries, workflows, etc.
  • Related issues are linked. To link internal trackers, use the issue IDs like backlog#4567
  • Explain clearly why you created the PR and what changes it introduces
  • All necessary steps are delivered, for example, tests, documentation, merging
    Releasing with semantic versioning #3375

@kyma-bot kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 18, 2024
@dbadura dbadura changed the title Feat: Create release process feat: Create release process Nov 19, 2024
@dbadura dbadura marked this pull request as ready for review November 19, 2024 12:28
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2024
@dbadura dbadura added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2024
@dbadura dbadura removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2024
Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

Check after merge if latest is build, and user can run Busola via docker using latest images.

Check internal docs deploy-PR-on-cluster if changes to kustomization will break our previous flow.

Comment on lines 28 to 30
context: backend
export-tags: true
tags: latest
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure that we can remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

export-tags is not needed in my opinion. According to docs:

description: Export parsed tags as build-args into dockerfile. Each tag will have format TAG_x, where x is the tag name passed along with the tag

I think that it's unnecessary complication and it hides the build logic.

export-tags: true
tags: latest
tags: ${{ inputs.tag != '' && inputs.tag || 'latest' }}
build-args: ${{ inputs.tag != '' && format('tag={0}', inputs.tag) || '' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure after first release on dev, that it is working and displayed correctly

Comment on lines 31 to 36
# integrations:
# needs: create-draft
# secrets: inherit
# uses: ./.github/workflows/_integration-tests.yaml
# with:
# image: europe-docker.pkg.dev/kyma-project/prod/serverless-operator:${{ github.event.inputs.name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove it for now, will be done in separate task

# needs: upgrade-images
create-release:
name: Create release
needs: [ build-web, build-backend ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
needs: [ build-web, build-backend ]
needs: [ build-web, build-backend, build-local ]

Comment on lines 46 to +47
fetch-depth: 0
ref: ${{ github.ref_name }} # Checkout to latest branch changes
ref: ${{ github.ref_name }} # checkout to latest branch changes ( by default this action checkouts to the SHA that triggers action )
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if this fetch-depth is needed

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, according to docs:

# Number of commits to fetch. 0 indicates all history for all branches and tags.
    # Default: 1
    fetch-depth: ''
    ```
If we want to create changelog, it's needed

@@ -15,7 +16,7 @@ ENV CI true

COPY . /app

RUN yq -i '.version = "'${default_tag}'"' public/version.yaml && \
RUN export TAG=${tag:-$default_tag} && yq -i '.version = "'${TAG}'"' public/version.yaml && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some comment to clarify what's going on

Comment on lines +20 to +22

envsubst < "${TMP_DIR}"/ingress/ingress.tpl.yaml > "${TMP_DIR}"/ingress/ingress.yaml
kubectl apply -k "${TMP_DIR}"/ingress --namespace=$NAMESPACE
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be changed with #3493

@dbadura dbadura linked an issue Nov 22, 2024 that may be closed by this pull request
4 tasks
Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for second review to approve

Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

Please add documentation: #3375 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. 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.

Releasing with semantic versioning
4 participants