kueue
kueue copied to clipboard
kubernetes-sigs
WIP: Use Helm to setup E2E tests env.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Use Helm to setup E2E tests env.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
The test-e2e-certmanager test requires Helm templates to be configured first.
Does this PR introduce a user-facing change?
NONE
Deploy Preview for kubernetes-sigs-kueue canceled.
| Name | Link |
|---|---|
| Latest commit | d9fabf0901eab295d8e4c8c4be6c1f81d3bfbdd3 |
| Latest deploy log | https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/68888a88b9f97f000840a882 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Do the tests pass entirely if run using HELM? If this is the case we could consider on nightly build using Helm, just so both ways of installation are tested. wdyt @tenzen-y ?
Do the tests pass entirely if run using HELM?
Let me test it one more time after changes.
/hold
LGTM label has been added.
/approve Thank you! /cherrypick release-0.13 /cherrypick release-0.12 to make future cherrypicks easier
@mimowo: once the present PR merges, I will cherry-pick it on top of release-0.12, release-0.13 in new PRs and assign them to you.
In response to this:
/approve Thank you! /cherrypick release-0.13 /cherrypick release-0.12 to make future cherrypicks easier
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
LGTM label has been added.
Do the tests pass entirely if run using HELM? If this is the case we could consider on nightly build using Helm, just so both ways of installation are tested. wdyt @tenzen-y ?
Why do we need this one? /hold
What is objective for this new E2E?
@mbobrovskyi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-kueue-test-e2e-main-1-30 | 6eab07b5cbf949557d377b468571f8725016a218 | link | true | /test pull-kueue-test-e2e-main-1-30 |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.
What is objective for this new E2E?
This feature allows contributors to test the code using a Helm setup. For example, in this PR, I needed to verify if it works with cert-manager. To do that, I just had to set this variable to true instead of running manual setup.
I also think it would be great to test our code with this setup in a periodic CI job.
+1, this PR is just for the developers to be able to easily run and verify installation with helm. There is no impact on CI pipelines.
Having said that, I would love to have some e2e tests for Helm, at least on nightly pipelne. We have had historically a number of bugs in helm already, and such a sanity check would help to prevent. We already have signals from users using Kueue on prod managed via Helm, so this setup is very important.
What is objective for this new E2E?
This feature allows contributors to test the code using a Helm setup. For example, in this PR, I needed to verify if it works with cert-manager. To do that, I just had to set this variable to true instead of running manual setup.
I also think it would be great to test our code with this setup in a periodic CI job.
Pointed PR problem manifests in generation, which is independent of the Kubernetes Cluster. So, we should add YAML generation Test instead of E2E using Kubernetes cluster.
Pointed PR problem manifests in generation, which is independent of the Kubernetes Cluster.
I think there were cases where the yaml syntax is correct, but incorrect configuration.
In any case, we can move the discussion if we need a job under dedicated issue: https://github.com/kubernetes-sigs/kueue/issues/5145.
This PR only aims to make such testing easier for developers.
Pointed PR problem manifests in generation, which is independent of the Kubernetes Cluster.
I think there were cases where the yaml syntax is correct, but incorrect configuration.
In any case, we can move the discussion if we need a job under dedicated issue: #5145.
This PR only aims to make such testing easier for developers.
For developer, I'm fine with that. For CI, let's consider what is root problem when we update Helm generation mechanism
@mbobrovskyi Thank you! /hold cancel /lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: mbobrovskyi, mimowo, tenzen-y
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [mimowo,tenzen-y]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@mimowo: new pull request created: #6234
In response to this:
/approve Thank you! /cherrypick release-0.13 /cherrypick release-0.12 to make future cherrypicks easier
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
@mimowo: #5174 failed to apply on top of branch "release-0.12":
Applying: Use Helm to setup E2E tests env.
Using index info to reconstruct a base tree...
M Makefile-test.mk
M hack/e2e-common.sh
Falling back to patching base and 3-way merge...
Auto-merging hack/e2e-common.sh
CONFLICT (content): Merge conflict in hack/e2e-common.sh
Auto-merging Makefile-test.mk
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Use Helm to setup E2E tests env.
In response to this:
/approve Thank you! /cherrypick release-0.13 /cherrypick release-0.12 to make future cherrypicks easier
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.