Skip to content

Commit

Permalink
fix sink crashloop when upgrading from 0.1 to 0.2
Browse files Browse the repository at this point in the history
The crashloop is due to 2 reasons:
1. We deprecated and removed the `params` field from the EventListener. So, when the reconciler tries to  unmarshal an old EventListener, it fails since it cannot parse the deleted `params` field.

2. The new event listener sink image expects `/etc/config-logging` to be mounted. However the reconciler code does not add the `volumeMount` for any existing sink deployments. So, the new pods go into a crashloop

To fix this, this commit does the following:
1. Add back the `params` field. Also, update the mutating webhook to delete any set `params` field in existing eventlisteners.
2. Add volumeMount for existing deployments if not present.

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
  • Loading branch information
dibyom committed Jan 22, 2020
1 parent 4ca05e0 commit 573f906
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 2 deletions.
7 changes: 7 additions & 0 deletions pkg/apis/triggers/v1alpha1/event_listener_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func (el *EventListener) SetDefaults(ctx context.Context) {
t := &el.Spec.Triggers[i]
upgradeBinding(t)
upgradeInterceptor(t)
removeParams(t)
}
}
}
Expand Down Expand Up @@ -41,3 +42,9 @@ func upgradeInterceptor(t *EventListenerTrigger) {
t.DeprecatedInterceptor = nil
}
}

func removeParams(t *EventListenerTrigger) {
if t.DeprecatedParams != nil {
t.DeprecatedParams = nil
}
}
25 changes: 23 additions & 2 deletions pkg/apis/triggers/v1alpha1/event_listener_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
)

Expand Down Expand Up @@ -141,8 +142,28 @@ func TestEventListenerSetDefaults(t *testing.T) {
},
},
wc: v1alpha1.WithUpgradeViaDefaulting,
},
}
}, {
name: "with upgrade context - deprecated params",
in: &v1alpha1.EventListener{
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
DeprecatedParams: []pipelinev1.Param{{
Name: "param-name",
Value: pipelinev1.ArrayOrString{
Type: "string",
StringVal: "static",
},
},
}},
}},
},
want: &v1alpha1.EventListener{
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{}},
},
},
wc: v1alpha1.WithUpgradeViaDefaulting,
}}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := tc.in
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/triggers/v1alpha1/event_listener_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ type EventListenerTrigger struct {

// TODO(#248): Remove this before 0.3 release.
DeprecatedBinding *EventListenerBinding `json:"binding,omitempty"`

// TODO(#): Remove this before 0.3 release
// DEPRECATED: Use TriggerBindings with static values instead
DeprecatedParams []pipelinev1.Param `json:"params,omitempty"`
}

// EventInterceptor provides a hook to intercept and pre-process events
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/triggers/v1alpha1/zz_generated.deepcopy.go

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

4 changes: 4 additions & 0 deletions pkg/reconciler/v1alpha1/eventlistener/eventlistener.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,10 @@ func (c *Reconciler) reconcileDeployment(el *v1alpha1.EventListener) error {
existingDeployment.Spec.Template.Spec.Containers[0].Command = nil
updated = true
}
if len(existingDeployment.Spec.Template.Spec.Containers[0].VolumeMounts) == 0 {
existingDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = container.VolumeMounts
updated = true
}
}
if updated {
if _, err := c.KubeClientSet.AppsV1().Deployments(el.Namespace).Update(existingDeployment); err != nil {
Expand Down
18 changes: 18 additions & 0 deletions pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ func Test_reconcileDeployment(t *testing.T) {
eventListener4.Spec.ServiceAccountName = updatedSa

var replicas int32 = 1
// deployment1 == initial deployment
deployment1 := &appsv1.Deployment{
ObjectMeta: generateObjectMeta(eventListener0),
Spec: appsv1.DeploymentSpec{
Expand Down Expand Up @@ -329,18 +330,23 @@ func Test_reconcileDeployment(t *testing.T) {
},
}

// deployment 2 == initial deployment + labels from eventListener
deployment2 := deployment1.DeepCopy()
deployment2.Labels = mergeLabels(generatedLabels, updateLabel)
deployment2.Spec.Selector.MatchLabels = mergeLabels(generatedLabels, updateLabel)
deployment2.Spec.Template.Labels = mergeLabels(generatedLabels, updateLabel)

// deployment 3 == initial deployment + updated replicas
deployment3 := deployment1.DeepCopy()
var updateReplicas int32 = 5
deployment3.Spec.Replicas = &updateReplicas

deployment4 := deployment1.DeepCopy()
deployment4.Spec.Template.Spec.ServiceAccountName = updatedSa

deploymentMissingVolumeMount := deployment1.DeepCopy()
deploymentMissingVolumeMount.Spec.Template.Spec.Containers[0].VolumeMounts = nil

tests := []struct {
name string
startResources test.Resources
Expand Down Expand Up @@ -422,6 +428,18 @@ func Test_reconcileDeployment(t *testing.T) {
EventListeners: []*v1alpha1.EventListener{eventListener4},
Deployments: []*appsv1.Deployment{deployment4},
},
}, {
name: "eventlistener-config-volume-mount-update",
startResources: test.Resources{
Namespaces: []*corev1.Namespace{namespaceResource},
EventListeners: []*v1alpha1.EventListener{eventListener2},
Deployments: []*appsv1.Deployment{deploymentMissingVolumeMount},
},
endResources: test.Resources{
Namespaces: []*corev1.Namespace{namespaceResource},
EventListeners: []*v1alpha1.EventListener{eventListener2},
Deployments: []*appsv1.Deployment{deployment2},
},
},
}
for i := range tests {
Expand Down

0 comments on commit 573f906

Please sign in to comment.