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

Add e2e test to cover basic autoscaler scenarios #8282

Closed
sbueringer opened this issue Mar 14, 2023 · 8 comments · Fixed by #8514
Closed

Add e2e test to cover basic autoscaler scenarios #8282

sbueringer opened this issue Mar 14, 2023 · 8 comments · Fixed by #8514
Assignees
Labels
area/testing Issues or PRs related to testing kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@sbueringer
Copy link
Member

sbueringer commented Mar 14, 2023

What would you like to be added (User Story)?

As a user it would be great if Cluster API would have coverage for basic autoscaler scenarios to ensure those work

Detailed Description

We would propose the following:

  • Add a new e2e test (that should be usable with various infra providers)
  • The e2e test should create a workload cluster with ClusterClass and deploy autoscaler on the workload cluster
  • And then test the following scenarios:
    • Create a MD with 2 replicas where autoscaler manages replicas from Day 1 (test scale up / down)
      • Note: This will cover that the defaulting webhook picks the replica count based on annotations so the autoscaler can act.
    • Take back control of the MD by removing the autoscaler annotations
      • Note: We might want to check that after taking back control the topology controller can scale up/down without the autoscaler fighting back.
    • Create a MD with 2 replicas without autoscaler annotations and then hand over control to the autoscaler by setting the annotations (Day 2)
      • Note: We should validate that while we set the annotations and unset the replica field the replica field is not reset to 1.

Anything else you would like to add?

No response

Label(s) to be applied

/kind feature
/area testing
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/testing Issues or PRs related to testing needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 14, 2023
@sbueringer
Copy link
Member Author

cc @ykakarap @fabriziopandini @jackfrancis @enxebre @elmiko

@fabriziopandini
Copy link
Member

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 14, 2023
@elmiko
Copy link
Contributor

elmiko commented Mar 14, 2023

this is great to see, i have a feeling that @fabriziopandini 's work on kubernetes-sigs/cluster-api-provider-kubemark#69 is going to pay off for this

@fabriziopandini
Copy link
Member

@elmiko yes, the kubeamark work is the foundation of #8262

@fabriziopandini
Copy link
Member

/assign @ykakarap

@enxebre
Copy link
Member

enxebre commented Mar 27, 2023

I'd expect us to cover MachinePool as well when as this gets merged kubernetes/autoscaler#4676 cc @mboersma

@sbueringer
Copy link
Member Author

I'd expect us to cover MachinePool as well when as this gets merged kubernetes/autoscaler#4676 cc @mboersma

Makes sense but I would like to get the current iteration merged first and we can extend the test to cover MachinePools in a follow up issue/PR

@fabriziopandini
Copy link
Member

+1 to have a first version of this test to merge ASAP, and then add MP in a follow up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues or PRs related to testing kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
6 participants