-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[WIP] 🌱 Drop MachinePools from topology self-hosted tests #9672
[WIP] 🌱 Drop MachinePools from topology self-hosted tests #9672
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This change should only be made on release-1.6
. Someone will have to open a new PR with the same contents against that branch.
/hold
/cc @kubernetes-sigs/cluster-api-release-team @nawazkh
@killianmuldoon: GitHub didn't allow me to request PR reviews from the following users: kubernetes-sigs/cluster-api-release-team. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
/area e2e-testing |
e16e89c
to
777cbe0
Compare
/test |
@killianmuldoon: The
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. |
/test pull-cluster-api-e2e-full-main |
Signed-off-by: killianmuldoon <[email protected]>
777cbe0
to
4ff97e0
Compare
/test pull-cluster-api-e2e-full-main |
@@ -84,8 +84,6 @@ func UpgradeClusterTopologyAndWaitForUpgrade(ctx context.Context, input UpgradeC | |||
Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling UpgradeClusterTopologyAndWaitForUpgrade") | |||
Expect(input.Cluster).ToNot(BeNil(), "Invalid argument. input.Cluster can't be nil when calling UpgradeClusterTopologyAndWaitForUpgrade") | |||
Expect(input.ControlPlane).ToNot(BeNil(), "Invalid argument. input.ControlPlane can't be nil when calling UpgradeClusterTopologyAndWaitForUpgrade") | |||
Expect(input.MachineDeployments).ToNot(BeEmpty(), "Invalid argument. input.MachineDeployments can't be empty when calling UpgradeClusterTopologyAndWaitForUpgrade") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a significant change in this PR, but I think we should allow upgrades of Clusters that don't have these workers defined in the yamls.
The alternative is to fill these lists in as empty at some point if they are nil, but it seems like there's little difference between the two at this point.
We shouldn't need to do this now that #8842 has merged. Let's keep monitoring the signal in the next few days and close this PR if we don't see the flake anymore. |
/close flakes are resolved |
@CecileRobertMichon: Closed this PR. 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. |
Creating this WIP PR as a template of what should be merged into the
release-1.6
branch - due to be created on November 14th - in order to complete #9656.Fixes #9656