Removing limits

[matzew]

Related to knative/eventing-contrib#457

Proposed Changes

• remove resource limits, since they were initial guesses ...

Release Note

Removing the resource limits (which is a hard maximum) for eventing-controller, eventing-webhook and sources-controller

[matzew]

/asign @evankanderson following up on a similar thread from the eventing-contrib repo ...

[duglin]

Why are we removing theses? W/o some kind of limit we're giving the pods the ability to be a noisy neighbor.

[matzew]

they are initial guess, and were causing errors already: (related, not here, but on sources -> https://bugzilla.redhat.com/show_bug.cgi?id=1714183)

so, I think we keep the requests (which is a minimum guarantee), but remove the guessed limits (which is a hard maximum).

I think @bbrowning and @mattmoor also talked/agreed that if we add any of those, we should be confident, instead of blind guesses, causing potential issues/harm

[duglin]

I think it also comes down to the scope of the harm. No limit means the entire node can go down (which we saw and is why we added limits) vs just one pod having issues and is easy to notice and debug.

[evankanderson]

Adding reservations (requests) should reduce the amount of bin-packing that happens, even if we don't set hard limits which cause the container to be killed when exceeded.

We might also want to tune the requests after measuring current container sizes.

[evankanderson]

/lgtm

[vaikas]

/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, vaikas-google

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [vaikas-google]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment