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

Old JobSink secrets lead to processing old events again while new events are lost #8323

Closed
tikr7 opened this issue Nov 11, 2024 · 4 comments · Fixed by #8331
Closed

Old JobSink secrets lead to processing old events again while new events are lost #8323

tikr7 opened this issue Nov 11, 2024 · 4 comments · Fixed by #8331
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@tikr7
Copy link

tikr7 commented Nov 11, 2024

First of all, I really like the new feature JobSink in Knative Eventing because it helps to execute long running processes while being fully compatible with the eventing stack!

Describe the bug
JobSink Secret mounted to the Job is not cleaned up after deleting the Job.
There is even no update on that Secret with new CloudEvent information.
This leads to processing old events again and the new event is lost.

Expected behavior
Either clean up Kubernetes Secret after Kubernetes Job has been deleted.
Or overwrite the existing Kubernetes Secret with the new CloudEvent before it gets executed.

To Reproduce
Lets assume, I trigger 3 Jobs via eventing (names of Jobs are somehow autogenerated):

job-sink-logger
job-sink-logger0
job-sink-logger0xgzzy

Lets clean them up (imagine we have 1000 of those):

kubectl delete job job-sink-logger job-sink-logger0 job-sink-logger0xgzzy

We still have the Secrets with the CloudEvent inside:

kubectl get secret
job-sink-logger
job-sink-logger0
job-sink-logger0xgzzy

Now lets assume, I trigger 3 jobs via eventing again (names of Jobs are somehow autogenerated):

job-sink-logger0
job-sink-logger1z
job-sink-logger3mpyh

Secret job-sink-logger0 was not updated and still has the old CloudEvent, hence the Job job-sink-logger0 processes the old event and the new event never gets processed.

Knative release version v1.15.4 (I assume also v1.16.0)

Additional context
If something is unclear, just let me know.

@tikr7 tikr7 added the kind/bug Categorizes issue or PR as related to a bug. label Nov 11, 2024
@tikr7
Copy link
Author

tikr7 commented Nov 11, 2024

@pierDipi could you help here? :)

@pierDipi
Copy link
Member

@tikr7 thanks for reporting, I'll take a look

pierDipi added a commit to pierDipi/eventing that referenced this issue Nov 18, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now secrets created
for a given event are bound to a given Job lifecycle.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
pierDipi added a commit to pierDipi/eventing that referenced this issue Nov 18, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
pierDipi added a commit to pierDipi/eventing that referenced this issue Nov 18, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
pierDipi added a commit to pierDipi/eventing that referenced this issue Nov 18, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
pierDipi added a commit to pierDipi/eventing that referenced this issue Nov 18, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
@pierDipi
Copy link
Member

With the associated PR #8331:

$ k get jobs,secrets 
NAME                                COMPLETIONS   DURATION   AGE
job.batch/job-sink-successh1dy3pu   12/12         37s        48s

NAME                             TYPE     DATA   AGE
secret/job-sink-successh1dy3pu   Opaque   1      48s
$ k delete jobs job-sink-successh1dy3pu 
job.batch "job-sink-successh1dy3pu" deleted
$ k get jobs,secrets 
No resources found in default namespace.

One additional note

A job and secret is created per unique event id and source, so if 2 events have the same id and source, then the event is considered the "same" event and a new job + secret will not be created if a job already exists.

This is following the CloudEvents specifciation: https://github.com/cloudevents/spec/blob/main/cloudevents/spec.md

Producers MUST ensure that source + id is unique for each distinct event.

knative-prow bot pushed a commit that referenced this issue Nov 19, 2024
…8331)

* JobSink: Delete secrets associated with jobs when jobs are deleted

As reported in #8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix jobsink name generator + add unit and fuzz tests

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix e2e test

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Lint

Signed-off-by: Pierangelo Di Pilato <[email protected]>

---------

Signed-off-by: Pierangelo Di Pilato <[email protected]>
knative-prow-robot pushed a commit to knative-prow-robot/eventing that referenced this issue Nov 19, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
knative-prow-robot pushed a commit to knative-prow-robot/eventing that referenced this issue Nov 19, 2024
As reported in knative#8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
knative-prow bot pushed a commit that referenced this issue Nov 19, 2024
… are deleted (#8332)

* JobSink: Delete secrets associated with jobs when jobs are deleted

As reported in #8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix jobsink name generator + add unit and fuzz tests

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix e2e test

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Lint

Signed-off-by: Pierangelo Di Pilato <[email protected]>

---------

Signed-off-by: Pierangelo Di Pilato <[email protected]>
Co-authored-by: Pierangelo Di Pilato <[email protected]>
knative-prow bot pushed a commit that referenced this issue Nov 19, 2024
… are deleted (#8333)

* JobSink: Delete secrets associated with jobs when jobs are deleted

As reported in #8323 old
JobSink secrets lead to processing old events again while new events
are lost.

Using OwnerReference and k8s garbage collection, now a secret created
for a given event is bound to a given Job lifecycle, so that when a job
is deleted, the associated secret will be deleted.

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix jobsink name generator + add unit and fuzz tests

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Fix e2e test

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* Lint

Signed-off-by: Pierangelo Di Pilato <[email protected]>

* add missing import

Signed-off-by: Matthias Wessendorf <[email protected]>

---------

Signed-off-by: Pierangelo Di Pilato <[email protected]>
Signed-off-by: Matthias Wessendorf <[email protected]>
Co-authored-by: Pierangelo Di Pilato <[email protected]>
Co-authored-by: Matthias Wessendorf <[email protected]>
@pierDipi
Copy link
Member

This will be released next Tuesday in a patch release for both 1.15 and 1.16

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.

2 participants