[features] migrate OOM kill #479
Conversation
celenechang
force-pushed
the
celene/oom_kill_featurefactory
branch
from
27fae79
to
cfc4fb6
Compare
| IsRequired *bool | ||
| Containers []apicommonv1.AgentContainerName |
There was a problem hiding this comment.
suggested rename, wdyt @clamoriniere ?
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## main #479 +/- ##
=======================================
Coverage 59.68% 59.68%
=======================================
Files 3 3
Lines 129 129
=======================================
Hits 77 77
Misses 40 40
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| IsRequired *bool | ||
| Containers []apicommonv1.AgentContainerName |
| } | ||
|
|
||
| return feature.RequiredComponents{ | ||
| Agent: feature.RequiredComponent{IsRequired: &f.enable}, |
There was a problem hiding this comment.
here by setting the IsRequired: false when f.enable is false make the Agent globally unwanted (with the current logic implemented in the Feature Factory).
the idea was:
- set to
false: we should not deploy this component - nil: the feature don't need it, but don't care if it is deployed
- set to
true: the feature need this component.
we can revisite the logic.
But I would suggest to no set it if f.enable == false
There was a problem hiding this comment.
right, thanks for catching that. i also forgot to add containers
* [features] migrate oom kill
What does this PR do?
Migrate OOM kill check to use FeatureFactory.
This requires #430 before merging.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Describe your test plan
Write there any instructions and details you may have to test your PR.