-
Notifications
You must be signed in to change notification settings - Fork 225
k8s resources spec changes #381
k8s resources spec changes #381
Conversation
/test pull-knative-eventing-sources-integration-tests |
config/500-controller.yaml
Outdated
requests: | ||
cpu: 100m | ||
memory: 20Mi | ||
memory: 1000Mi |
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.
shouldn't this be 100Mi?
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.
@vaikas-google it's the same as the sources-controller has in eventing
repo - happy to change, though
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.
@n3wscott @vaikas-google happy to change to 100Mi
- if so... than I will also do the same for sources_controller
in "eventing" repo (b/c that's where I did borrow this value from)
thoughts?
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.
1000Mi
seems like a typo.
ok, i will change!
On Thu 9. May 2019 at 21:46, Scott Nichols ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In config/500-controller.yaml
<#381 (comment)>
:
> requests:
cpu: 100m
- memory: 20Mi
+ memory: 1000Mi
1000Mi seems like a typo.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#381 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABGPTUJK4MSNVI25T7BR2LPUR5QPANCNFSM4HJITG4A>
.
--
Sent from Gmail Mobile
|
@n3wscott I've updated the fix, and corrected the typo |
@vaikas-google @n3wscott would be good to have this in for the 0.6.0 release ? |
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.
I'm slightly terrified of what could cause the GitHub receive adapter to need 1GB of RAM, but the default request of 100MB seems reasonable.
/lgtm
/ok-to-test
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, matzew 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 |
Applying values from eventing core
see: https://github.com/knative/eventing/blob/master/config/500-sources-controller.yaml#L42-L47
/assign @n3wscott