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

Unified mechanism for Cluster Autoscaler feature flags #5432

Open
x13n opened this issue Jan 19, 2023 · 3 comments
Open

Unified mechanism for Cluster Autoscaler feature flags #5432

x13n opened this issue Jan 19, 2023 · 3 comments
Labels
area/cluster-autoscaler area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@x13n
Copy link
Member

x13n commented Jan 19, 2023

Which component are you using?:

cluster-autoscaler

Is your feature request designed to solve a problem? If so describe the problem this feature should solve.:

Enabling new features in Cluster Autoscaler usually requires a feature flag. Existing examples include --scale-down-enabled (sic!), --debugging-snapshot-enabled or --parallel-drain. These flags contribute to an already existing huge API surface of Cluster Autoscaler flags. Additionally, implementation of such flags sometimes requires passing them through a few layers of abstraction (e.g. through AutoscalingOptions), making the code harder to maintain.

Describe the solution you'd like.:

There should be a single flag to control feature enablement. Feature flags should be accessible from the code - they are global to the entire CA process anyway. I'd reuse the approach taken in kube-apiserver: http://go/kuber/blob/1ca2180f300bb83e4581c7efb02ab91b7027b6f3/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go#L32

Describe any alternative solutions you've considered.:

Build a custom mechanism for this. On one hand we don't really need all the alpha/beta/GA in Cluster Autoscaler, but it will be simpler to reuse what is already there (especially since CA already takes a dependency on core k8s).

Additional context.:

/help

@x13n x13n added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 19, 2023
@k8s-ci-robot
Copy link
Contributor

@x13n:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

Which component are you using?:

cluster-autoscaler

Is your feature request designed to solve a problem? If so describe the problem this feature should solve.:

Enabling new features in Cluster Autoscaler usually requires a feature flag. Existing examples include --scale-down-enabled (sic!), --debugging-snapshot-enabled or --parallel-drain. These flags contribute to an already existing huge API surface of Cluster Autoscaler flags. Additionally, implementation of such flags sometimes requires passing them through a few layers of abstraction (e.g. through AutoscalingOptions), making the code harder to maintain.

Describe the solution you'd like.:

There should be a single flag to control feature enablement. Feature flags should be accessible from the code - they are global to the entire CA process anyway. I'd reuse the approach taken in kube-apiserver: http://go/kuber/blob/1ca2180f300bb83e4581c7efb02ab91b7027b6f3/staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go#L32

Describe any alternative solutions you've considered.:

Build a custom mechanism for this. On one hand we don't really need all the alpha/beta/GA in Cluster Autoscaler, but it will be simpler to reuse what is already there (especially since CA already takes a dependency on core k8s).

Additional context.:

/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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jan 19, 2023
@guopeng0
Copy link
Contributor

guopeng0 commented Dec 2, 2023

Hi @x13n
I’m interested in the issue and would like to take on the task. Any guidance or suggestions on how to start? Excited to contribute to the project and help resolve this issue.

@x13n
Copy link
Member Author

x13n commented Dec 4, 2023

Hi @guopeng0
Thanks for your interest in this. I think I would start from building some proof of concept and ideally discussing on the SIG meeting on Monday. Reusing the k8s library I linked above seems like a good starting point.

@towca towca added the area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cluster-autoscaler area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants