Deprecate PodTemplate and ServiceType in favour of Resource

tektoncd · Jan 16, 2021 · 5e1ff4e · 5e1ff4e
1 parent d5ba007
commit 5e1ff4e
Show file tree

Hide file tree

Showing 11 changed files with 183 additions and 42 deletions.
diff --git a/docs/eventlisteners.md b/docs/eventlisteners.md
@@ -173,6 +173,8 @@ rules:
 
 ### ServiceType
 
+The `serviceType` field is deprecated. Consider using `Resources` field.
+
 The `serviceType` field is optional. EventListener sinks are exposed via
 [Kubernetes Services](https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types).
 By default, the serviceType is `ClusterIP` which means any pods running in the
@@ -193,6 +195,8 @@ If you want to deploy more than one pod, you can specify the number to `replicas
 
 ### PodTemplate
 
+The `podTemplate` field is deprecated. Consider using `Resources` field.
+
 The `podTemplate` field is optional. A PodTemplate is specifications for 
 creating EventListener pod. A PodTemplate consists of:
 - `tolerations` - list of toleration which allows pods to schedule onto the nodes with matching taints.
@@ -219,10 +223,15 @@ Resource field helps to provide Kubernetes or custom resource information.
 For more info on the design refer [TEP-0008](https://github.com/tektoncd/community/blob/master/teps/0008-support-knative-service-for-triggers-eventlistener-pod.md)
 
 Right now the `resources` field is optional in order to support backward compatibility with original behavior of `podTemplate`, `serviceType` and `serviceAccountName` fieds.
-In the future, we plan to deprecate `serviceAccountName`, `serviceType` and `podTemplate` from the EventListener spec in favor of the `resources` field.
+In the future, we plan to remove `serviceType` and `podTemplate` from the EventListener spec in favor of the `resources` field.
 
 For now `resources` has support for `kubernetesResource` but later it will have a support for Custom CRD`(ex: Knative Service)` as `customResource`
 
+`kubernetesResource` have two fields
+* ServiceType
+* Spec(PodTemplateSpec)
+
+
 ```yaml
 spec:
   resources:

diff --git a/examples/eventlisteners/eventlistener-podtemplate.yaml b/examples/eventlisteners/eventlistener-podtemplate.yaml
@@ -5,9 +5,13 @@ metadata:
   name: listener-podtemplate
 spec:
   serviceAccountName: tekton-triggers-example-sa
-  podTemplate:
-    nodeSelector:
-      app: test
+  resources:
+    kubernetesResource:
+      spec:
+        template:
+          spec:
+            nodeSelector:
+              app: test
   triggers:
     - name: foo-trig
       bindings:

diff --git a/examples/eventlisteners/eventlistener-tolerations.yaml b/examples/eventlisteners/eventlistener-tolerations.yaml
@@ -5,12 +5,16 @@ metadata:
   name: listener-tolerations
 spec:
   serviceAccountName: tekton-triggers-example-sa
-  podTemplate:
-    tolerations:
-    - key: key
-      value: value
-      operator: Equal
-      effect: NoSchedule
+  resources:
+    kubernetesResource:
+      spec:
+        template:
+          spec:
+            tolerations:
+            - key: key
+              value: value
+              operator: Equal
+              effect: NoSchedule
   triggers:
     - name: foo-trig
       bindings:

diff --git a/pkg/apis/triggers/v1alpha1/event_listener_defaults.go b/pkg/apis/triggers/v1alpha1/event_listener_defaults.go
@@ -31,5 +31,38 @@ func (el *EventListener) SetDefaults(ctx context.Context) {
 			triggerSpecBindingArray(el.Spec.Triggers[i].Bindings).
 				defaultBindings()
 		}
+		// Remove Deprecated Resource Fields
+		// To be removed in a later release #904
+		el.Spec.updatePodTemplate()
+		el.Spec.updateServiceType()
+	}
+}
+
+// To be Removed in a later release #904
+func (spec *EventListenerSpec) updatePodTemplate() {
+	if spec.DeprecatedPodTemplate.NodeSelector != nil {
+		if spec.Resources.KubernetesResource == nil {
+			spec.Resources.KubernetesResource = &KubernetesResource{}
+		}
+		spec.Resources.KubernetesResource.Template.Spec.NodeSelector = spec.DeprecatedPodTemplate.NodeSelector
+		spec.DeprecatedPodTemplate.NodeSelector = nil
+	}
+	if spec.DeprecatedPodTemplate.Tolerations != nil {
+		if spec.Resources.KubernetesResource == nil {
+			spec.Resources.KubernetesResource = &KubernetesResource{}
+		}
+		spec.Resources.KubernetesResource.Template.Spec.Tolerations = spec.DeprecatedPodTemplate.Tolerations
+		spec.DeprecatedPodTemplate.Tolerations = nil
+	}
+}
+
+// To be Removed in a later release #904
+func (spec *EventListenerSpec) updateServiceType() {
+	if spec.DeprecatedServiceType != "" {
+		if spec.Resources.KubernetesResource == nil {
+			spec.Resources.KubernetesResource = &KubernetesResource{}
+		}
+		spec.Resources.KubernetesResource.ServiceType = spec.DeprecatedServiceType
+		spec.DeprecatedServiceType = ""
 	}
 }
