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

Send SIGTERM to webserver in teardown of gitfs tests #53953

Merged
merged 4 commits into from Jul 25, 2019

Conversation

@Ch3LL
Copy link
Contributor

commented Jul 22, 2019

What does this PR do?

The tests:

    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_all_saltenvs
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_all_saltenvs_base
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_includes_disabled
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_includes_enabled
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_includes_enabled_solves___env___with_mountpoint
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_mountpoint_parameter
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_multiple_sources_dev_master_merge_lists
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_multiple_sources_dev_master_no_merge_lists
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_multiple_sources_master_dev_merge_lists
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_multiple_sources_master_dev_no_merge_lists
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_multiple_sources_with_pillarenv
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_root_and_mountpoint_parameters
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_root_parameter
    integration.pillar.test_git_pillar.TestGitPythonHTTP.test_single_source

sometimes fail. This nginx issue https://trac.nginx.org/nginx/ticket/753 may be causing our issue when we attempt to start/stop nginx between each git pillar test.

@waynew
waynew approved these changes Jul 22, 2019
@Ch3LL Ch3LL changed the title Check if gitfs server fails to setup for tests Send SIGTERM to webserver in teardown of gitfs tests Jul 22, 2019
@waynew
waynew approved these changes Jul 22, 2019
@@ -653,7 +653,9 @@ def tearDownClass(cls):
for proc in (cls.case.nginx_proc, cls.case.uwsgi_proc):
if proc is not None:
try:
proc.send_signal(signal.SIGQUIT)
proc.send_signal(signal.SIGTERM)
if proc.is_running():

This comment has been minimized.

Copy link
@teran-mckinney

teran-mckinney Jul 23, 2019

Not too familiar with this code in particular, but I would consider adding a tiny sleep before the if here. Responding to SIGTERM is not going to be instant, even 10-100ms might do the trick. Othewise, I would just stick with one "aggressive" signal.

Secondly, SIGQUIT is perhaps not what you want. I think you might be more interested in doing SIGKILL.

https://en.wikipedia.org/wiki/Signal_(IPC)#SIGQUIT

@dwoz
dwoz approved these changes Jul 25, 2019
@dwoz

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

Merging without the flaky amazon 2 PR build

@dwoz dwoz merged commit cc1cda1 into saltstack:2019.2.1 Jul 25, 2019
23 of 24 checks passed
23 of 24 checks passed
ci/py2/amazon2 This commit cannot be built
Details
WIP Ready for review
Details
ci/docs This commit looks good
Details
ci/lint This commit looks good
Details
ci/py2/centos6 This commit looks good
Details
ci/py2/centos7 This commit looks good
Details
ci/py2/centos7/tcp This commit looks good
Details
ci/py2/debian8 This commit looks good
Details
ci/py2/debian9 This commit looks good
Details
ci/py2/fedora29 This commit looks good
Details
ci/py2/ubuntu1604 This commit looks good
Details
ci/py2/ubuntu1604/tcp This commit looks good
Details
ci/py2/ubuntu1804 This commit looks good
Details
ci/py2/windows2016 This commit looks good
Details
ci/py3/amazon2 This commit looks good
Details
ci/py3/centos7 This commit looks good
Details
ci/py3/centos7/tcp This commit looks good
Details
ci/py3/debian8 This commit looks good
Details
ci/py3/debian9 This commit looks good
Details
ci/py3/fedora29 This commit looks good
Details
ci/py3/ubuntu1604 This commit looks good
Details
ci/py3/ubuntu1604/tcp This commit looks good
Details
ci/py3/ubuntu1804 This commit looks good
Details
ci/py3/windows2016 This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.