Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Retire deadLetterChannel, use DeliveryStatus consistently #5746

Merged
merged 1 commit into from
Sep 23, 2021
Merged

Retire deadLetterChannel, use DeliveryStatus consistently #5746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/delivery/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ capabilities and are free to add more delivery options.

### Exposing underlying DLC

Channel implementation supporting dead letter channel should advertise it in
Channel implementation supporting dead letter channel should resolve it to a URI in
their status.

```go
Expand All @@ -133,7 +133,7 @@ type DeliveryStatus struct {
// DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
// where failed events are sent to.
// +optional
DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
}
```

Expand Down
77 changes: 61 additions & 16 deletions docs/eventing-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,24 @@ SubscribableStatus
</tr>
<tr>
<td>
<code>DeliveryStatus</code><br/>
<em>
<a href="#duck.knative.dev/v1.DeliveryStatus">
DeliveryStatus
</a>
</em>
</td>
<td>
<p>
(Members of <code>DeliveryStatus</code> are embedded into this type.)
</p>
<em>(Optional)</em>
<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
resolved delivery options.</p>
</td>
</tr>
<tr>
<td>
<code>deadLetterChannel</code><br/>
<em>
<a href="https://pkg.go.dev/knative.dev/pkg/apis/duck/v1#KReference">
Expand All @@ -273,7 +291,8 @@ knative.dev/pkg/apis/duck/v1.KReference
<td>
<em>(Optional)</em>
<p>DeadLetterChannel is a KReference and is set by the channel when it supports native error handling via a channel
Failed messages are delivered here.</p>
Failed messages are delivered here.
Deprecated in favor of DeliveryStatus, to be removed September 2022.</p>
</td>
</tr>
</tbody>
Expand Down Expand Up @@ -375,7 +394,10 @@ For exponential policy, backoff delay is backoffDelay*2^<numberOfRetries>.</p>
<h3 id="duck.knative.dev/v1.DeliveryStatus">DeliveryStatus
</h3>
<p>
<p>DeliveryStatus contains the Status of an object supporting delivery options.</p>
(<em>Appears on:</em><a href="#duck.knative.dev/v1.ChannelableStatus">ChannelableStatus</a>, <a href="#eventing.knative.dev/v1.BrokerStatus">BrokerStatus</a>, <a href="#eventing.knative.dev/v1.TriggerStatus">TriggerStatus</a>, <a href="#messaging.knative.dev/v1.SubscriptionStatusPhysicalSubscription">SubscriptionStatusPhysicalSubscription</a>)
</p>
<p>
<p>DeliveryStatus contains the Status of an object supporting delivery options. This type is intended to be embedded into a status struct.</p>
</p>
<table>
<thead>
Expand All @@ -387,16 +409,16 @@ For exponential policy, backoff delay is backoffDelay*2^<numberOfRetries>.</p>
<tbody>
<tr>
<td>
<code>deadLetterChannel</code><br/>
<code>deadLetterSinkUri</code><br/>
<em>
<a href="https://pkg.go.dev/knative.dev/pkg/apis/duck/v1#KReference">
knative.dev/pkg/apis/duck/v1.KReference
<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
knative.dev/pkg/apis.URL
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
<p>DeadLetterSink is a KReference that is the reference to the native, platform specific channel
where failed events are sent to.</p>
</td>
</tr>
Expand Down Expand Up @@ -1721,6 +1743,23 @@ knative.dev/pkg/apis/duck/v1.Addressable
delivered into the Broker mesh.</p>
</td>
</tr>
<tr>
<td>
<code>DeliveryStatus</code><br/>
<em>
<a href="#duck.knative.dev/v1.DeliveryStatus">
DeliveryStatus
</a>
</em>
</td>
<td>
<p>
(Members of <code>DeliveryStatus</code> are embedded into this type.)
</p>
<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
resolved delivery options.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="eventing.knative.dev/v1.TriggerFilter">TriggerFilter
Expand Down Expand Up @@ -1889,16 +1928,19 @@ knative.dev/pkg/apis.URL
</tr>
<tr>
<td>
<code>deadLetterSinkUri</code><br/>
<code>DeliveryStatus</code><br/>
<em>
<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
knative.dev/pkg/apis.URL
<a href="#duck.knative.dev/v1.DeliveryStatus">
DeliveryStatus
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>DeadLetterSinkURI is the resolved URI of the dead letter sink for this Trigger.</p>
<p>
(Members of <code>DeliveryStatus</code> are embedded into this type.)
</p>
<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
resolved delivery options.</p>
</td>
</tr>
</tbody>
Expand Down Expand Up @@ -3828,16 +3870,19 @@ knative.dev/pkg/apis.URL
</tr>
<tr>
<td>
<code>deadLetterSinkUri</code><br/>
<code>DeliveryStatus</code><br/>
<em>
<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
knative.dev/pkg/apis.URL
<a href="#duck.knative.dev/v1.DeliveryStatus">
DeliveryStatus
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>ReplyURI is the fully resolved URI for the spec.delivery.deadLetterSink.</p>
<p>
(Members of <code>DeliveryStatus</code> are embedded into this type.)
</p>
<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
resolved delivery options.</p>
</td>
</tr>
</tbody>
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/duck/v1/channelable_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,13 @@ type ChannelableStatus struct {
duckv1.AddressStatus `json:",inline"`
// Subscribers is populated with the statuses of each of the Channelable's subscribers.
SubscribableStatus `json:",inline"`
// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
// resolved delivery options.
// +optional
DeliveryStatus `json:",inline"`
// DeadLetterChannel is a KReference and is set by the channel when it supports native error handling via a channel
// Failed messages are delivered here.
// Deprecated in favor of DeliveryStatus, to be removed September 2022.
// +optional
DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/duck/v1/delivery_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,10 @@ const (
BackoffPolicyExponential BackoffPolicyType = "exponential"
)

// DeliveryStatus contains the Status of an object supporting delivery options.
// DeliveryStatus contains the Status of an object supporting delivery options. This type is intended to be embedded into a status struct.
type DeliveryStatus struct {
// DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
// DeadLetterSink is a KReference that is the reference to the native, platform specific channel
// where failed events are sent to.
// +optional
DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
}
9 changes: 5 additions & 4 deletions pkg/apis/duck/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 1 addition & 21 deletions pkg/apis/duck/v1beta1/delivery_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,4 @@ func (sink *DeliverySpec) ConvertFrom(ctx context.Context, from apis.Convertible
}
}

// ConvertTo implements apis.Convertible
func (source *DeliveryStatus) ConvertTo(ctx context.Context, to apis.Convertible) error {
switch sink := to.(type) {
case *eventingduckv1.DeliveryStatus:
sink.DeadLetterChannel = source.DeadLetterChannel
return nil
default:
return fmt.Errorf("unknown version, got: %T", sink)
}
}

// ConvertFrom implements apis.Convertible
func (sink *DeliveryStatus) ConvertFrom(ctx context.Context, from apis.Convertible) error {
switch source := from.(type) {
case *eventingduckv1.DeliveryStatus:
sink.DeadLetterChannel = source.DeadLetterChannel
return nil
default:
return fmt.Errorf("unknown version, got: %T", source)
}
}
// DeliveryStatus v1beta1 is not convertable to v1 (Channel ref type vs URL)
90 changes: 2 additions & 88 deletions pkg/apis/duck/v1beta1/delivery_conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/google/go-cmp/cmp"
v1 "knative.dev/eventing/pkg/apis/duck/v1"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
pkgduck "knative.dev/pkg/apis/duck/v1"
)

Expand All @@ -39,18 +38,6 @@ func TestDeliverySpecConversionBadType(t *testing.T) {
}
}

func TestDeliveryStatusConversionBadType(t *testing.T) {
good, bad := &DeliveryStatus{}, &DeliveryStatus{}

if err := good.ConvertTo(context.Background(), bad); err == nil {
t.Errorf("ConvertTo() = %#v, wanted error", bad)
}

if err := good.ConvertFrom(context.Background(), bad); err == nil {
t.Errorf("ConvertFrom() = %#v, wanted error", good)
}
}

// Test v1beta1 -> v1 -> v1beta1
func TestDeliverySpecConversion(t *testing.T) {
var retryCount int32 = 10
Expand Down Expand Up @@ -205,78 +192,5 @@ func TestDeliverySpecConversionV1(t *testing.T) {
}
}

// Test v1beta1 -> v1 -> v1beta1
func TestDeliveryStatusConversion(t *testing.T) {
tests := []struct {
name string
in *DeliveryStatus
err *string
}{{
name: "min configuration",
in: &DeliveryStatus{
DeadLetterChannel: &duckv1.KReference{
Kind: "dlKind",
Namespace: "dlNamespace",
Name: "dlName",
APIVersion: "dlAPIVersion",
},
},
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ver := &v1.DeliveryStatus{}
err := test.in.ConvertTo(context.Background(), ver)
if err != nil {
if test.err == nil || *test.err != err.Error() {
t.Error("ConvertTo() =", err)
}
return
}
got := &DeliveryStatus{}
if err := got.ConvertFrom(context.Background(), ver); err != nil {
t.Error("ConvertFrom() =", err)
}
if diff := cmp.Diff(test.in, got); diff != "" {
t.Error("roundtrip (-want, +got) =", diff)
}
})
}
}

// Test v1 -> v1beta1 -> v1
func TestDeliveryStatusConversionV1(t *testing.T) {
tests := []struct {
name string
in *v1.DeliveryStatus
err *string
}{{
name: "min configuration",
in: &v1.DeliveryStatus{
DeadLetterChannel: &duckv1.KReference{
Kind: "dlKind",
Namespace: "dlNamespace",
Name: "dlName",
APIVersion: "dlAPIVersion",
},
},
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ver := &DeliveryStatus{}
err := ver.ConvertFrom(context.Background(), test.in)
if err != nil {
if test.err == nil || *test.err != err.Error() {
t.Error("ConvertFrom() =", err)
}
return
}
got := &v1.DeliveryStatus{}
if err := ver.ConvertTo(context.Background(), got); err != nil {
t.Error("ConvertTo() =", err)
}
if diff := cmp.Diff(test.in, got); diff != "" {
t.Error("roundtrip (-want, +got) =", diff)
}
})
}
}
// v1beta1 and v1 DeliveryStatus are not convertable to each other.
// (channel ref vs apis.URL)
4 changes: 4 additions & 0 deletions pkg/apis/eventing/v1/broker_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ type BrokerStatus struct {
// delivered into the Broker mesh.
// +optional
Address duckv1.Addressable `json:"address,omitempty"`

// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
// resolved delivery options.
eventingduckv1.DeliveryStatus `json:",inline"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/eventing/v1/trigger_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ type TriggerStatus struct {
// +optional
SubscriberURI *apis.URL `json:"subscriberUri,omitempty"`

// DeadLetterSinkURI is the resolved URI of the dead letter sink for this Trigger.
// +optional
DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
// resolved delivery options.
eventingduckv1.DeliveryStatus `json:",inline"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
7 changes: 2 additions & 5 deletions pkg/apis/eventing/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading