-
Notifications
You must be signed in to change notification settings - Fork 630
Cluster scoped Bus #117
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.
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
Cluster scoped Bus #117
Changes from all commits
7142e3b
fcd4bb9
6511214
0aa0ad4
6195673
105b764
3c1be83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # Copyright 2018 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. | ||
| apiVersion: apiextensions.k8s.io/v1beta1 | ||
| kind: CustomResourceDefinition | ||
| metadata: | ||
| name: clusterbuses.channels.knative.dev | ||
| spec: | ||
| scope: Cluster | ||
| group: channels.knative.dev | ||
| version: v1alpha1 | ||
| names: | ||
| kind: ClusterBus | ||
| plural: clusterbuses | ||
| singular: clusterbus |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,15 +31,18 @@ import ( | |
| type Channel struct { | ||
| meta_v1.TypeMeta `json:",inline"` | ||
| meta_v1.ObjectMeta `json:"metadata"` | ||
| Spec ChannelSpec `json:"spec"` | ||
| Status *ChannelStatus `json:"status,omitempty"` | ||
| Spec ChannelSpec `json:"spec"` | ||
| Status ChannelStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // ChannelSpec (what the user wants) for a channel | ||
| type ChannelSpec struct { | ||
|
|
||
| // Name of the bus backing this channel (optional) | ||
| Bus string `json:"bus` | ||
| // Bus name of the bus backing this channel (mutually exclusive with ClusterBus) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it would be cleaner to always include the namespace for the bus in the channel and remove clusterBus here. Or add a busNamespace property to signal which namespace we are referring to. I have seen this clusterFoo and Foo pattern before and I have always felt like it complicates the understanding of the object over the wire...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my understanding, a ClusterBus is not the same thing as a (namespaced) Bus in a namespace different from the channel namespace. At least in usage intention. It is a Bus that is widely available for anyone to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's a good way to use the lack of an explicit namespace to mean cluster scoped. Users are not used to having to explicitly state the namespace - this will complicate templates/charts/etc. I would expect a name with no explicit namespace to mean 'my namespace', not cluster scope.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @pmorie here. The alternative implementation would be to do something like: spec:
bus:
type: cluster
name: pubsubThe trade off is a single field in config that is set, and the user has to know which field to use, or two nested fields that both must be set. I don't have a strong opinion as to which model is best.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh nice, I like this idea ^, it gets us closer to label selector style syntax. I could be alone, but it bothers me when there are mutually exclusive fields
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm willing to go with the group consensus here. |
||
| Bus string `json:"bus"` | ||
|
|
||
| // ClusterBus name of the clusterbus backing this channel (mutually exclusive with Bus) | ||
| ClusterBus string `json:"clusterBus"` | ||
|
|
||
| // Arguments configuration arguments for the channel | ||
| Arguments *[]Argument `json:"arguments,omitempty"` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * Copyright 2018 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 v1alpha1 | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
|
|
||
| meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // +genclient | ||
| // +genclient:noStatus | ||
| // +genclient:nonNamespaced | ||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
| // +k8s:defaulter-gen=true | ||
|
|
||
| // ClusterBus represents the clusterbuses.channels.knative.dev CRD | ||
| type ClusterBus struct { | ||
| meta_v1.TypeMeta `json:",inline"` | ||
| meta_v1.ObjectMeta `json:"metadata"` | ||
| Spec ClusterBusSpec `json:"spec"` | ||
| Status ClusterBusStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // ClusterBusSpec (what the user wants) for a clusterbus | ||
| type ClusterBusSpec = BusSpec | ||
|
|
||
| // ClusterBusStatus (computed) for a clusterbus | ||
| type ClusterBusStatus struct { | ||
| } | ||
|
|
||
| func (b *ClusterBus) BacksChannel(channel *Channel) bool { | ||
| return len(b.Namespace) == 0 && b.Name == channel.Spec.ClusterBus | ||
| } | ||
|
|
||
| func (b *ClusterBus) GetSpec() *BusSpec { | ||
| return &b.Spec | ||
| } | ||
|
|
||
| func (b *ClusterBus) GetSpecJSON() ([]byte, error) { | ||
| return json.Marshal(b.Spec) | ||
| } | ||
|
|
||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
|
||
| // ClusterBusList returned in list operations | ||
| type ClusterBusList struct { | ||
| meta_v1.TypeMeta `json:",inline"` | ||
| meta_v1.ListMeta `json:"metadata"` | ||
| Items []ClusterBus `json:"items"` | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@pmorie do you have a better pattern for working with cluster and namespaced resources in a similar way?
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 their specs are going to be the exact same, this seems better than what I've had to do in the past :)