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

Add resource limits to Ansible operator scaffolding #3573

Closed
asmacdo opened this issue Jul 27, 2020 · 5 comments · Fixed by #5274
Closed

Add resource limits to Ansible operator scaffolding #3573

asmacdo opened this issue Jul 27, 2020 · 5 comments · Fixed by #5274
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. language/ansible Issue is related to an Ansible operator project lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs discussion
Milestone

Comments

@asmacdo
Copy link
Member

asmacdo commented Jul 27, 2020

The Helm operator scaffolds resource limits in the manager manifest:

resources:
limits:
cpu: 100m
memory: 90Mi
requests:
cpu: 100m
memory: 60Mi

However these resource limits were too restrictive for ansible operators. Since v0.19 scaffolded Ansible operators do not have resource limits, I am considering this a new feature that can be added post 1.0 release.

@camilamacedo86 camilamacedo86 added language/ansible Issue is related to an Ansible operator project kind/feature Categorizes issue or PR as related to a new feature. labels Jul 27, 2020
@estroz estroz added this to the v1.1.0 milestone Aug 3, 2020
@kensipe kensipe added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Aug 3, 2020
@camilamacedo86
Copy link
Contributor

It is important because common attacks take advantage of it. See; https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html#rule-7-limit-resources-memory-cpu-file-descriptors-processes-restarts

PS. The default values are not enough for Ansible. So, the complexity here is to find what values should be appropriated.

@camilamacedo86 camilamacedo86 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 28, 2020
@jberkhahn jberkhahn modified the milestones: v1.1.0, v1.2.0, Backlog Sep 30, 2020
@jberkhahn jberkhahn added needs discussion and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Sep 30, 2020
@asmacdo
Copy link
Member Author

asmacdo commented Sep 30, 2020

Some research is needed to determine what these default values should be. We have deprioritized for now-- please feel free to chime in or open a PR with suggested defaults.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 30, 2020
@camilamacedo86 camilamacedo86 added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 21, 2021
@camilamacedo86
Copy link
Contributor

/lifecycle frozen

@camilamacedo86
Copy link
Contributor

@asmacdo, @fabianvf, @jmrodri shows like we need to be priorized this one.
I started to work on some POC tests to address this need.

We need to have default values that work for our samples, note that users will be notified that they ought to optimize the values based on their projects. See: https://github.com/operator-framework/operator-sdk/pull/5330/files#diff-063b9bbaede872aca0d6afe93d010d359c86cbc7ad4c1eab8652d8a5a46f6158R50-R51

It was also added in the docs:

Optimize manager resource values in config/manager/manager.yaml according to project requirements. It is recommended to define resources limits in order to follow good practices and for security reasons. More info: Managing Resources for Containers and Docker Security Cheat Sheet

https://sdk.operatorframework.io/docs/best-practices/common-recommendation/

And then, see: operator-framework/api#172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. language/ansible Issue is related to an Ansible operator project lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants