From e0e50341a747c24727b407fcff19548d9b1288bc Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Tue, 23 Oct 2018 16:00:28 -0400 Subject: [PATCH] Flatten ProvisionerReference ProvisionerReference contains a single required field. We can simplify the object model by moving the ObjectRef for a provisioner up one level. --- docs/spec/spec.md | 16 ++--- pkg/apis/eventing/v1alpha1/channel_types.go | 3 +- .../v1alpha1/channel_validation_test.go | 72 +++++++------------ .../v1alpha1/provisioner_reference.go | 29 -------- pkg/apis/eventing/v1alpha1/source_types.go | 2 +- .../v1alpha1/source_validation_test.go | 24 +++---- .../v1alpha1/zz_generated.deepcopy.go | 33 ++------- .../eventing/inmemory/channel/reconcile.go | 6 +- .../inmemory/channel/reconcile_test.go | 36 +++------- .../inmemory/clusterprovisioner/reconcile.go | 8 +-- .../clusterprovisioner/reconcile_test.go | 35 +++------ 11 files changed, 70 insertions(+), 194 deletions(-) delete mode 100644 pkg/apis/eventing/v1alpha1/provisioner_reference.go diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 2450d41cf5a..295ccdc015b 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -26,7 +26,7 @@ its subscribers._ | Field | Type | Description | Limitations | | ------------- | ---------------------------------- | -------------------------------------------------------------------------- | -------------------------------------- | -| provisioner\* | ProvisionerReference | The name of the provisioner to create the resources that back the Channel. | Immutable. | +| provisioner\* | ObjectReference | The name of the provisioner to create the resources that back the Channel. | Immutable. | | arguments | runtime.RawExtension (JSON object) | Arguments to be passed to the provisioner. | | | subscribers | ChannelSubscriberSpec[] | Information about subscriptions used to implement message forwarding. | Filled out by Subscription Controller. | @@ -125,9 +125,9 @@ or a Channel system that receives and delivers events._ #### Spec -| Field | Type | Description | Limitations | -| ---------- | ---------------------------------- | ------------------------------------------------------------------------------------------------- | ---------------- | -| parameters | runtime.RawExtension (JSON object) | Description of the arguments able to be passed by the provisioned resource (not enforced in 0.1). | JSON Schema | +| Field | Type | Description | Limitations | +| ---------- | ---------------------------------- | ------------------------------------------------------------------------------------------------- | ----------- | +| parameters | runtime.RawExtension (JSON object) | Description of the arguments able to be passed by the provisioned resource (not enforced in 0.1). | JSON Schema | \*: Required @@ -150,14 +150,6 @@ or a Channel system that receives and delivers events._ ## Shared Object Schema -### ProvisionerReference - -| Field | Type | Description | Limitations | -| ----- | --------------- | ----------- | ----------- | -| ref\* | ObjectReference | | | - -\*: Required - ### EndpointSpec | Field | Type | Description | Limitations | diff --git a/pkg/apis/eventing/v1alpha1/channel_types.go b/pkg/apis/eventing/v1alpha1/channel_types.go index d5122b08344..162213e1208 100644 --- a/pkg/apis/eventing/v1alpha1/channel_types.go +++ b/pkg/apis/eventing/v1alpha1/channel_types.go @@ -64,8 +64,7 @@ type ChannelSpec struct { Generation int64 `json:"generation,omitempty"` // Provisioner defines the name of the Provisioner backing this channel. - // TODO: +optional If missing, a default Provisioner may be selected for the Channel. - Provisioner *ProvisionerReference `json:"provisioner,omitempty"` + Provisioner *corev1.ObjectReference `json:"provisioner,omitempty"` // Arguments defines the arguments to pass to the Provisioner which provisions // this Channel. diff --git a/pkg/apis/eventing/v1alpha1/channel_validation_test.go b/pkg/apis/eventing/v1alpha1/channel_validation_test.go index be8b16e2978..cd229348e8e 100644 --- a/pkg/apis/eventing/v1alpha1/channel_validation_test.go +++ b/pkg/apis/eventing/v1alpha1/channel_validation_test.go @@ -33,10 +33,8 @@ func TestChannelValidation(t *testing.T) { name: "valid", cr: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, @@ -51,10 +49,8 @@ func TestChannelValidation(t *testing.T) { name: "subscribers array", cr: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{{ @@ -68,10 +64,8 @@ func TestChannelValidation(t *testing.T) { name: "empty subscriber at index 1", cr: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{{ @@ -89,10 +83,8 @@ func TestChannelValidation(t *testing.T) { name: "2 empty subscribers", cr: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{{}, {}}, @@ -124,10 +116,8 @@ func TestChannelImmutableFields(t *testing.T) { name: "good (new)", new: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, @@ -137,19 +127,15 @@ func TestChannelImmutableFields(t *testing.T) { name: "good (no change)", new: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, old: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, @@ -158,10 +144,8 @@ func TestChannelImmutableFields(t *testing.T) { name: "good (arguments change)", new: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Arguments: &runtime.RawExtension{ Raw: []byte("\"foo\":\"bar\""), @@ -170,10 +154,8 @@ func TestChannelImmutableFields(t *testing.T) { }, old: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Arguments: &runtime.RawExtension{ Raw: []byte(`{"foo":"baz"}`), @@ -185,10 +167,8 @@ func TestChannelImmutableFields(t *testing.T) { name: "bad (not channel)", new: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, @@ -200,19 +180,15 @@ func TestChannelImmutableFields(t *testing.T) { name: "bad (provisioner changes)", new: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, old: &Channel{ Spec: ChannelSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "bar", - }, + Provisioner: &corev1.ObjectReference{ + Name: "bar", }, }, }, diff --git a/pkg/apis/eventing/v1alpha1/provisioner_reference.go b/pkg/apis/eventing/v1alpha1/provisioner_reference.go deleted file mode 100644 index 1d0bbca4cbe..00000000000 --- a/pkg/apis/eventing/v1alpha1/provisioner_reference.go +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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 ( - corev1 "k8s.io/api/core/v1" -) - -// ProvisionerReference defines the strategy for selecting a Provisioner for a -// provisioned resource. Shared by Channel and Source. -type ProvisionerReference struct { - // A reference to a specific Provisioner. - //TODO: +optional add selector - Ref *corev1.ObjectReference `json:"ref,omitempty"` -} diff --git a/pkg/apis/eventing/v1alpha1/source_types.go b/pkg/apis/eventing/v1alpha1/source_types.go index 5b58b416b95..519b8b868ab 100644 --- a/pkg/apis/eventing/v1alpha1/source_types.go +++ b/pkg/apis/eventing/v1alpha1/source_types.go @@ -70,7 +70,7 @@ type SourceSpec struct { // Provisioner is used to create any backing resources and configuration. // +required - Provisioner *ProvisionerReference `json:"provisioner,omitempty"` + Provisioner *corev1.ObjectReference `json:"provisioner,omitempty"` // Arguments defines the arguments to pass to the Provisioner which provisions // this Source. diff --git a/pkg/apis/eventing/v1alpha1/source_validation_test.go b/pkg/apis/eventing/v1alpha1/source_validation_test.go index 212a24ad9a3..ac11bc0e4a8 100644 --- a/pkg/apis/eventing/v1alpha1/source_validation_test.go +++ b/pkg/apis/eventing/v1alpha1/source_validation_test.go @@ -35,10 +35,8 @@ func TestSourceValidate(t *testing.T) { name: "minimum valid", cr: &Source{ Spec: SourceSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, }, }, @@ -46,10 +44,8 @@ func TestSourceValidate(t *testing.T) { name: "full valid", cr: &Source{ Spec: SourceSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Arguments: &runtime.RawExtension{ Raw: []byte(`{"foo":"baz"}`), @@ -65,10 +61,8 @@ func TestSourceValidate(t *testing.T) { name: "invalid, set extra channel parameters", cr: &Source{ Spec: SourceSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Channel: &corev1.ObjectReference{ Name: "bar", @@ -87,10 +81,8 @@ func TestSourceValidate(t *testing.T) { name: "invalid, set extra channel as not a channel", cr: &Source{ Spec: SourceSpec{ - Provisioner: &ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "foo", - }, + Provisioner: &corev1.ObjectReference{ + Name: "foo", }, Channel: &corev1.ObjectReference{ Name: "backwards", diff --git a/pkg/apis/eventing/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1alpha1/zz_generated.deepcopy.go index dcbd9410622..af77b21c691 100644 --- a/pkg/apis/eventing/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1alpha1/zz_generated.deepcopy.go @@ -129,8 +129,8 @@ func (in *ChannelSpec) DeepCopyInto(out *ChannelSpec) { if *in == nil { *out = nil } else { - *out = new(ProvisionerReference) - (*in).DeepCopyInto(*out) + *out = new(v1.ObjectReference) + **out = **in } } if in.Arguments != nil { @@ -290,31 +290,6 @@ func (in *ClusterProvisionerStatus) DeepCopy() *ClusterProvisionerStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProvisionerReference) DeepCopyInto(out *ProvisionerReference) { - *out = *in - if in.Ref != nil { - in, out := &in.Ref, &out.Ref - if *in == nil { - *out = nil - } else { - *out = new(v1.ObjectReference) - **out = **in - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProvisionerReference. -func (in *ProvisionerReference) DeepCopy() *ProvisionerReference { - if in == nil { - return nil - } - out := new(ProvisionerReference) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResultStrategy) DeepCopyInto(out *ResultStrategy) { *out = *in @@ -409,8 +384,8 @@ func (in *SourceSpec) DeepCopyInto(out *SourceSpec) { if *in == nil { *out = nil } else { - *out = new(ProvisionerReference) - (*in).DeepCopyInto(*out) + *out = new(v1.ObjectReference) + **out = **in } } if in.Arguments != nil { diff --git a/pkg/controller/eventing/inmemory/channel/reconcile.go b/pkg/controller/eventing/inmemory/channel/reconcile.go index 439361e8978..df4c15e6103 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile.go @@ -241,7 +241,7 @@ func (r *reconciler) createVirtualService(ctx context.Context, c *eventingv1alph func newK8sService(c *eventingv1alpha1.Channel) *corev1.Service { labels := map[string]string{ "channel": c.Name, - "provisioner": c.Spec.Provisioner.Ref.Name, + "provisioner": c.Spec.Provisioner.Name, } return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -273,9 +273,9 @@ func newK8sService(c *eventingv1alpha1.Channel) *corev1.Service { func newVirtualService(channel *eventingv1alpha1.Channel) *istiov1alpha3.VirtualService { labels := map[string]string{ "channel": channel.Name, - "provisioner": channel.Spec.Provisioner.Ref.Name, + "provisioner": channel.Spec.Provisioner.Name, } - destinationHost := controller.ServiceHostName(controller.ClusterBusDispatcherServiceName(channel.Spec.Provisioner.Ref.Name), system.Namespace) + destinationHost := controller.ServiceHostName(controller.ClusterBusDispatcherServiceName(channel.Spec.Provisioner.Name), system.Namespace) return &istiov1alpha3.VirtualService{ ObjectMeta: metav1.ObjectMeta{ Name: controller.ChannelVirtualServiceName(channel.Name), diff --git a/pkg/controller/eventing/inmemory/channel/reconcile_test.go b/pkg/controller/eventing/inmemory/channel/reconcile_test.go index 13466bad4ae..adebef982f0 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile_test.go @@ -111,10 +111,8 @@ var ( Kind: "Channel", }, Spec: eventingv1alpha1.ChannelSpec{ - Provisioner: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: cpName, - }, + Provisioner: &corev1.ObjectReference{ + Name: cpName, }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{ @@ -141,10 +139,8 @@ var ( Kind: "Channel", }, Spec: eventingv1alpha1.ChannelSpec{ - Provisioner: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "some-other-provisioner", - }, + Provisioner: &corev1.ObjectReference{ + Name: "some-other-provisioner", }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{ @@ -164,10 +160,8 @@ var ( Kind: "Channel", }, Spec: eventingv1alpha1.ChannelSpec{ - Provisioner: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: cpName, - }, + Provisioner: &corev1.ObjectReference{ + Name: cpName, }, Channelable: &duckv1alpha1.Channelable{ Subscribers: []duckv1alpha1.ChannelSubscriberSpec{ @@ -225,7 +219,7 @@ func TestReconcile(t *testing.T) { { Name: "Channel not reconciled - nil ref", InitialState: []runtime.Object{ - makeChannelNilRef(), + makeChannelNilProvisioner(), }, }, { @@ -472,10 +466,8 @@ func makeChannel() *eventingv1alpha1.Channel { UID: cUID, }, Spec: eventingv1alpha1.ChannelSpec{ - Provisioner: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: cpName, - }, + Provisioner: &corev1.ObjectReference{ + Name: cpName, }, }, } @@ -508,21 +500,15 @@ func makeChannelNilProvisioner() *eventingv1alpha1.Channel { return c } -func makeChannelNilRef() *eventingv1alpha1.Channel { - c := makeChannel() - c.Spec.Provisioner.Ref = nil - return c -} - func makeChannelWithWrongProvisionerNamespace() *eventingv1alpha1.Channel { c := makeChannel() - c.Spec.Provisioner.Ref.Namespace = "wrong-namespace" + c.Spec.Provisioner.Namespace = "wrong-namespace" return c } func makeChannelWithWrongProvisionerName() *eventingv1alpha1.Channel { c := makeChannel() - c.Spec.Provisioner.Ref.Name = "wrong-name" + c.Spec.Provisioner.Name = "wrong-name" return c } diff --git a/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go b/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go index a11cb53ca2f..582ec3a6879 100644 --- a/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go +++ b/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile.go @@ -104,11 +104,11 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err } // IsControlled determines if the in-memory Channel Controller should control (and therefore -// reconcile) a given object, based on that object's ClusterProvisioner reference. kind is the kind +// reconcile) a given object, based on that object's provisioner reference. kind is the kind // of that object. -func IsControlled(ref *eventingv1alpha1.ProvisionerReference, kind string) bool { - if ref != nil && ref.Ref != nil { - return shouldReconcile(ref.Ref.Namespace, ref.Ref.Name, kind) +func IsControlled(ref *corev1.ObjectReference, kind string) bool { + if ref != nil { + return shouldReconcile(ref.Namespace, ref.Name, kind) } return false } diff --git a/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile_test.go b/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile_test.go index f0bd62dbdce..4084ab231d7 100644 --- a/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/clusterprovisioner/reconcile_test.go @@ -75,7 +75,7 @@ func TestInjectClient(t *testing.T) { func TestIsControlled(t *testing.T) { testCases := map[string]struct { - ref *eventingv1alpha1.ProvisionerReference + ref *corev1.ObjectReference kind string isControlled bool }{ @@ -84,46 +84,31 @@ func TestIsControlled(t *testing.T) { kind: "Channel", isControlled: false, }, - "ref nil": { - ref: &eventingv1alpha1.ProvisionerReference{ - Ref: nil, - }, - kind: "Channel", - isControlled: false, - }, "wrong namespace": { - ref: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Namespace: "other", - Name: Name, - }, + ref: &corev1.ObjectReference{ + Namespace: "other", + Name: Name, }, kind: "Channel", isControlled: false, }, "wrong name": { - ref: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: "other-name", - }, + ref: &corev1.ObjectReference{ + Name: "other-name", }, kind: "Channel", isControlled: false, }, "wrong kind": { - ref: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: Name, - }, + ref: &corev1.ObjectReference{ + Name: Name, }, kind: "Source", isControlled: false, }, "is controlled": { - ref: &eventingv1alpha1.ProvisionerReference{ - Ref: &corev1.ObjectReference{ - Name: Name, - }, + ref: &corev1.ObjectReference{ + Name: Name, }, kind: "Channel", isControlled: true,