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 HPA support for VMAgent #863

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Conversation

rakesh-von
Copy link
Contributor

No description provided.

Copy link
Contributor

@zekker6 zekker6 left a comment

Choose a reason for hiding this comment

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

Hello @rakesh-von! Thank you for the PR, please check a few comments.

@@ -253,6 +253,13 @@ persistence:
# -- Bind Persistent Volume by labels. Must match all labels of targeted PV.
matchLabels: {}

# Horizontal Pod Autoscaling
horizontalPodAutoscaler:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename it to be autoscaling?
It will make it shorter and less error-prone to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

@rakesh-von Sorry for not being completely clear, I suggested renaming horizontalPodAutoscaler to autoscaling. So the whole field name will be shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zekker6 in every other chart it is horizontalPodAutoscaler, if i change it to horizontalPodAutoscaling or just autoscaling then vmagent would be the outlier. Are you sure you want this to be renamed to autoscaling?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rakesh-von Good point, let's keep it as it is now then. Thanks!

| horizontalPodAutoscaler.enabled | bool | `false` | Use HPA for vmagent |
| horizontalPodAutoscaler.maxReplicas | int | `10` | Maximum replicas for HPA to use to to scale vmagent |
| horizontalPodAutoscaler.minReplicas | int | `1` | Minimum replicas for HPA to use to scale vmagent |
| horizontalPodAutoscaler.metrics | object | `` | Metric for HPA to use to scale vmagent |
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on values.yaml metrics field should be a list. Could you sync docs to make sure it includes the latest updates from values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@rakesh-von
Copy link
Contributor Author

Hi @zekker6 ,

Made the proposed changes, can you please review it now?

@antoinedeschenes
Copy link
Contributor

Is there a purpose to automatically scaling VMAgent replicas?

@dg-nvm
Copy link

dg-nvm commented Feb 23, 2024

@antoinedeschenes it is usefull in multi-layer architecture, using remote-write protocol. Therefore vmagent scraping is not the one that is processing data later in the chain, and we would want to scale these.

@zekker6 zekker6 merged commit 514d5ad into VictoriaMetrics:master Feb 26, 2024
6 of 7 checks passed
zekker6 added a commit that referenced this pull request Feb 26, 2024
zekker6 added a commit that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants