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

fix: fixed autoscaling with the hpa #210

Closed
wants to merge 3 commits into from

Conversation

TomerFi
Copy link
Contributor

@TomerFi TomerFi commented Dec 9, 2021

Added a Resources field to the WildFlyServerSpec type.

This will allow the user to specify the CPU requirement for containers,
which is mandatory for the HPA calculation of CPU utilization.

Example of HPA eligible application:

apiVersion: wildfly.org/v1alpha1
kind: WildFlyServer
metadata:
  name: quickstart
spec:
  applicationImage: "quay.io/wildfly-quickstarts/wildfly-operator-quickstart:18.0"
  replicas: 2
  resources:
    requests:
      cpu: "0.5"

Added a string Selector field of type string to the WildFlyServerStatus type.

This will be used for storing the selector label which is populated by the operator with the key app.kubernetes.io/instance.
The value for the label is created from the prefix wildfly- concatenated with the application name.
The same label is also applied to the underlying containers via the StatefulSet's template.

This is required for targeting the CR with the HPA instead of the underlying StatefulSet,
which is required to maintain the CR as the source for any of its resources.

Copy link
Member

@jmesnil jmesnil left a comment

Choose a reason for hiding this comment

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

Aside from my comment on the label, this looks good.

Thanks a lot for fixing this issue!

pkg/controller/wildflyserver/wildflyserver_controller.go Outdated Show resolved Hide resolved
@TomerFi TomerFi changed the title Allow autoscaling the operator with HPA fix: fixed autoscaling with the hpa Dec 16, 2021
@TomerFi TomerFi requested a review from jmesnil December 16, 2021 08:21
Copy link
Member

@jmesnil jmesnil left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@jmesnil
Copy link
Member

jmesnil commented Dec 16, 2021

This fixes #208 & #211

@BobVanB
Copy link
Contributor

BobVanB commented Jan 12, 2022

Good evening, any progress with this pull request?

@jmesnil jmesnil mentioned this pull request Jan 17, 2022
@jmesnil
Copy link
Member

jmesnil commented Jan 25, 2022

I'm closing your PR as it has been merged in the aggregated PR #215. Thanks!

@jmesnil jmesnil closed this Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants