Skip to content

Conversation

longwuyuan
Copy link
Contributor

What this PR does / why we need it:

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • CVE Report (Scanner found CVE and adding report)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation only

Which issue/s this PR fixes

fixes #10038

How Has This Been Tested?

  • Can be tested after merge for integration with ArgoCD by reporting user

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have added unit and/or e2e tests to cover my changes.
  • All new and existing tests passed.

/triage accepted

cc @tao12345666333 @strongjz @rikatz @cpanato

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jun 5, 2023
@k8s-ci-robot k8s-ci-robot requested review from cpanato and strongjz June 5, 2023 11:43
@k8s-ci-robot k8s-ci-robot added needs-priority area/helm Issues or PRs related to helm charts size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 5, 2023
@longwuyuan
Copy link
Contributor Author

/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jun 5, 2023
@strongjz
Copy link
Member

strongjz commented Jun 5, 2023

Can you update the title to add helm or chart update?

@longwuyuan
Copy link
Contributor Author

/retitle "helmchart hpa template change for mem spec before cpu spec"

@k8s-ci-robot k8s-ci-robot changed the title ensured hpa mem spec before cpu spec "helmchart hpa template change for mem spec before cpu spec" Jun 5, 2023
@longwuyuan
Copy link
Contributor Author

longwuyuan commented Jun 5, 2023

Can you update the title to add helm or chart update?

@strongjz changed title

@peters5 @M0dj0 if all tests pass and all good, this merge will be available only on next chart release or until next release you have to use directly from the github project src URL

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

/hold for all tests

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 5, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, longwuyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2023
@cpanato
Copy link
Member

cpanato commented Jun 5, 2023

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 5, 2023
@k8s-ci-robot k8s-ci-robot merged commit 7d27f39 into kubernetes:main Jun 5, 2023
@longwuyuan longwuyuan deleted the issue-10038 branch June 5, 2023 15:11
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 <[email protected]>
@peimanja
Copy link

peimanja commented Jun 9, 2023

When will be a new chart release including this fix ?

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 <[email protected]>
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 <[email protected]>

* add CHANGELOG entry

---------

Signed-off-by: Patrick O’Brien <[email protected]>
@jukie jukie mentioned this pull request Jun 22, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/helm Issues or PRs related to helm charts cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reordering of metrics in controller HPA resource is causing sync problems

5 participants