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

test(e2e): fix helm upgrade test #6422

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

jakubdyszkiewicz
Copy link
Contributor

Before my recent PR #6403 Helm Upgrade test did not even upgrade Kuma. It was executing Upgrade on the same Helm Kuma version.

Helm opts are stored in the ControlPlane object, therefore there are carried to Upgrade command. The problem was that the first install was calling WithoutHelmOpt("global.image.tag") so that it disable default override of global.image.tag https://github.com/kumahq/kuma/blob/master/test/framework/k8s_cluster.go#L409 . However, it was carried to Upgrade() call. Therefore it was using the default app version from helm which is https://github.com/kumahq/kuma/blob/master/deployments/charts/kuma/Chart.yaml#L6

This worked on my machine, because as I develop I had images with 0.0.0-preview.vlocal-build tag locally.

I also removed WithHelmOpt("global.image.registry", Config.KumaImageRegistry) since it's redundant. There is already such default https://github.com/kumahq/kuma/blob/master/test/framework/k8s_cluster.go#L407

Checklist prior to review

  • Link to relevant issue as well as docs and UI issues --
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Tests (Unit test, E2E tests, manual test on universal and k8s) --
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? --
  • Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz jakubdyszkiewicz added the ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) label Mar 31, 2023
@jakubdyszkiewicz jakubdyszkiewicz requested review from a team, slonka and lukidzi and removed request for a team March 31, 2023 11:45
Copy link
Contributor

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

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

Nice one!

@slonka slonka enabled auto-merge (squash) March 31, 2023 12:12
@jakubdyszkiewicz
Copy link
Contributor Author

@Mergifyio backport release-2.1

@mergify
Copy link
Contributor

mergify bot commented Mar 31, 2023

backport release-2.1

✅ Backports have been created

@slonka slonka merged commit 7c834a9 into kumahq:master Mar 31, 2023
mergify bot pushed a commit that referenced this pull request Mar 31, 2023
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
(cherry picked from commit 7c834a9)
mergify bot added a commit that referenced this pull request Mar 31, 2023
test(e2e): fix helm upgrade test (#6422)

Signed-off-by: Jakub Dyszkiewicz <[email protected]>
(cherry picked from commit 7c834a9)

Co-authored-by: Jakub Dyszkiewicz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants