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

change to upgrade job values #547

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

soumyapattnaik
Copy link

@soumyapattnaik soumyapattnaik commented Feb 15, 2024

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the values.yaml or README.md
  • Title of the PR starts with chart name (e.g. [velero])

@jenting
Copy link
Collaborator

jenting commented Feb 15, 2024

Why do we require to increase the memory limits?

@soumyapattnaik
Copy link
Author

soumyapattnaik commented Feb 15, 2024

Why do we require to increase the memory limits?

Please refer to below PR by my team member who did it for azure backup. In some customer we observed that upgradejob were getting OOMKilled after the maintenance window, they only got mitigated after they increased the limits to 512

earlier PR- #514

anshulahuja98
anshulahuja98 previously approved these changes Feb 21, 2024
@anshulahuja98
Copy link
Collaborator

@jenting can we uncomment https://github.com/vmware-tanzu/helm-charts/pull/524/files and bump up memory based on this PR.
This number has worked well for us in downstream usage.

@anshulahuja98 anshulahuja98 dismissed their stale review February 21, 2024 06:57

might need to uncomment default values

@anshulahuja98
Copy link
Collaborator

CC @qiuming-best

jenting
jenting previously approved these changes Feb 21, 2024
@qiuming-best
Copy link
Collaborator

qiuming-best commented Feb 22, 2024

@jenting lint-test failed with checking the chart version, should we bump up the version? or could someone approve it directly?

As I've committed to this PR, I cannot approve it now.

@jenting
Copy link
Collaborator

jenting commented Feb 22, 2024

@qiuming-best yez, the Chart version should upgrade as well.

@anshulahuja98
Copy link
Collaborator

@qiuming-best what are your thoughts on uncommenting the values?

@anshulahuja98
Copy link
Collaborator

For version bump I'll ask @soumyapattnaik

@jenting
Copy link
Collaborator

jenting commented Feb 22, 2024

Please don't uncomment, user could overwrite the value by their own. The comment value is the suggestion, not the mandatory value.

@anshulahuja98
Copy link
Collaborator

Alright sure
Have pinged @soumyapattnaik to bump the chart version, we can approve post that and checkin.

@soumyapattnaik
Copy link
Author

updated chart.yaml to 6.0.1

@jenting
Copy link
Collaborator

jenting commented May 17, 2024

Could you please help bump the version?

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