🌱 Add MP back to dualstack E2E test #10135
Conversation
k8s-ci-robot
added
area/clusterclass
|
/test pull-cluster-api-e2e-full-dualstack-and-ipv6-main |
|
@willie-yao: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
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/test-infra repository. |
|
I'm a bit confused why |
|
Okay I actually got an error from the ipv6 test on the fourth run with this error: |
|
@killianmuldoon I've retested the dualstack test around 8 times and it only failed once on the dualstack spec (other failures were flakes on other specs). What was the original error that caused the dualstack tests to fail? This is the test run that failed: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api/10135/pull-cluster-api-e2e-dualstack-and-ipv6-main/1757196147123818496 |
|
/test pull-cluster-api-e2e-conformance-ci-latest-main |
|
/test pull-cluster-api-e2e-conformance-main |
|
Sorry I've been missing the updated here! For context the dualstack tests runs the full CAPI tests suite - the same as e2e full - and adds a couple of dualstack conformance tests on top. I can't remember the source of the failures in #9477 unfortunately, and I'm not sure why it's not written down anywhere 😅. I will say looking at the history for this PR - https://prow.k8s.io/pr-history/?org=kubernetes-sigs&repo=cluster-api&pr=10135 - this looks way too flaky to merge. If I'm right then there's something in how we're running these tests that means they only fail when we touch the non-dualstack network of the MachinePools in some way. |
|
@killianmuldoon No problem, thanks for the update! I saw that the dualstack test was a bit flaky on this PR, but the same error did not show up twice and it looks like the conformance tests also passed. I think @fabriziopandini mentioned in the office hours that they did a few fixes to the MPs that may have fixed the issue. I'll continue to run these tests and see if there's anything reoccuring. |
|
AFAIR it's not just the conformance tests that are the problem in this instance - it's also down to which CAPI components end up on which nodes. One idea is to set up a repeating test - maybe five runs - and see if this change can pass that. |
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
|
@willie-yao - I think there are code changes needed on this one. If you'd like to organise something we can try to get to the bottom of it a bit by going through some of the logs etc. |
|
@killianmuldoon Sure thing, I'd be happy to pair on this. Since the same errors existed without MachinePools, I'm a bit unsure where to look to fix this as I'm not too familiar with the dualstack implementation. Is there somewhere I should look to get a better idea for this? |
|
I think the best place would be to debug through the logs of the failed jobs to understand exactly what's going on when they fial |
|
@willie-yao could you post some equivalent dualstack test failures that occur on clusters without MachinePools? |
|
Just a general comment. I think we probably should get the tests green on main first before introducing another potential source of errors. Except if we feel confident the new MP part is stable and won't add more flakiness |
I definitely agree. I'm seeing the same flakes on this PR as the ones that show up on main. It does seem like adding MPs makes it more flaky, but the reason is not super clear to me. |
|
Update: We decided in office hours that we should fix the flakes in the dualstack test for MachineDeployments before implementing them for MachinePools. Therefore, the dualstack test for MachinePools will be non-blocking for MachinePool graduation. |
|
PR needs rebase. 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/test-infra repository. |
k8s-ci-robot
added
the
needs-rebase
willie-yao
force-pushed
the
cc-mp-dualstack
branch
from
f2faa70 to
009f139
Compare
k8s-ci-robot
added
size/M
k8s-ci-robot
added
size/S
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
1 similar comment
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
|
Different/unrelated flake: #10356
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
|
I would suggest let's merge this PR. And once this was also stable for a bit (maybe a week?). Let's continue with merging dualstack & e2e-main? (cc @killianmuldoon) I think we should be mostly good, just want to do it incrementally and another week shouldn't hurt. |
|
sounds good to me |
|
(Or depending on how confident we are in this PR, maybe we'll do it the other way around) |
|
This might be a MachinePool-specific issue: When testing Cluster API working on self-hosted clusters using ClusterClass [ClusterClass] Should pivot the bootstrap cluster to a self-hosted cluster |
|
@sbueringer is your comment about pivoting to a self-hosted cluster feedback for a potential chante to this PR? |
I was just quoting the name of the failed test 😃 |
|
Ah wait, this PR doesn't influence that test. Or at least not directly |
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
2 similar comments
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main |
|
I think I would go ahead and merge it @killianmuldoon @chrischdi WDYT? |
|
ok for me :-) The last failure seems to be unrelated to this PR (although I cannot find it happening in the past). /lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 5de96bb21ff12366a864b41095eb8d36f4b66646
|
sbueringer
changed the title
|
/test pull-cluster-api-e2e-dualstack-and-ipv6-main For a green run |
|
/approve Thx @willie-yao !! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Let's go ahead with merging the two jobs |
What this PR does / why we need it:
This PR adds MachinePools back to the dualstack E2E test and fixes the issue found in #9477
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 part of #10028
/area clusterclass
/area e2e-testing