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.

Sign up for GitHub

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

Jump to bottom

🌱 Switch e2e to kind #2222

Open
wants to merge 4 commits into
base: release-0.9
Choose a base branch
from
Open

🌱 Switch e2e to kind #2222

wants to merge 4 commits into from

Conversation

lentzi90
Copy link
Member

@lentzi90 lentzi90 commented Jan 28, 2025 •
edited
Loading

What this PR does / why we need it:

This is a manual backport of #2209 and includes #2221

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

@metal3-io-bot metal3-io-bot added this to the BMO - v0.9 milestone Jan 28, 2025
@metal3-io-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@metal3-io-bot metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 28, 2025
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from lentzi90. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 28, 2025
@lentzi90 lentzi90 force-pushed the lentzi90/e2e-kind-release-0.9 branch from 4d6a63c to a16ce3f Compare January 28, 2025 09:42
@lentzi90
Copy link
Member Author

Note that this also affects CAPM3 e2e, specifically this kustomization must be changed because of the change to the ironic CA: https://github.com/metal3-io/cluster-api-provider-metal3/blob/main/test/e2e/data/ironic-deployment/overlays/release-27.0/kustomization.yaml

@lentzi90 lentzi90 marked this pull request as ready for review January 28, 2025 09:45
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 28, 2025
@metal3-io-bot metal3-io-bot requested review from honza and Rozzii January 28, 2025 09:45
@lentzi90 lentzi90 force-pushed the lentzi90/e2e-kind-release-0.9 branch from a16ce3f to 182250b Compare January 28, 2025 09:48
@lentzi90
Copy link
Member Author

/hold

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2025
@lentzi90
E2E: Clean up unused overlays
a0a69d5 
Remove BMO overlays no longer used in the e2e tests.
Drop upgrade from ironic-24.0 as it is out of support and not needed to
be tested anymore.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
E2E: Use kind instead of minikube
bc9db02 
Minikube is having troubles starting sometimes.
It was nice to work with since the VM could easily be attached to the
same network as the BMH VMs, but it is possible to work around that also
with kind.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
Fixture: Upload artifacts also on failure
028f265 
Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
E2E: Delete BMHs prior to namespace deletion
6c381b4 
Sometimes the fixture tests hit the timeout for namespace deletion.
The BMO logs indicate that BMO is trying to create new objects while the
namespace is terminating. For example HardwareDetails. To avoid this, I
think we can trigger deletion of the BMHs before we delete the
namespace.

We are running a bit close to the 1m deadline on successful runs
in the re-inspection test.
I believe this is explained by an extra reconcile loop when the
hardwaredetails are updated because of the inspection. No other fixture
test is close to this deadline normally.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90 lentzi90 force-pushed the lentzi90/e2e-kind-release-0.9 branch from 182250b to 6c381b4 Compare January 28, 2025 10:35
@tuminoid
Copy link
Member

/test metal3-ubuntu-e2e-integration-test-release-1-9

@lentzi90
Copy link
Member Author

I think that should fail since we would need to update https://github.com/metal3-io/cluster-api-provider-metal3/blob/main/test/e2e/data/ironic-deployment/overlays/release-27.0/kustomization.yaml to make it work

@tuminoid
Copy link
Member

tuminoid commented Jan 28, 2025 •
edited
Loading

I think that should fail since we would need to update https://github.com/metal3-io/cluster-api-provider-metal3/blob/main/test/e2e/data/ironic-deployment/overlays/release-27.0/kustomization.yaml to make it work

I'm aware, just wanted to see it fail before cherry-picks in CAPM3 merges to verify the assumption. Let's rerun after metal3-io/cluster-api-provider-metal3#2279 merges.

@tuminoid
Copy link
Member

I think that should fail since we would need to update https://github.com/metal3-io/cluster-api-provider-metal3/blob/main/test/e2e/data/ironic-deployment/overlays/release-27.0/kustomization.yaml to make it work

I'm aware, just wanted to see it fail before cherry-picks in CAPM3 merges to verify the assumption. Let's rerun after metal3-io/cluster-api-provider-metal3#2279 merges.

Hmmm, it passed without the patch. It is merged now, but it was not when this was started. Why?

@lentzi90
Copy link
Member Author

lentzi90 commented Jan 29, 2025 •
edited
Loading

Ok I figured it out. metal3-io/cluster-api-provider-metal3#2279 backports the fix to CAPM3 release-1.9. The fix itself was to change the kustomization that points to BMO main, because we only touched that BMO branch so far.

The metal3-ubuntu-e2e-integration-test-release-1-9 test is checking out CAPM3 release-1.9, which then deploys BMO main, and hence is compatible with the patched CAPM3.

Edit:
We are changing the replacements here and at the top of that kustomization.yaml, we are referencing BMO main

@lentzi90
Copy link
Member Author

If we merge this PR, we would break kustomizations pointing to BMO release-0.9, like this one

@tuminoid
Copy link
Member

The metal3-ubuntu-e2e-integration-test-release-1-9 test is checking out CAPM3 release-1.9, which then deploys BMO main, and hence is compatible with the patched CAPM3.

What? It is supposed to use BMO 0.9, why it is deploying BMO main?

@lentzi90
Copy link
Member Author

I was wrong. Looking at the wrong branch or repo.
Looks like CAPM3 release-1.9 pivot test is using the deploy script but I didn't manage to find how it deploys it the first time. Through dev-env then I guess?

@tuminoid
Copy link
Member

I was wrong. Looking at the wrong branch or repo. Looks like CAPM3 release-1.9 pivot test is using the deploy script but I didn't manage to find how it deploys it the first time. Through dev-env then I guess?

@mquhuy you've been looking at this dev-env/e2e interaction lately, any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s3rj1k s3rj1k Awaiting requested review from s3rj1k

@zhouhao3 zhouhao3 Awaiting requested review from zhouhao3

@honza honza Awaiting requested review from honza

@Rozzii Rozzii Awaiting requested review from Rozzii

Assignees
No one assigned
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
BMO - v0.9
Development

Successfully merging this pull request may close these issues.
3 participants
@lentzi90 @metal3-io-bot @tuminoid