-
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
🌱 Add scale testing for upgrades #9077
🌱 Add scale testing for upgrades #9077
Conversation
846b51a
to
d4f69ad
Compare
KubernetesVersion: input.KubernetesUpgradeVersion, | ||
}, input.WaitForKubeProxyUpgrade...) | ||
|
||
if !input.SkipKubeProxyCheck { |
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.
Added this knob on the testing input to allow skipping this check. This requires routing from the host to the port on the in-memory provider pod which represents the API server of the workload cluster. I didn't figure out a way to enable this, but in this iteration upgrade checks for etcd, kube-proxy and coreDNS do not work.
@@ -127,7 +127,13 @@ func GetControlPlaneMachinesByCluster(ctx context.Context, input GetControlPlane | |||
Expect(input.ClusterName).ToNot(BeEmpty(), "Invalid argument. input.ClusterName can't be empty when calling GetControlPlaneMachinesByCluster") | |||
Expect(input.Namespace).ToNot(BeEmpty(), "Invalid argument. input.Namespace can't be empty when calling GetControlPlaneMachinesByCluster") | |||
|
|||
options := append(byClusterOptions(input.ClusterName, input.Namespace), controlPlaneMachineOptions()...) |
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.
AFAICT this never worked with multiple clusters as one label selector overrode the other - this ended up getting all machineDeployments in the namespace for me. Replaced it with a simpler inline definition of the label selector.
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.
Fun! nice catch!
if err != nil { | ||
_ = resp.WriteErrorString(http.StatusInternalServerError, err.Error()) | ||
return | ||
} | ||
if selector != nil { | ||
listOpts = append(listOpts, client.MatchingFieldsSelector{Selector: selector}) | ||
if fieldSelector != nil { |
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 incidental, but I fixed it up in the course of the PR and decided to keep it. Can split it out if needed.
d4f69ad
to
96b9c5a
Compare
96b9c5a
to
47676c9
Compare
The basic machinery is in place here - I think we should move forward with trying to merge it as-is and take the problems, as follow-ups. |
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.
/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 |
@killianmuldoon Can you please rebase? I'll try to find time to review right after |
47676c9
to
b1598e8
Compare
@sbueringer - sorry it took so long to get back to this. I'll spend some cycles on it if you get time to review though. |
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.
Thx for following-up to!
@@ -127,7 +127,13 @@ func GetControlPlaneMachinesByCluster(ctx context.Context, input GetControlPlane | |||
Expect(input.ClusterName).ToNot(BeEmpty(), "Invalid argument. input.ClusterName can't be empty when calling GetControlPlaneMachinesByCluster") | |||
Expect(input.Namespace).ToNot(BeEmpty(), "Invalid argument. input.Namespace can't be empty when calling GetControlPlaneMachinesByCluster") | |||
|
|||
options := append(byClusterOptions(input.ClusterName, input.Namespace), controlPlaneMachineOptions()...) |
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.
Fun! nice catch!
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
b1598e8
to
71bc6fe
Compare
Thank you very much! Really appreciate that you followed-up on this :) /test pull-cluster-api-e2e-main /lgtm |
LGTM label has been added. Git tree hash: d30effd733b14d1e37ab9c66f4d13866cda8f2b7
|
[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 |
/area testing |
Add upgrades to the Cluster API scale tests.
Part of #8814