-
Notifications
You must be signed in to change notification settings - Fork 66
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
WINC-703: [config] adds controller manager resource limits #2099
Conversation
@wgahnagl: This pull request references WINC-703 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make bundle
might be required to propagate these changes into the CSV
8a1aafb
to
fbfc5cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
nit: it's generally good practice to put any commands you ran to generate changes in the commit message, something like "Ran make bundle
for CSV changes"
resources: | ||
requests: | ||
cpu: 20m | ||
memory: 50Mi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for the requests:
, but we need a limit
as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MCO doesn't have a limit
set for its resources. Should I check what other operators are using for their limits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd say. requests
helps the scheduler to place the pod, and limit
is actually what controls the resource boundaries.
@wgahnagl Please mention in the PR description that this command ran. |
@wgahnagl: This pull request references WINC-703 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
eb93b50
to
3d01e64
Compare
config/manager/manager.yaml
Outdated
requests: | ||
cpu: 20m | ||
memory: 50Mi | ||
limits: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch the order of limits
and requests
so they are alphabetical which would match what is added to the bundle CSV.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mansikulkarni96, wgahnagl 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 |
Adds resource requests cpu: 20m memory: 50Mi taken from existing MCO resource requests, and Adds resource limits cpu: 100m memory:300Mi, found through testing.
/lgtm |
/test remaining-required |
/test nutanix-e2e-operator
Retesting to see if this is a flake |
@wgahnagl: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Adds resource limits for containers, taken from MCO's resource limits.
Testing done on a 4.14 GCP cluster with 10 windows worker node, and a 4.14 platform none cluster, with one windows worker node.
ran make bundle