Messaging v1beta1 Types #2373
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /* | ||
| Copyright 2020 The Knative Authors | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1beta1 | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "knative.dev/pkg/apis" | ||
| "knative.dev/pkg/apis/duck" | ||
| duckv1 "knative.dev/pkg/apis/duck/v1" | ||
| ) | ||
|
|
||
| // +genduck | ||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
|
||
| // Channelable is a skeleton type wrapping Subscribable and Addressable in the manner we expect resource writers | ||
| // defining compatible resources to embed it. We will typically use this type to deserialize | ||
| // Channelable ObjectReferences and access their subscription and address data. This is not a real resource. | ||
| type Channelable struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| // Spec is the part where the Channelable fulfills the Subscribable contract. | ||
| Spec ChannelableSpec `json:"spec,omitempty"` | ||
|
|
||
| Status ChannelableStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // ChannelableSpec contains Spec of the Channelable object | ||
| type ChannelableSpec struct { | ||
| SubscribableSpec `json:",inline"` | ||
|
|
||
| // DeliverySpec contains options controlling the event delivery | ||
| // +optional | ||
| Delivery *DeliverySpec `json:"delivery,omitempty"` | ||
|
vaikas marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // ChannelableStatus contains the Status of a Channelable object. | ||
| type ChannelableStatus struct { | ||
| // inherits duck/v1 Status, which currently provides: | ||
| // * ObservedGeneration - the 'Generation' of the Service that was last processed by the controller. | ||
| // * Conditions - the latest available observations of a resource's current state. | ||
| duckv1.Status `json:",inline"` | ||
| // AddressStatus is the part where the Channelable fulfills the Addressable contract. | ||
| duckv1.AddressStatus `json:",inline"` | ||
| // Subscribers is populated with the statuses of each of the Channelable's subscribers. | ||
| SubscribableStatus `json:",inline"` | ||
| // DeadLetterChannel is set by the channel when it supports native error handling via a channel | ||
| // Failed messages are delivered here. | ||
| // +optional | ||
| DeadLetterChannel *corev1.ObjectReference `json:"deadLetterChannel,omitempty"` | ||
| } | ||
|
|
||
| var ( | ||
| // Verify Channelable resources meet duck contracts. | ||
| _ duck.Populatable = (*Channelable)(nil) | ||
| _ duck.Implementable = (*Channelable)(nil) | ||
| _ apis.Listable = (*Channelable)(nil) | ||
| ) | ||
|
|
||
| // Populate implements duck.Populatable | ||
| func (c *Channelable) Populate() { | ||
| c.Spec.SubscribableSpec = SubscribableSpec{ | ||
| // Populate ALL fields | ||
| Subscribers: []SubscriberSpec{{ | ||
| UID: "2f9b5e8e-deb6-11e8-9f32-f2801f1b9fd1", | ||
| Generation: 1, | ||
| SubscriberURI: apis.HTTP("call1"), | ||
| ReplyURI: apis.HTTP("sink2"), | ||
| }, { | ||
| UID: "34c5aec8-deb6-11e8-9f32-f2801f1b9fd1", | ||
| Generation: 2, | ||
| SubscriberURI: apis.HTTP("call2"), | ||
| ReplyURI: apis.HTTP("sink2"), | ||
| }}, | ||
| } | ||
| retry := int32(5) | ||
| linear := BackoffPolicyLinear | ||
| delay := "5s" | ||
| c.Spec.Delivery = &DeliverySpec{ | ||
| DeadLetterSink: &duckv1.Destination{ | ||
| Ref: &corev1.ObjectReference{ | ||
| Name: "aname", | ||
| }, | ||
| URI: &apis.URL{ | ||
| Scheme: "http", | ||
| Host: "test-error-domain", | ||
| }, | ||
| }, | ||
| Retry: &retry, | ||
| BackoffPolicy: &linear, | ||
| BackoffDelay: &delay, | ||
| } | ||
| c.Status = ChannelableStatus{ | ||
| AddressStatus: duckv1.AddressStatus{ | ||
| Address: &duckv1.Addressable{ | ||
| URL: &apis.URL{ | ||
| Scheme: "http", | ||
| Host: "test-domain", | ||
| }, | ||
| }, | ||
| }, | ||
| SubscribableStatus: SubscribableStatus{ | ||
| Subscribers: []SubscriberStatus{{ | ||
| UID: "2f9b5e8e-deb6-11e8-9f32-f2801f1b9fd1", | ||
| ObservedGeneration: 1, | ||
| Ready: corev1.ConditionTrue, | ||
| Message: "Some message", | ||
| }, { | ||
| UID: "34c5aec8-deb6-11e8-9f32-f2801f1b9fd1", | ||
| ObservedGeneration: 2, | ||
| Ready: corev1.ConditionFalse, | ||
| Message: "Some message", | ||
| }}, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // GetFullType implements duck.Implementable | ||
| func (s *Channelable) GetFullType() duck.Populatable { | ||
| return &Channelable{} | ||
| } | ||
|
|
||
| // GetListType implements apis.Listable | ||
| func (c *Channelable) GetListType() runtime.Object { | ||
| return &ChannelableList{} | ||
| } | ||
|
|
||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
|
||
| // ChannelableList is a list of Channelable resources. | ||
| type ChannelableList struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata"` | ||
|
|
||
| Items []Channelable `json:"items"` | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| /* | ||
| Copyright 2020 The Knative Authors | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1beta1 | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| "knative.dev/pkg/apis" | ||
| duckv1 "knative.dev/pkg/apis/duck/v1" | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| ) | ||
|
|
||
| func TestChannelableGetListType(t *testing.T) { | ||
| c := &Channelable{} | ||
| switch c.GetListType().(type) { | ||
| case *ChannelableList: | ||
| // expected | ||
| default: | ||
| t.Errorf("expected GetListType to return *ChannelableList, got %T", c.GetListType()) | ||
| } | ||
| } | ||
|
|
||
| func TestChannelablePopulate(t *testing.T) { | ||
| got := &Channelable{} | ||
|
|
||
| retry := int32(5) | ||
| linear := BackoffPolicyLinear | ||
| delay := "5s" | ||
| want := &Channelable{ | ||
| Spec: ChannelableSpec{ | ||
| SubscribableSpec: SubscribableSpec{ | ||
| Subscribers: []SubscriberSpec{{ | ||
| UID: "2f9b5e8e-deb6-11e8-9f32-f2801f1b9fd1", | ||
| Generation: 1, | ||
| SubscriberURI: apis.HTTP("call1"), | ||
| ReplyURI: apis.HTTP("sink2"), | ||
| }, { | ||
| UID: "34c5aec8-deb6-11e8-9f32-f2801f1b9fd1", | ||
| Generation: 2, | ||
| SubscriberURI: apis.HTTP("call2"), | ||
| ReplyURI: apis.HTTP("sink2"), | ||
| }}, | ||
| }, | ||
| Delivery: &DeliverySpec{ | ||
| DeadLetterSink: &duckv1.Destination{ | ||
| Ref: &corev1.ObjectReference{ | ||
| Name: "aname", | ||
| }, | ||
| URI: &apis.URL{ | ||
| Scheme: "http", | ||
| Host: "test-error-domain", | ||
| }, | ||
| }, | ||
| Retry: &retry, | ||
| BackoffPolicy: &linear, | ||
| BackoffDelay: &delay, | ||
| }, | ||
| }, | ||
|
|
||
| Status: ChannelableStatus{ | ||
| AddressStatus: duckv1.AddressStatus{ | ||
| Address: &duckv1.Addressable{ | ||
| URL: &apis.URL{ | ||
| Scheme: "http", | ||
| Host: "test-domain", | ||
| }, | ||
| }, | ||
| }, | ||
| SubscribableStatus: SubscribableStatus{ | ||
| Subscribers: []SubscriberStatus{{ | ||
| UID: "2f9b5e8e-deb6-11e8-9f32-f2801f1b9fd1", | ||
| ObservedGeneration: 1, | ||
| Ready: corev1.ConditionTrue, | ||
| Message: "Some message", | ||
| }, { | ||
| UID: "34c5aec8-deb6-11e8-9f32-f2801f1b9fd1", | ||
| ObservedGeneration: 2, | ||
| Ready: corev1.ConditionFalse, | ||
| Message: "Some message", | ||
| }}, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| got.Populate() | ||
|
|
||
| if diff := cmp.Diff(want, got); diff != "" { | ||
| t.Errorf("Unexpected difference (-want, +got): %v", diff) | ||
| } | ||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| Copyright 2020 The Knative Authors | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1beta1 | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| duckv1 "knative.dev/pkg/apis/duck/v1" | ||
| ) | ||
|
|
||
| // DeliverySpec contains the delivery options for event senders, | ||
| // such as channelable and source. | ||
| type DeliverySpec struct { | ||
| // DeadLetterSink is the sink receiving event that couldn't be sent to | ||
| // a destination. | ||
| // +optional | ||
| DeadLetterSink *duckv1.Destination `json:"deadLetterSink,omitempty"` | ||
|
vaikas marked this conversation as resolved.
|
||
|
|
||
| // Retry is the minimum number of retries the sender should attempt when | ||
| // sending an event before moving it to the dead letter sink. | ||
| // +optional | ||
| Retry *int32 `json:"retry,omitempty"` | ||
|
|
||
| // BackoffPolicy is the retry backoff policy (linear, exponential) | ||
| // +optional | ||
| BackoffPolicy *BackoffPolicyType `json:"backoffPolicy,omitempty"` | ||
|
|
||
| // BackoffDelay is the delay before retrying. | ||
| // More information on Duration format: https://www.ietf.org/rfc/rfc3339.txt | ||
| // | ||
| // For linear policy, backoff delay is the time interval between retries. | ||
| // For exponential policy , backoff delay is backoffDelay*2^<numberOfRetries> | ||
| // +optional | ||
| BackoffDelay *string `json:"backoffDelay,omitempty"` | ||
| } | ||
|
|
||
| // BackoffPolicyType is the type for backoff policies | ||
| type BackoffPolicyType string | ||
|
|
||
| const ( | ||
| // Linear backoff policy | ||
| BackoffPolicyLinear BackoffPolicyType = "linear" | ||
|
|
||
| // Exponential backoff policy | ||
| BackoffPolicyExponential BackoffPolicyType = "exponential" | ||
| ) | ||
|
|
||
| // DeliveryStatus contains the Status of an object supporting delivery options. | ||
| type DeliveryStatus struct { | ||
| // DeadLetterChannel is the reference to the native, platform specific channel | ||
| // where failed events are sent to. | ||
| // +optional | ||
| DeadLetterChannel *corev1.ObjectReference `json:"deadLetterChannel,omitempty"` | ||
|
Contributor
There was a problem hiding this comment. maybe the type should be api.URL.
Contributor
Author
There was a problem hiding this comment. In the status, it should be a reference to a Channel (well, really, Subscribable). That way you can Subscribe to the events that have failed. Make sense?
Contributor
There was a problem hiding this comment. I had the same thought until I realized that this really is a ref to a channel that the owning channel created. I am +1 on DeadLetterChannel
Contributor
There was a problem hiding this comment. Maybe this object also needs a deadlettersinkuri? If the resource has a delivery spec in the spec there is currently no place to show a resolved delivery deadleattersink. Or there are two kinds of status objects, one where the channel makes you a deadletter channel, and ones that use the resolved deadletter destination |
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If Delivery moved into Subscribable, then we can drop Channelable all together and a channel becomes
Addressable + Subscribable.
What do you think?
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.
make sense to me. Big 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.
Besides removing one duck type (which I kind of think adds clarity), are there other benefits to removing it? I think being able to operate on a Channel ducktype as a single object in the consuming code base is kind of nice (instead of operating on Addressable / Subscribable) separately.
Not against it, just not sure what the concrete benefits of it are.
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.
sorry, I meant that actually having the Channelable as a separate duck type adds clarity, not removing it. So, what are the benefits for removing it?
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.
Simplify the number of things, channelable does not add anything new, just the union of two other ducks
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.
But the code using it now needs to set up two watches if it cares about channels (one for addressable and one for subscribable) and extra serializations / etc.? Am I just confused here? Seems like it has downsides and I'm not seeing the upside of removing the duck.
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.
Given the discussion above about Delivery NOT moving to Subscribable (since it needs to be available per Subscription) but moving to SubscriberSpec and the fact that there's also a DeadLetterChannel in the Channel, I don't see this happening. But perhaps I'm missing something here? So it's not only Subscribable, Addressable but also has DeadLetterChannel in the status.