Skip to content
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

Problem upgrading to v0.3.0 #459

Closed
ncskier opened this issue Feb 27, 2020 · 4 comments · Fixed by #461
Closed

Problem upgrading to v0.3.0 #459

ncskier opened this issue Feb 27, 2020 · 4 comments · Fixed by #461
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@ncskier
Copy link
Member

ncskier commented Feb 27, 2020

Expected Behavior

I expect to be able to upgrade to v0.3.0 without changing my EventListener.

Actual Behavior

When upgrading to v0.3.0, the EventListener no longer works properly. When it receives an event, there seem to be errors extracting event data.

Here's an example error:

Get error when upgrading: {"level":"error","logger":"eventlistener","caller":"sink/sink.go:153","msg":"couldn't create resource with group version kind \"tekton.dev/v1alpha1, Resource=pipelineresources\": namespaces \"$(params.webhooks-tekton-target-namespace)\" not found","knative.dev/controller":"eventlistener","/triggers-eventid":"g8pzc","/trigger":"ghe-test-triggers-3-push-event","stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:153\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:95"}

I think that I also saw a case when the $(params.example) were not filled in the TriggerTemplate.

I've found that these errors go away after deleting and recreating the EventListener resource. However, we have not announced this as a breaking change. Also, I think that we should fix this unless there's a good reason why it is breaking.

Steps to Reproduce the Problem

  1. Install Triggers v0.2.1
  2. Create a working EventListener + TriggerBinding + TriggerTemplate with parameters being extracted from the event.
  3. Install Triggers v0.3.0
  4. Send an HTTP event to the EventListener and see if everything works properly
@ncskier ncskier added the kind/bug Categorizes issue or PR as related to a bug. label Feb 27, 2020
@akihikokuroda
Copy link
Member

/assign

@akihikokuroda
Copy link
Member

Issue is missing b.kind in the TriggerBindings

dibyom added a commit to dibyom/triggers that referenced this issue Feb 27, 2020
When upgrading from 0.2.1 to 0.3, the Sink might have
to process a Binding that has no `Kind` set. This is
because, we only persist the default Kind when the Binding
object is created/updated. The reconciler also sets the defaults
by calling `SetDefaults` but that change is not persisted to etcd.

In the future, we might want to consider updating the object from the
reconciler itself if any default values were set.

Fixes tektoncd#459

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
@dibyom
Copy link
Member

dibyom commented Feb 27, 2020

Yeah, we set the default in the reconciler but never persist it. And the ResolveTrigger function in the sink expects a Kind to be set

dibyom added a commit to dibyom/triggers that referenced this issue Feb 27, 2020
When upgrading from 0.2.1 to 0.3, the Sink might have
to process a Binding that has no `Kind` set. This is
because, we only persist the default Kind when the Binding
object is created/updated. The reconciler also sets the defaults
by calling `SetDefaults` but that change is not persisted to etcd.

In the future, we might want to consider updating the object from the
reconciler itself if any default values were set.

Fixes: tektoncd#459

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
@akihikokuroda
Copy link
Member

/unassign

dibyom added a commit to dibyom/triggers that referenced this issue Feb 28, 2020
When upgrading from 0.2.1 to 0.3, the Sink might have
to process a Binding that has no `Kind` set. This is
because, we only persist the default Kind when the Binding
object is created/updated. The reconciler also sets the defaults
by calling `SetDefaults` but that change is not persisted to etcd.

In the future, we might want to consider updating the object from the
reconciler itself if any default values were set.

Fixes: tektoncd#459

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
tekton-robot pushed a commit that referenced this issue Feb 28, 2020
When upgrading from 0.2.1 to 0.3, the Sink might have
to process a Binding that has no `Kind` set. This is
because, we only persist the default Kind when the Binding
object is created/updated. The reconciler also sets the defaults
by calling `SetDefaults` but that change is not persisted to etcd.

In the future, we might want to consider updating the object from the
reconciler itself if any default values were set.

Fixes: #459

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants