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

Helm: allow scheduling multiple pods from same zone on same node #5031

Merged
merged 11 commits into from
Jun 1, 2023

Conversation

alex5517
Copy link
Contributor

@alex5517 alex5517 commented May 17, 2023

What this PR does

Which issue(s) this PR fixes or relates to

Fixes #5018

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@alex5517 alex5517 requested a review from a team as a code owner May 17, 2023 03:59
@CLAassistant
Copy link

CLAassistant commented May 17, 2023

CLA assistant check
All committers have signed the CLA.

@dimitarvdimitrov dimitarvdimitrov self-requested a review May 17, 2023 08:12
@dimitarvdimitrov
Copy link
Contributor

thanks for addressing my feedback from #5018

We have a few committed manifests in the repo so it's easier to keep track of effects of changes to the chart. The CI is failing because these manifests are out of date since your change. You can update them by running make build-helm-tests and committing the diff

@alex5517
Copy link
Contributor Author

@dimitarvdimitrov,

Thanks, i have updated the tests.

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution. I think it would be great to also get @krajorama's feedback on this

@dimitarvdimitrov dimitarvdimitrov changed the title Fix/zoneaware anti affinity Helm: allow scheduling multiple pods from same zone on same node May 17, 2023
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
@alex5517
Copy link
Contributor Author

@krajorama

Thanks, i merged your suggestion.

@alex5517
Copy link
Contributor Author

@krajorama

Can this be merged?

Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Approved with comment, please update changelog with the modification and to not conflict with main thank you! (I was out of office)

operations/helm/charts/mimir-distributed/CHANGELOG.md Outdated Show resolved Hide resolved
@dimitarvdimitrov dimitarvdimitrov enabled auto-merge (squash) June 1, 2023 12:36
@dimitarvdimitrov dimitarvdimitrov merged commit 202d63a into grafana:main Jun 1, 2023
@alex5517 alex5517 deleted the fix/zoneaware-anti-affinity branch June 1, 2023 13:11
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.

app.kubernetes.io/component label wrong for zone aware deployment.
4 participants