Return 429 when ingress if sending too man messages, instead of 500

[tayarani]

Fixes #1697

Proposed Changes

• When the ingress is overloaded, propagate a 429 back to the caller instead of masking it with a 500.

Release Note

Return 429 (too many messages) when the ingress broker is sending too many messages, instead of 500 (internal server error).

Docs

[Harwayne]

Can you import bundler.ErrOverflow rather than redefine it here?

[tayarani]

Done

[tayarani]

I'm not too familiar with the code yet, but this change doesn't seem like something easily testable in our unit tests. Is it ok for me to push this without tests? @Harwayne

[knative-test-reporter-robot]

The following jobs failed:

Test name	Triggers	Retries
pull-google-knative-gcp-integration-tests	2020-09-18 14:26:08.683 +0000 UTC	1/3

Automatically retrying due to test flakiness...
/test pull-google-knative-gcp-integration-tests

[Harwayne]

/lgtm

I'll leave approval to someone more familiar with the Broker area.

[grantr]

/cc @yolocs

[yolocs]

You can actually test this code. You can have a fake DecoupleSink implementation (https://github.com/google/knative-gcp/blob/master/pkg/broker/ingress/handler.go#L76) that always returns bundler.ErrOverflow on Send. Then you can check if the resp code is 429.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-google-knative-gcp-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/broker/ingress/handler.go	89.7%	89.8%	0.2

[yolocs]

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tayarani, yolocs

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:

• OWNERS [yolocs]

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