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.

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

🌱 test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster #11082

Merged
merged 10 commits into from
Aug 26, 2024

Conversation

chrischdi
Copy link
Member

What this PR does / why we need it:

Adjusts autoscale tests to:

  • Allow running cluster-autoscaler inside the management cluster
    • Note: for providers there may be no connectivity from the workload to the management cluster running in kind
  • make machinepool's optional for the test:
    • Note: not all providers support machinepools (e.g. CAPV)
  • add option for scale to/from zero tests

/area e2e-testing

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 #

@k8s-ci-robot k8s-ci-robot added area/e2e-testing Issues or PRs related to e2e testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 22, 2024
@chrischdi
Copy link
Member Author

/test help

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-build-main
  • /test pull-cluster-api-e2e-blocking-main
  • /test pull-cluster-api-e2e-conformance-ci-latest-main
  • /test pull-cluster-api-e2e-conformance-main
  • /test pull-cluster-api-e2e-main
  • /test pull-cluster-api-e2e-mink8s-main
  • /test pull-cluster-api-e2e-upgrade-1-31-1-32-main
  • /test pull-cluster-api-test-main
  • /test pull-cluster-api-test-mink8s-main
  • /test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-apidiff-main
  • pull-cluster-api-build-main
  • pull-cluster-api-e2e-blocking-main
  • pull-cluster-api-test-main
  • pull-cluster-api-verify-main

In response to this:

/test help

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 22, 2024
@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-main

@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-main

2 similar comments
@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-main

@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-main

@@ -143,7 +157,7 @@ type AddScaleUpDeploymentAndWaitInput struct {
func AddScaleUpDeploymentAndWait(ctx context.Context, input AddScaleUpDeploymentAndWaitInput, intervals ...interface{}) {
By("Create a scale up deployment with resource requests to force scale up")
if input.ContainerImage == "" {
input.ContainerImage = "registry.k8s.io/pause"
input.ContainerImage = "registry.k8s.io/pause:3.10"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not reference latest, did e.g. not work on arm64.

@@ -479,7 +479,7 @@ func DeployUnevictablePod(ctx context.Context, input DeployUnevictablePodInput)
Containers: []corev1.Container{
{
Name: "web",
Image: "registry.k8s.io/pause:latest",
Image: "registry.k8s.io/pause:3.10",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not reference latest, did e.g. not work on arm64.

@@ -648,5 +648,6 @@ func AssertMachineDeploymentReplicas(ctx context.Context, input AssertMachineDep
g.Expect(input.Getter.Get(ctx, key, md)).To(Succeed(), fmt.Sprintf("failed to get MachineDeployment %s", klog.KObj(input.MachineDeployment)))
g.Expect(md.Spec.Replicas).Should(Not(BeNil()), fmt.Sprintf("MachineDeployment %s replicas should not be nil", klog.KObj(md)))
g.Expect(*md.Spec.Replicas).Should(Equal(input.Replicas), fmt.Sprintf("MachineDeployment %s replicas should match expected replicas", klog.KObj(md)))
g.Expect(md.Status.Replicas).Should(Equal(input.Replicas), fmt.Sprintf("MachineDeployment %s status.replicas should match expected replicas", klog.KObj(md)))
Copy link
Member Author

@chrischdi chrischdi Aug 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the rollout finished and is stable. Otherwise the expected numbers in the next scale steps may be wrong because the scale up deployment considers nodes which will vanish.

@@ -365,5 +365,6 @@ func AssertMachinePoolReplicas(ctx context.Context, input AssertMachinePoolRepli
g.Expect(input.Getter.Get(ctx, key, mp)).To(Succeed(), fmt.Sprintf("failed to get MachinePool %s", klog.KObj(input.MachinePool)))
g.Expect(mp.Spec.Replicas).Should(Not(BeNil()), fmt.Sprintf("MachinePool %s replicas should not be nil", klog.KObj(mp)))
g.Expect(*mp.Spec.Replicas).Should(Equal(input.Replicas), fmt.Sprintf("MachinePool %s replicas should match expected replicas", klog.KObj(mp)))
g.Expect(mp.Status.Replicas).Should(Equal(input.Replicas), fmt.Sprintf("MachinePool %s replicas should match expected replicas", klog.KObj(mp)))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the rollout finished and is stable. Otherwise the expected numbers in the next scale steps may be wrong because the scale up deployment considers nodes which will vanish.

@chrischdi chrischdi changed the title 🌱 [WIP] test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster 🌱 test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster Aug 23, 2024
@chrischdi chrischdi changed the title 🌱 test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster 🌱 [WIP] test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster Aug 23, 2024
@chrischdi chrischdi force-pushed the pr-e2e-autoscaler-in-mgmt branch 3 times, most recently from 796ca22 to 995201a Compare August 23, 2024 10:19
@chrischdi chrischdi changed the title 🌱 [WIP] test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster 🌱 test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster Aug 23, 2024
@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-main

/assign @sbueringer

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-e2e-main 995201a link true /test pull-cluster-api-e2e-main

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.

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-sigs/prow repository. I understand the commands that are listed here.

@chrischdi
Copy link
Member Author

Can't explain it. Running in parallel again and testing locally

/retest

test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/e2e/autoscaler.go Outdated Show resolved Hide resolved
test/framework/autoscaler_helpers.go Outdated Show resolved Hide resolved
@sbueringer sbueringer added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 26, 2024
@sbueringer
Copy link
Member

Thank you!

/lgtm
/approve

@sbueringer
Copy link
Member

sbueringer commented Aug 26, 2024

/cherry-pick release-1.8

(for easier & earlier adoption by providers)

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 46049df5be6e5aa06b82bc357cd12dfec35f2cee

@k8s-infra-cherrypick-robot

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.8 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.8

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-sigs/prow repository.

@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2024
@k8s-ci-robot k8s-ci-robot merged commit 29a6e9a into kubernetes-sigs:main Aug 26, 2024
21 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.9 milestone Aug 26, 2024
@k8s-infra-cherrypick-robot

@sbueringer: new pull request created: #11093

In response to this:

/cherry-pick release-1.8

(for easier & earlier adoption by providers)

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-sigs/prow repository.

@sbueringer
Copy link
Member

cc @elmiko (Just fyi)

NareshKoduru pushed a commit to NareshKoduru/cluster-api that referenced this pull request Aug 29, 2024
regenerate CRDs

🌱 test: improve autoscale tests for to/from zero and running autoscaler in bootstrap cluster (kubernetes-sigs#11082)

* test: allow deploying autoscaler to management cluster

* test: make machine pools optional in autoscaler test

* test: implement optional scale from/to zero tests for autoscale

* test: allow modification of apigroup for infrastructure

* test: wait for rollouts to finish in autoscaler tests

* test: drop cleaning up autoscaler for machine pools

* review fix

* add comment about AutoScaleFromZero

* remove autoscale from zero test for unsupported MachinePools

* review fixes

update cert-manager to 1.15.3

Signed-off-by: Troy Connor <[email protected]>

Collect additional logs with CAPD log collector

Signed-off-by: Alexandr Demicev <[email protected]>

:seedling: Bump tj-actions/changed-files in the all-github-actions group

Bumps the all-github-actions group with 1 update: [tj-actions/changed-files](https://github.com/tj-actions/changed-files).

Updates `tj-actions/changed-files` from 44.5.7 to 45.0.0
- [Release notes](https://github.com/tj-actions/changed-files/releases)
- [Changelog](https://github.com/tj-actions/changed-files/blob/main/HISTORY.md)
- [Commits](tj-actions/changed-files@c65cd88...40853de)

---
updated-dependencies:
- dependency-name: tj-actions/changed-files
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: all-github-actions
...

Signed-off-by: dependabot[bot] <[email protected]>

:seedling: Bump google.golang.org/api

Bumps the all-go-mod-patch-and-minor group with 1 update in the /hack/tools directory: [google.golang.org/api](https://github.com/googleapis/google-api-go-client).

Updates `google.golang.org/api` from 0.193.0 to 0.194.0
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.193.0...v0.194.0)

---
updated-dependencies:
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: all-go-mod-patch-and-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

🌱  Fix error handling when the resource is not found (kubernetes-sigs#10907)

* fix: error handling when the resource is not found

Signed-off-by: sivchari <[email protected]>

* fix: test

* fix: owner cluster handling

Signed-off-by: sivchari <[email protected]>

* remove duplicated error

Signed-off-by: sivchari <[email protected]>

* remove log variable

Signed-off-by: sivchari <[email protected]>

* fix error handling when the controller reads the cluster

Signed-off-by: sivchari <[email protected]>

* revert test modification

Signed-off-by: sivchari <[email protected]>

* delete log

Signed-off-by: sivchari <[email protected]>

* remove unnecessary deletion

Signed-off-by: sivchari <[email protected]>

* add detail of error

Signed-off-by: sivchari <[email protected]>

---------

Signed-off-by: sivchari <[email protected]>

Add nilIsZero to all KSM metric configs where needed

Signed-off-by: Tobias Giese <[email protected]>

sorted labels and annotations in alphabatical order

Signed-off-by: hackeramitkumar <[email protected]>

📖 Fix CAPZ redirection links in quick-start page

Trigger Build

Trigger Build

Add nilIsZero to all KSM metric configs where needed

Signed-off-by: Tobias Giese <[email protected]>

sorted labels and annotations in alphabatical order

Signed-off-by: hackeramitkumar <[email protected]>

📖 Fix CAPZ redirection links in quick-start page

Trigger Build

📖 Fix CAPZ redirection links in quick-start page
@Sunnatillo Sunnatillo mentioned this pull request Nov 19, 2024
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/e2e-testing Issues or PRs related to e2e testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants