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.

Sign up for GitHub

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

Jump to bottom

Fix usercluster manager oom #12089

Merged
merged 2 commits into from
Mar 27, 2023
Merged

Fix usercluster manager oom #12089

merged 2 commits into from
Mar 27, 2023

Conversation

vgramer
Copy link
Contributor

@vgramer vgramer commented Mar 23, 2023

What this PR does / why we need it:
After an upgrade, Helm will purge the release history.

Which issue(s) this PR fixes:

ref #12078

What type of PR is this?

/kind bug

Special notes for your reviewer:
the backport in 2.22 will be done manually as it required an additional step to overcome the OOM kill.

Does this PR introduce a user-facing change? Then add your Release Note here:

applications: fix OOM on usercluster-controller by limiting the history of helm releases.

Documentation:

NONE

@vgramer
applications helm integration test: refactor upgradeTest to make the …
d41f75e 
…expected releaseVersion parametrable

Signed-off-by: Vincent Gramer <vincent@kubermatic.com>
@kubermatic-bot kubermatic-bot added docs/none Denotes a PR that doesn't need documentation (changes). dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Mar 23, 2023
@vgramer vgramer added this to the KKP 2.23 milestone Mar 23, 2023
@kubermatic-bot kubermatic-bot added sig/app-management Denotes a PR or issue as being assigned to SIG App Management. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 23, 2023
@vgramer
Copy link
Contributor Author

vgramer commented Mar 23, 2023

/hold
to avoid conflict with the big refactoring PR.

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2023
@vgramer vgramer mentioned this pull request Mar 23, 2023
Merged
@vgramer
Copy link
Contributor Author

vgramer commented Mar 23, 2023

/retest

@xrstf
Copy link
Contributor

xrstf commented Mar 24, 2023

/unhold

This is fine, it won't collide too much with my refactoring. Thanks for the consideration!

@kubermatic-bot kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 24, 2023
@vgramer
applications: restrict helm history to avoid OOM kill
61f431a 
Signed-off-by: Vincent Gramer <vincent@kubermatic.com>
@vgramer vgramer force-pushed the fix-usercluster-manager-oom branch from 2a634d8 to 61f431a Compare March 24, 2023 16:29
@embik
Copy link
Member

embik commented Mar 27, 2023

/retest

@embik
Copy link
Member

embik commented Mar 27, 2023

@WeirdMachine I think we can lgtm this PR, WDYT?

@mate4st
Copy link
Contributor

mate4st commented Mar 27, 2023

/approve
/lgtm

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2023
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 5a397e3e43492da1593747b0e41d47c1c406fcc4

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vgramer, WeirdMachine

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

@kubermatic-bot kubermatic-bot merged commit fccab09 into kubermatic:main Mar 27, 2023
@xmudrii xmudrii added the backport-needed Denotes a PR or issue that has not been fully backported. label Mar 27, 2023
@embik embik added backport-complete Denotes a PR or issue which has been fully backported to all required release branches. and removed backport-needed Denotes a PR or issue that has not been fully backported. labels Mar 27, 2023
@mate4st mate4st mentioned this pull request May 8, 2023
Closed
@xrstf xrstf mentioned this pull request May 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mate4st mate4st mate4st left review comments

Assignees

@mate4st mate4st

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-complete Denotes a PR or issue which has been fully backported to all required release branches. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/app-management Denotes a PR or issue as being assigned to SIG App Management. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
KKP 2.23
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vgramer @xrstf @embik @mate4st @kubermatic-bot @xmudrii