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

Reordering HPA spec.metrics to match HPA ordering #3078

Merged

Conversation

poblahblahblah
Copy link
Contributor

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:

Checklist

  • Changelog updated or skip changelog label added
  • Documentation updated
  • Template tests added for new features
  • Integration tests added or modified for major features

@poblahblahblah poblahblahblah requested a review from a team as a code owner June 7, 2023 18:08
@sumo-drosiek
Copy link
Contributor

@poblahblahblah Thanks for the PR!

Seems that templates are not generated correctly. Probably there is now newline rendered because of - in helm templates

@swiatekm
Copy link

swiatekm commented Jun 12, 2023

It's a bit concerning that template tests don't catch this. We should have a test that unmarshals all the templates into proper K8s objects to check for structural issues like this one. Right now, the tests use Unstructured, which is just a glorified map with metadata.

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
* nginxinc/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 poblahblahblah force-pushed the address-argocd-hpa-reordering-issue branch from 7b06a4f to 82f149d Compare June 12, 2023 18:04
@poblahblahblah
Copy link
Contributor Author

Ooof, good catch @sumo-drosiek

Fixed and pushed. Tested again and helm template . now renders correctly.

@swiatekm
Copy link

@swiatekm swiatekm merged commit 39bdc9a into SumoLogic:main Jun 13, 2023
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.

3 participants