diff --git a/pkg/apis/triggers/v1alpha1/event_listener_defaults_test.go b/pkg/apis/triggers/v1alpha1/event_listener_defaults_test.go
@@ -22,6 +22,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/ptr"
 )
 
@@ -86,6 +88,67 @@ func TestEventListenerSetDefaults(t *testing.T) {
 				Replicas: ptr.Int32(1),
 			},
 		},
+	}, {
+		name: "deprecate podTemplate nodeselector to resource",
+		in: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				DeprecatedPodTemplate: v1alpha1.PodTemplate{
+					NodeSelector: map[string]string{"beta.kubernetes.io/os": "linux"},
+				},
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Resources: v1alpha1.Resources{
+					KubernetesResource: &v1alpha1.KubernetesResource{
+						WithPodSpec: duckv1.WithPodSpec{
+							Template: duckv1.PodSpecable{
+								Spec: corev1.PodSpec{
+									NodeSelector: map[string]string{"beta.kubernetes.io/os": "linux"},
+								},
+							},
+						}},
+				},
+			},
+		},
+	}, {
+		name: "deprecate serviceType to resource",
+		in: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				DeprecatedServiceType: "NodePort",
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Resources: v1alpha1.Resources{
+					KubernetesResource: &v1alpha1.KubernetesResource{
+						ServiceType: "NodePort",
+					},
+				},
+			},
+		},
+	}, {
+		name: "deprecate podTemplate toleration to resource",
+		in: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				DeprecatedPodTemplate: v1alpha1.PodTemplate{Tolerations: []corev1.Toleration{{Key: "key"}}},
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Resources: v1alpha1.Resources{
+					KubernetesResource: &v1alpha1.KubernetesResource{
+						WithPodSpec: duckv1.WithPodSpec{
+							Template: duckv1.PodSpecable{
+								Spec: corev1.PodSpec{Tolerations: []corev1.Toleration{{Key: "key"}}},
+							},
+						}},
+				},
+			},
+		},
 	}, {
 		name: "different value for replicas other than 0",
 		in: &v1alpha1.EventListener{

diff --git a/pkg/apis/triggers/v1alpha1/event_listener_types.go b/pkg/apis/triggers/v1alpha1/event_listener_types.go
@@ -55,11 +55,13 @@ type EventListener struct {
 type EventListenerSpec struct {
 	ServiceAccountName string                 `json:"serviceAccountName"`
 	Triggers           []EventListenerTrigger `json:"triggers"`
-	ServiceType        corev1.ServiceType     `json:"serviceType,omitempty"`
-	Replicas           *int32                 `json:"replicas,omitempty"`
-	PodTemplate        PodTemplate            `json:"podTemplate,omitempty"`
-	NamespaceSelector  NamespaceSelector      `json:"namespaceSelector,omitempty"`
-	Resources          Resources              `json:"resources,omitempty"`
+	// To be removed in a later release #904
+	DeprecatedServiceType corev1.ServiceType `json:"serviceType,omitempty"`
+	Replicas              *int32             `json:"replicas,omitempty"`
+	// To be removed in a later release #904
+	DeprecatedPodTemplate PodTemplate       `json:"podTemplate,omitempty"`
+	NamespaceSelector     NamespaceSelector `json:"namespaceSelector,omitempty"`
+	Resources             Resources         `json:"resources,omitempty"`
 }
 
 type Resources struct {

diff --git a/pkg/apis/triggers/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/triggers/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go
@@ -156,7 +156,7 @@ func reconcileObjectMeta(existing *metav1.ObjectMeta, desired metav1.ObjectMeta)
 
 func (r *Reconciler) reconcileService(ctx context.Context, logger *zap.SugaredLogger, el *v1alpha1.EventListener) error {
 	// for backward compatibility with original behavior
-	var serviceType = el.Spec.ServiceType
+	var serviceType corev1.ServiceType
 	if el.Spec.Resources.KubernetesResource != nil && el.Spec.Resources.KubernetesResource.ServiceType != "" {
 		serviceType = el.Spec.Resources.KubernetesResource.ServiceType
 	}
@@ -371,9 +371,6 @@ func getDeployment(el *v1alpha1.EventListener, c Config) *appsv1.Deployment {
 	)
 	podlabels = mergeMaps(el.Labels, GenerateResourceLabels(el.Name, c.StaticResourceLabels))
 
-	// For backward compatibility with podTemplate, serviceAccountName field as part of eventlistener.
-	tolerations = el.Spec.PodTemplate.Tolerations
-	nodeSelector = el.Spec.PodTemplate.NodeSelector
 	serviceAccountName = el.Spec.ServiceAccountName
 
 	vol := []corev1.Volume{{

diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go
@@ -491,15 +491,33 @@ func TestReconcile(t *testing.T) {
 	})
 
 	elWithNodePortServiceType := makeEL(withStatus, func(el *v1alpha1.EventListener) {
-		el.Spec.ServiceType = corev1.ServiceTypeNodePort
+		el.Spec.Resources.KubernetesResource = &v1alpha1.KubernetesResource{
+			ServiceType: corev1.ServiceTypeNodePort,
+		}
 	})
 
 	elWithTolerations := makeEL(withStatus, func(el *v1alpha1.EventListener) {
-		el.Spec.PodTemplate.Tolerations = updateTolerations
+		el.Spec.Resources.KubernetesResource = &v1alpha1.KubernetesResource{
+			WithPodSpec: duckv1.WithPodSpec{
+				Template: duckv1.PodSpecable{
+					Spec: corev1.PodSpec{
+						Tolerations: updateTolerations,
+					},
+				},
+			},
+		}
 	})
 
 	elWithNodeSelector := makeEL(withStatus, func(el *v1alpha1.EventListener) {
-		el.Spec.PodTemplate.NodeSelector = updateNodeSelector
+		el.Spec.Resources.KubernetesResource = &v1alpha1.KubernetesResource{
+			WithPodSpec: duckv1.WithPodSpec{
+				Template: duckv1.PodSpecable{
+					Spec: corev1.PodSpec{
+						NodeSelector: updateNodeSelector,
+					},
+				},
+			},
+		}
 	})
 
 	elWithReplicas := makeEL(withStatus, func(el *v1alpha1.EventListener) {

diff --git a/test/builder/eventlistener.go b/test/builder/eventlistener.go
@@ -109,7 +109,7 @@ func EventListenerReplicas(replicas int32) EventListenerSpecOp {
 // EventListenerPodTemplate sets the specified pod template of the EventListener.
 func EventListenerPodTemplate(podTemplate v1alpha1.PodTemplate) EventListenerSpecOp {
 	return func(spec *v1alpha1.EventListenerSpec) {
-		spec.PodTemplate = podTemplate
+		spec.DeprecatedPodTemplate = podTemplate
 	}
 }
 

diff --git a/test/eventlistener_scale_test.go b/test/eventlistener_scale_test.go
@@ -24,10 +24,11 @@ import (
 	"testing"
 
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
-	bldr "github.com/tektoncd/triggers/test/builder"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+	"knative.dev/pkg/ptr"
 	knativetest "knative.dev/pkg/test"
 )
 
@@ -47,24 +48,34 @@ func TestEventListenerScale(t *testing.T) {
 	createServiceAccount(t, c, namespace, saName)
 	// Create an EventListener with 1000 Triggers
 	var err error
-	el := bldr.EventListener("my-eventlistener", namespace, bldr.EventListenerSpec(
-		bldr.EventListenerServiceAccount(saName),
-		bldr.EventListenerReplicas(3),
-		bldr.EventListenerPodTemplate(
-			bldr.EventListenerPodTemplateSpec(
-				bldr.EventListenerPodTemplateNodeSelector(map[string]string{"beta.kubernetes.io/os": "linux"}),
-				bldr.EventListenerPodTemplateTolerations([]corev1.Toleration{
-					{
-						Key:      "key",
-						Operator: "Equal",
-						Value:    "value",
-						Effect:   "NoSchedule",
-					},
-				}),
-			),
-		),
-	))
 
+	el := &triggersv1.EventListener{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-eventlistener",
+			Namespace: namespace,
+		},
+		Spec: triggersv1.EventListenerSpec{
+			ServiceAccountName: saName,
+			Replicas:           ptr.Int32(3),
+			Resources: triggersv1.Resources{
+				KubernetesResource: &triggersv1.KubernetesResource{
+					WithPodSpec: duckv1.WithPodSpec{
+						Template: duckv1.PodSpecable{
+							Spec: corev1.PodSpec{
+								NodeSelector: map[string]string{"beta.kubernetes.io/os": "linux"},
+								Tolerations: []corev1.Toleration{{
+									Key:      "key",
+									Operator: "Equal",
+									Value:    "value",
+									Effect:   "NoSchedule",
+								}},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
 	for i := 0; i < 1000; i++ {
 		trigger := triggersv1.EventListenerTrigger{
 			Bindings: []*triggersv1.EventListenerBinding{{