Failures to send Replies are considered delivery failures

google / knative-gcp

GCP event implementations to use with Knative Eventing.

https://github.com/knative/eventing

Apache License 2.0

159 stars 75 forks source link

Failures to send Replies are considered delivery failures #2107

Closed Harwayne closed 3 years ago

Harwayne commented 3 years ago

Fixes #2106.

Proposed Changes

If the reply event is not successfully delivered, then the overall event delivery via the Trigger is considered a failure.
Add unit tests for successful replies.
Fix malformed CloudEvent reply failure unit test, which passed for the wrong reason.

Release Note

🐛 Failures to send a Trigger's reply back into the Broker are now retried.

knative-prow-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Harwayne

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](https://github.com/google/knative-gcp/blob/master/OWNERS)~~ [Harwayne] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

zhongduo commented 3 years ago

I hope the conformance test can capture it some day.

knative-metrics-robot commented 3 years ago

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/handler/processors/deliver/processor.go	76.1%	77.8%	1.6

zhongduo commented 3 years ago

/lgtm

Harwayne commented 3 years ago

@Harwayne: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command pull-google-knative-gcp-conformance-tests f49fba5 link /test pull-google-knative-gcp-conformance-tests Full PR test history. Your PR dashboard.

Message:MountVolume.SetUp failed for volume "default-token-lxzmb" : failed to sync secret cache: timed out waiting for the condition

/retest

Harwayne commented 3 years ago

/retest

Harwayne commented 3 years ago

MountVolume.SetUp failed for volume "default-token-qffpt" : failed to sync secret cache: timed out waiting for the condition

/retest

Harwayne commented 3 years ago

MountVolume.SetUp failed for volume "default-token-8ddw2" : failed to sync secret cache: timed out waiting for the condition

/retest

Harwayne commented 3 years ago

Error from server: error when creating "https://storage.googleapis.com/knative-nightly/net-istio/latest/net-istio.yaml": admission webhook "pilot.validation.istio.io" denied the request: unrecognized type PeerAuthentication

/retest