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

[simple-app] Re-order HPA metrics #102

Merged
merged 1 commit into from
Jan 31, 2022
Merged

[simple-app] Re-order HPA metrics #102

merged 1 commit into from
Jan 31, 2022

Conversation

ericayin
Copy link
Contributor

@ericayin ericayin commented Jan 27, 2022

@ericayin ericayin requested a review from a team as a code owner January 27, 2022 23:58
@@ -342,7 +342,7 @@ autoscaling:
# -- Configures the HPA to target a particular CPU utilization percentage
targetCPUUtilizationPercentage: 80
# -- If supplied, configures the HPA to target a particular Memory utilization percentage
# targetMemoryUtilizationPercentage: 80
Copy link
Contributor

Choose a reason for hiding this comment

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

Eh. I think it should be uncommented, but also left blank. You don't generally want to autoscale on CPU and Memory in most cases without lots of tuning. That's why it was left off here. It doesn't change the capability of the downstream chart from setting the value though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooh yea that makes sense, i'll leave it as is

@ericayin ericayin force-pushed the simple-app-hpa-memory branch from ae9af48 to d25e2c1 Compare January 28, 2022 01:08
@ericayin ericayin changed the title [simple-app] Uncomment HPA target memory value [simple-app] Re-order HPA metrics Jan 28, 2022
@ericayin ericayin requested a review from diranged January 28, 2022 01:11
Copy link
Contributor

@diranged diranged left a comment

Choose a reason for hiding this comment

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

👍 thank you!

@@ -2,7 +2,7 @@ apiVersion: v2
name: simple-app
description: Default Microservice Helm Chart
type: application
version: 0.16.2
version: 0.16.3
Copy link
Contributor

Choose a reason for hiding this comment

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

@ericayin oh rebase.. we're on 0.17+ now..

@ericayin ericayin force-pushed the simple-app-hpa-memory branch from d25e2c1 to 5d609b6 Compare January 31, 2022 18:36
@ericayin ericayin merged commit 82fbc63 into main Jan 31, 2022
@ericayin ericayin deleted the simple-app-hpa-memory branch January 31, 2022 19:00
poblahblahblah added a commit to poblahblahblah/sumologic-kubernetes-collection that referenced this pull request Jun 7, 2023
This addresses an issue when using ArgoCD (and maybe other GitOps operators)
where Kubernetes reorders the objects under the spec.metrics key thus causing
Sync issues with ArgoCD.

Originally reported to the ArgoCD project here:
argoproj/argo-cd#1079

Originally reported to the Kubernetes project here:
kubernetes/kubernetes#74099

Other projects and companies have also addressed this by simply reordering
the metrics section:

* kubernetes/ingress-nginx#10043
* nginx/kubernetes-ingress#3773
* grafana/helm-charts#758
* open-telemetry/opentelemetry-helm-charts#103
* Nextdoor/k8s-charts#102

Signed-off-by: Patrick O’Brien <patrick.obrien@thetradedesk.com>
poblahblahblah added a commit to poblahblahblah/sumologic-kubernetes-collection that referenced this pull request Jun 12, 2023
This addresses an issue when using ArgoCD (and maybe other GitOps operators)
where Kubernetes reorders the objects under the spec.metrics key thus causing
Sync issues with ArgoCD.

Originally reported to the ArgoCD project here:
argoproj/argo-cd#1079

Originally reported to the Kubernetes project here:
kubernetes/kubernetes#74099

Other projects and companies have also addressed this by simply reordering
the metrics section:

* kubernetes/ingress-nginx#10043
* nginx/kubernetes-ingress#3773
* grafana/helm-charts#758
* open-telemetry/opentelemetry-helm-charts#103
* Nextdoor/k8s-charts#102

Signed-off-by: Patrick O’Brien <patrick.obrien@thetradedesk.com>
swiatekm pushed a commit to SumoLogic/sumologic-kubernetes-collection that referenced this pull request Jun 13, 2023
* Reordering HPA metrics to match HPA ordering

This addresses an issue when using ArgoCD (and maybe other GitOps operators)
where Kubernetes reorders the objects under the spec.metrics key thus causing
Sync issues with ArgoCD.

Originally reported to the ArgoCD project here:
argoproj/argo-cd#1079

Originally reported to the Kubernetes project here:
kubernetes/kubernetes#74099

Other projects and companies have also addressed this by simply reordering
the metrics section:

* kubernetes/ingress-nginx#10043
* nginx/kubernetes-ingress#3773
* grafana/helm-charts#758
* open-telemetry/opentelemetry-helm-charts#103
* Nextdoor/k8s-charts#102

Signed-off-by: Patrick O’Brien <patrick.obrien@thetradedesk.com>

* add CHANGELOG entry

---------

Signed-off-by: Patrick O’Brien <patrick.obrien@thetradedesk.com>
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.

2 participants