-
Notifications
You must be signed in to change notification settings - Fork 8
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
Test job cancel when scheduled on Kubernetes #8
Comments
As part of #7 it appears that sending a signal to a running container doesn't work. PR #21 contains a fix, but still killing the spider doesn't seem to do anything. An important reason is probably that the spider is run as the init process, as PID 1, and cannot be killed. |
Fixed behaviour during run in PR #21. |
At this moment, we look at # kill pod (retry is disabled, so there should be only one pod)
pod = self._get_pod(project, job_id)
if pod: # if a pod has just ended, we're good already, don't kill
# make sure container is running - https://stackoverflow.com/a/74833787
if all([c.state.running for c in pod.status.container_statuses]):
self._k8s_kill(pod.metadata.name, Signals['SIG' + signal].value)
else:
# refactor code to fall through to delete the job instead Not encountered yet, so not including this check for now. |
One approach to make this, is to add an option to create a suspended job, e.g. when the schedule endpoint is called a special query parameter (one that wouldn't be used as a setting), or perhaps a special header. Then the test can use it for testing this scenario |
Stopping jobs mostly works, but it has a number of cases to test.
Can you think of more corner-cases?
Especially in the first case, there may be various stages (e.g. on Kubernetes, waiting for resources, pulling the image).
See also documentation on scrapyd's cancel endpoint.
Note that in PR #21 tests have been added, including some for job cancellation. The main thing now is testing that a job is removed when it is cancelled before it has started. This issue is now about implementing that, including finding a way to test it reliably.
The text was updated successfully, but these errors were encountered: