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

fix: refactor docker tag and helm version determination via triggers #945

Merged
merged 1 commit into from
May 22, 2024

Conversation

maze88
Copy link
Contributor

@maze88 maze88 commented May 20, 2024

Git tag events (regardless of the branch) would not trigger the old workflow, and push branch events would trigger it - including the Helm CI, which would sometimes not be able to determine any version to set, thus failing.

This commit fixes the above, by:

  • Add a trigger for pushed tag events.
  • Determine docker tags and chart versions based on trigger event type.
  • Add conditions to Helm CI job steps, allowing it to be skipped (but not failed!) in cases where it is not required to run (such as an untagged intermediate commits on release branches).

@coveralls
Copy link
Collaborator

coveralls commented May 20, 2024

Pull Request Test Coverage Report for Build 9169939491

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.1%) to 64.181%

Files with Coverage Reduction New Missed Lines %
controllers/nicclusterpolicy_controller.go 1 78.21%
controllers/ipoibnetwork_controller.go 2 83.33%
pkg/state/state_ipoib_network.go 4 75.24%
Totals Coverage Status
Change from base Build 9075384735: 0.1%
Covered Lines: 3254
Relevant Lines: 5070

💛 - Coveralls

almaslennikov
almaslennikov previously approved these changes May 21, 2024
Copy link
Collaborator

@almaslennikov almaslennikov left a comment

Choose a reason for hiding this comment

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

A few nits, otherwise LGTM

.github/workflows/network-operator-docker-and-helm-ci.yaml Outdated Show resolved Hide resolved
.github/workflows/network-operator-docker-and-helm-ci.yaml Outdated Show resolved Hide resolved
run: |
git_tag=${{ github.ref_name }}
app_version=$git_tag
chart_version=${git_tag:1} # without the 'v' prefix
Copy link
Collaborator

Choose a reason for hiding this comment

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

(Can do after the merge, just so we don't forget) Need to make a test release to check that all versions in the helm chart / image versions are correct

Signed-off-by: Michael Zeevi <mzeevi@nvidia.com>
@e0ne
Copy link
Collaborator

e0ne commented May 22, 2024

CI failures are not related to this change, so it's safe to merge

@e0ne e0ne merged commit f2e4b26 into Mellanox:master May 22, 2024
13 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants