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

Pipelinerun controller creates duplicate events #2857

Closed
GregDritschler opened this issue Jun 24, 2020 · 3 comments · Fixed by #2860
Closed

Pipelinerun controller creates duplicate events #2857

GregDritschler opened this issue Jun 24, 2020 · 3 comments · Fixed by #2860
Assignees
Labels
area/testing Issues or PRs related to testing kind/bug Categorizes issue or PR as related to a bug. kind/flake Categorizes issue or PR as related to a flakey test

Comments

@GregDritschler
Copy link
Contributor

Expected Behavior

When an event occurs a single time, the event count is 1.

Actual Behavior

The event count is 2.

We have e2e tests that verify event counts. This bug causes them to fail. Here is one example:

https://tekton-releases.appspot.com/build/tekton-prow/pr-logs/pull/tektoncd_pipeline/2850/pull-tekton-pipeline-integration-tests/1275691939903049728/

    TestPipelineRun/pipeline_succeeds_when_task_skipped_due_to_failed_condition: pipelinerun_test.go:214: Expected 1 number of successful events from pipelinerun and taskrun but got 2; list of receieved events : "&v1.Event{TypeMeta:v1.TypeMeta{Kind:\"\", APIVersion:\"\"}, ObjectMeta:v1.ObjectMeta{Name:\"pipelinerun2.161b6ac6ae32bdbe\", GenerateName:\"\", Namespace:\"arendelle-2hhc6\", SelfLink:\"/api/v1/namespaces/arendelle-2hhc6/events/pipelinerun2.161b6ac6ae32bdbe\", UID:\"b0eb6536-beb7-4e47-bd41-1dc33675eaca\", ResourceVersion:\"2255\", Generation:0, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581095, loc:(*time.Location)(0x39d65e0)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ClusterName:\"\", ManagedFields:[]v1.ManagedFieldsEntry(nil)}, InvolvedObject:v1.ObjectReference{Kind:\"PipelineRun\", Namespace:\"arendelle-2hhc6\", Name:\"pipelinerun2\", UID:\"3afc45c6-2e3f-4736-b1ee-6cc0af5feed4\", APIVersion:\"tekton.dev/v1beta1\", ResourceVersion:\"5184\", FieldPath:\"\"}, Reason:\"Succeeded\", Message:\"Tasks Completed: 0 (Failed: 0, Cancelled 0), Skipped: 2\", Source:v1.EventSource{Component:\"PipelineRun\", Host:\"\"}, FirstTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581094, loc:(*time.Location)(0x39d65e0)}}, LastTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581094, loc:(*time.Location)(0x39d65e0)}}, Count:1, Type:\"Normal\", EventTime:v1.MicroTime{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, Series:(*v1.EventSeries)(nil), Action:\"\", Related:(*v1.ObjectReference)(nil), ReportingController:\"\", ReportingInstance:\"\"}, &v1.Event{TypeMeta:v1.TypeMeta{Kind:\"\", APIVersion:\"\"}, ObjectMeta:v1.ObjectMeta{Name:\"pipelinerun2.161b6ac6ae32bdbe\", GenerateName:\"\", Namespace:\"arendelle-2hhc6\", SelfLink:\"/api/v1/namespaces/arendelle-2hhc6/events/pipelinerun2.161b6ac6ae32bdbe\", UID:\"b0eb6536-beb7-4e47-bd41-1dc33675eaca\", ResourceVersion:\"2256\", Generation:0, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581095, loc:(*time.Location)(0x39d65e0)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ClusterName:\"\", ManagedFields:[]v1.ManagedFieldsEntry(nil)}, InvolvedObject:v1.ObjectReference{Kind:\"PipelineRun\", Namespace:\"arendelle-2hhc6\", Name:\"pipelinerun2\", UID:\"3afc45c6-2e3f-4736-b1ee-6cc0af5feed4\", APIVersion:\"tekton.dev/v1beta1\", ResourceVersion:\"5184\", FieldPath:\"\"}, Reason:\"Succeeded\", Message:\"Tasks Completed: 0 (Failed: 0, Cancelled 0), Skipped: 2\", Source:v1.EventSource{Component:\"PipelineRun\", Host:\"\"}, FirstTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581094, loc:(*time.Location)(0x39d65e0)}}, LastTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63728581094, loc:(*time.Location)(0x39d65e0)}}, Count:2, Type:\"Normal\", EventTime:v1.MicroTime{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, Series:(*v1.EventSeries)(nil), Action:\"\", Related:(*v1.ObjectReference)(nil), ReportingController:\"\", ReportingInstance:\"\"}"

Steps to Reproduce the Problem

The problem can be seen with any PipelineRun but it is intermittent (perhaps because the event broadcaster is catching the duplicate?).

Additional Info

There are 2 calls to events.Emit, one at the end of the inner reconcile function (line 455) and another at the end of the outer reconcile function (line 210). This produces the same event twice. This bug may have been introduced in PR #2805.

  • Kubernetes version: 1.16

  • Tekton Pipeline version: Unreleased (master)

@vdemeester
Copy link
Member

/kind bug
/kind flake
/area testing

/cc @afrittoli

@tekton-robot tekton-robot added kind/bug Categorizes issue or PR as related to a bug. kind/flake Categorizes issue or PR as related to a flakey test area/testing Issues or PRs related to testing labels Jun 24, 2020
@afrittoli
Copy link
Member

Because the time stamps are identical, my impression is that something goes wrong in the fake recorder. I don't see how the controller could send the event twice with the same timestamps otherwise.

I've started looking into this, I'll try to see if this is something we can address in the test code.

@afrittoli afrittoli self-assigned this Jun 24, 2020
@afrittoli
Copy link
Member

Because the time stamps are identical, my impression is that something goes wrong in the fake recorder. I don't see how the controller could send the event twice with the same timestamps otherwise.

I've started looking into this, I'll try to see if this is something we can address in the test code.

never mind, I didn't read the issue properly - thank you for catching this!

afrittoli added a commit to afrittoli/pipeline that referenced this issue Jun 24, 2020
When finishReconcileUpdateEmitEvents was introduced in PR tektoncd#2805,
I forgot to remove one of the original event.Emit, so one event
was sent twice. The fake recorder sometimes probably recognised
the duplicate and filtered it out, causing flackiness in tests,
as unit tests expect a fixed number of events.

Thanks @GregDritschler for troubleshooting this!

Fixes: tektoncd#2857
@afrittoli afrittoli mentioned this issue Jun 24, 2020
4 tasks
afrittoli added a commit to afrittoli/pipeline that referenced this issue Jun 24, 2020
When finishReconcileUpdateEmitEvents was introduced in PR tektoncd#2805,
I forgot to remove one of the original event.Emit, so one event
was sent twice. The fake recorder sometimes probably recognised
the duplicate and filtered it out, causing flackiness in tests,
as unit tests expect a fixed number of events.

Thanks @GregDritschler for troubleshooting this!

Fixes: tektoncd#2857
tekton-robot pushed a commit that referenced this issue Jun 24, 2020
When finishReconcileUpdateEmitEvents was introduced in PR #2805,
I forgot to remove one of the original event.Emit, so one event
was sent twice. The fake recorder sometimes probably recognised
the duplicate and filtered it out, causing flackiness in tests,
as unit tests expect a fixed number of events.

Thanks @GregDritschler for troubleshooting this!

Fixes: #2857
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues or PRs related to testing kind/bug Categorizes issue or PR as related to a bug. kind/flake Categorizes issue or PR as related to a flakey test
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants