Add test for scaling machineSets#22544
Conversation
| } | ||
|
|
||
| // getNodesFromMachineSet returns an array of nodes backed by machines owned by a given machineSet | ||
| func getNodesFromMachineSet(c *kubernetes.Clientset, dc dynamic.Interface, machineSetName string) ([]*corev1.Node, error) { |
There was a problem hiding this comment.
I don't think we should define this type of behavior here. This really belongs in one of our projects, we should consume it as a library.
|
What exactly is this PR supposed to avoid/support? |
|
#22544 (comment) As the title says it "Adds test coverage for scaling machineSets" which is a feature we support in our product. From modifying a replica number to having a new running node there are multiple components involved: machine api, networking, container runtime, mco, etc. This is a gate to prevent any component to break this feature, e.g https://bugzilla.redhat.com/show_bug.cgi?id=1698624 |
|
@enxebre: The following tests failed, say
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. DetailsInstructions 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 understand the commands that are listed here. |
|
We are already testing this in e2e-aws-operator job, https://github.com/openshift/cluster-api-actuator-pkg/blob/master/pkg/e2e/infra/infra.go#L261, but this job runs only on MAO related repos. Because e2e-aws runs over different repos that are required by installer, adding these changes here will make sure that a change in any of those repos will not break machinseset functionality. /lgtm |
| }, scalingTime, 5*time.Second).Should(o.BeTrue()) | ||
|
|
||
| // expect new nodes to come up for machineSet1 | ||
| o.Eventually(func() bool { |
There was a problem hiding this comment.
may be a for loop will help code reuse.
|
tested it locally and there is an issue related to reading replica count from machinset. |
|
/lgtm cancel |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: enxebre If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
closed by #22564 |
Scaling machines/nodes is a feature we support. From modifying a replica number to having a new running node there are multiple components involved: machine api, networking, container runtime, mco, etc. This is a gate to prevent any component to break this feature, e.g https://bugzilla.redhat.com/show_bug.cgi?id=1698253, https://bugzilla.redhat.com/show_bug.cgi?id=1698624