Skip to content

Conversation

@sanjana2505006
Copy link

what was happening

When a K8s job is deleted, its pods are often deleted as well. The finaliser was trying to find the pods for the job, failing (because they were already gone), and then logging an ERROR.

changes i made

I updated the logic to check if the job itself is marked for deletion (deletion_timestamp is set). If so, it now correctly interprets the missing Pods as "expected behaviour" during cleanup and logs a DEBUG message instead of an ERROR.

this PR resolves issue #14

@sanjana2505006
Copy link
Author

Hello @EHJ-52n,
Could you please review the PR and let me know if any further changes are needed?
Thank you!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move your new if out of the existing one. I think, having it before line 168 is better.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me, that your test should be located in the test_finalizer.py.
This might highlight the case, that your test is not doing anything different from the already existing test test_handle_job_ended_event_does_not_fail_if_no_pod_found_for_job. So, please elaborate, why this test is required and how your new behaviour is tested with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants