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

don't kill the server if some modules are still running #109

Conversation

goneri
Copy link
Member

@goneri goneri commented Jun 22, 2022

Depends-On: #110

Ensure we don't have any ongoing task before we shutdown the server.

The syndrome was errors like this:

UnexpectedFailure: Cannot decode plugin answer: b''\n"

Closes: ansible-collections/vmware.vmware_rest#336

@softwarefactory-project-zuul
Copy link

Build failed.

ansible-test-sanity-docker-devel FAILURE in 8m 21s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 8m 12s
✔️ ansible-test-cloud-integration-vmware-rest-python39 SUCCESS in 20m 10s
✔️ build-ansible-collection SUCCESS in 5m 01s
✔️ ansible-test-splitter SUCCESS in 2m 36s
✔️ integration-kubernetes.core-with-turbo-1 SUCCESS in 27m 25s
✔️ integration-kubernetes.core-with-turbo-2 SUCCESS in 31m 30s
✔️ integration-kubernetes.core-with-turbo-3 SUCCESS in 22m 35s
✔️ ansible-tox-linters SUCCESS in 4m 12s
✔️ ansible-galaxy-importer SUCCESS in 4m 25s

@goneri goneri force-pushed the don-t-kill-the-server-if-some-modules-are-still-running_12970 branch from 50426e4 to 2f5072e Compare June 22, 2022 16:18
@softwarefactory-project-zuul
Copy link

Build failed.

ansible-test-sanity-docker-devel FAILURE in 9m 37s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 9m 38s
✔️ ansible-test-cloud-integration-vmware-rest-python39 SUCCESS in 22m 32s
✔️ build-ansible-collection SUCCESS in 7m 24s
✔️ ansible-test-splitter SUCCESS in 2m 52s
✔️ integration-kubernetes.core-with-turbo-1 SUCCESS in 26m 00s
✔️ integration-kubernetes.core-with-turbo-2 SUCCESS in 32m 50s
✔️ integration-kubernetes.core-with-turbo-3 SUCCESS in 22m 00s
✔️ ansible-tox-linters SUCCESS in 7m 28s
✔️ ansible-galaxy-importer SUCCESS in 4m 45s

@goneri
Copy link
Member Author

goneri commented Jun 22, 2022

recheck

@goneri goneri added the mergeit label Jun 22, 2022
@softwarefactory-project-zuul
Copy link

This change depends on a change that failed to merge.

@goneri
Copy link
Member Author

goneri commented Jun 23, 2022

regate

@softwarefactory-project-zuul
Copy link

This change depends on a change that failed to merge.

@goneri goneri force-pushed the don-t-kill-the-server-if-some-modules-are-still-running_12970 branch from 2f5072e to 9dd824a Compare June 23, 2022 14:51
@goneri goneri requested review from abikouo and gravesm June 23, 2022 15:02
Ensure we don't have any ongoing task before we shutdown the server.

The syndrome was errors like this:

    UnexpectedFailure: Cannot decode plugin answer: b''\n"

Closes: ansible-collections/vmware.vmware_rest#336
@goneri goneri force-pushed the don-t-kill-the-server-if-some-modules-are-still-running_12970 branch from 9dd824a to 62f3cad Compare June 23, 2022 15:03
@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ ansible-test-sanity-docker-devel SUCCESS in 8m 14s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 51s
✔️ ansible-test-cloud-integration-vmware-rest-python39 SUCCESS in 20m 01s
✔️ build-ansible-collection SUCCESS in 4m 29s
✔️ ansible-test-splitter SUCCESS in 2m 44s
✔️ integration-kubernetes.core-with-turbo-1 SUCCESS in 25m 42s
✔️ integration-kubernetes.core-with-turbo-2 SUCCESS in 33m 15s
✔️ integration-kubernetes.core-with-turbo-3 SUCCESS in 22m 26s
✔️ ansible-tox-linters SUCCESS in 3m 50s
✔️ ansible-galaxy-importer SUCCESS in 5m 09s

Comment on lines +310 to +311
self.jobs_started = 0
self.jobs_done = 0
Copy link
Member

Choose a reason for hiding this comment

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

I feel like you could simplify this by just having a self.task_running boolean variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can have several tasks running in parallel.


async def handle(self, reader, writer):
self._watcher.cancel()

self.jobs_started += 1
raw_data = await reader.read()
if not raw_data:
return
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this potentially going to lead to the server process never being killed?

@softwarefactory-project-zuul
Copy link

Build succeeded (gate pipeline).

✔️ ansible-test-sanity-docker-devel SUCCESS in 7m 43s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 8m 12s
✔️ ansible-test-cloud-integration-vmware-rest-python39 SUCCESS in 16m 49s
✔️ build-ansible-collection SUCCESS in 4m 38s
✔️ ansible-test-splitter SUCCESS in 3m 04s
✔️ integration-kubernetes.core-with-turbo-1 SUCCESS in 26m 36s
✔️ integration-kubernetes.core-with-turbo-2 SUCCESS in 32m 09s
✔️ integration-kubernetes.core-with-turbo-3 SUCCESS in 20m 14s
✔️ ansible-tox-linters SUCCESS in 3m 58s
✔️ ansible-galaxy-importer SUCCESS in 4m 17s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 30a0429 into ansible-collections:main Jun 23, 2022
goneri added a commit to goneri/cloud.common that referenced this pull request Jun 23, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 24, 2022
turbo: ensure we clean up the leaked jobs

See: #109 (comment)

Reviewed-by: Mike Graves <mgraves@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vCenter NTP conflict with multiple vCenter in Inventory with cloud.common > 2.0.3
2 participants