Remove glance-control from the test suite

This patch removes the use of glance-control from the test suite. Instead
of forking out glance-control and letting it fork out the other programs,
we will now fork out the other programs directly. The file which tested
glance-control has also been removed. While we will still have
glance-control as part of the distribution for a bit and thus it should
still be tested, that test was doing so in a way that depended upon the
forking behavior previously found in the test suite.

This patch also requires the patch
https://review.openstack.org/#/c/26076/ on which it is dependent. As a
side effect it tests for the problem found in bug: 1068051

Additionally, this fixes a race condition. Previously each functional
test called cleanup() in tearDown(). Cleanup would open a pid file and
send a SIGTERM to the processes (api, registry, scrubber). However,
it did not wait for this process to die and get cleaned up. The signal
handler in the processes is over ridden so the death is not instant.
This leaves a chance for server from a previous test to be actively
listening on a port when the next test starts.

fixes bug: 1173415
blueprint: refactoring-better-faster-stronger-functional-tests

Change-Id: Iabdb6fdc13b5f1993590b5a801a0e89df1a544c7
This commit is contained in:
John Bresnahan 2013-05-03 10:50:42 -10:00
parent 926b6c9db0
commit 866cb846f5
6 changed files with 137 additions and 340 deletions

View File

@ -68,17 +68,18 @@ class Server(object):
self.conf_file_name = None
self.conf_base = None
self.paste_conf_base = None
self.server_control = 'glance-control'
self.exec_env = None
self.deployment_flavor = ''
self.show_image_direct_url = False
self.enable_v1_api = True
self.enable_v2_api = True
self.server_control_options = ''
self.needs_database = False
self.log_file = None
self.sock = sock
self.fork_socket = True
self.process_pid = None
self.server_program = None
self.stop_kill = False
def write_conf(self, **kwargs):
"""
@ -138,9 +139,7 @@ class Server(object):
self.create_database()
cmd = ("%(server_control)s --pid-file=%(pid_file)s "
"%(server_control_options)s "
"%(server_name)s start %(conf_file_name)s"
cmd = ("%(server_program)s --config-file %(conf_file_name)s"
% self.__dict__)
# close the sock and release the unused port closer to start time
if self.exec_env:
@ -156,34 +155,41 @@ class Server(object):
exec_env[utils.GLANCE_TEST_SOCKET_FD_STR] = str(fd)
self.sock.close()
rc = execute(cmd,
no_venv=self.no_venv,
exec_env=exec_env,
expect_exit=expect_exit,
expected_exitcode=expected_exitcode,
context=overridden)
self.process_pid = test_utils.fork_exec(cmd,
logfile=os.devnull,
exec_env=exec_env)
self.stop_kill = not expect_exit
if self.pid_file:
pf = open(self.pid_file, 'w')
pf.write('%d\n' % self.process_pid)
pf.close()
if not expect_exit:
rc = 0
try:
os.kill(self.process_pid, 0)
except OSError:
raise RuntimeError("The process did not start")
else:
rc = test_utils.wait_for_fork(
self.process_pid,
expected_exitcode=expected_exitcode)
# avoid an FD leak
if self.sock:
os.close(fd)
self.sock = None
return rc
return (rc, '', '')
def reload(self, expect_exit=True, expected_exitcode=0, **kwargs):
"""
Call glane-control reload for a specific server.
Start and stop the service to reload
Any kwargs passed to this method will override the configuration
value in the conf file used in starting the servers.
"""
cmd = ("%(server_control)s --pid-file=%(pid_file)s "
"%(server_control_options)s "
"%(server_name)s reload %(conf_file_name)s"
% self.__dict__)
return execute(cmd,
no_venv=self.no_venv,
exec_env=self.exec_env,
expect_exit=expect_exit,
expected_exitcode=expected_exitcode)
self.stop()
return self.start(expect_exit=expect_exit,
expected_exitcode=expected_exitcode, **kwargs)
def create_database(self):
"""Create database if required for this server"""
@ -233,11 +239,13 @@ class Server(object):
"""
Spin down the server.
"""
cmd = ("%(server_control)s --pid-file=%(pid_file)s "
"%(server_name)s stop %(conf_file_name)s"
% self.__dict__)
return execute(cmd, no_venv=self.no_venv, exec_env=self.exec_env,
expect_exit=True)
if not self.process_pid:
raise Exception('why is this being called? %s' % self.server_name)
if self.stop_kill:
os.kill(self.process_pid, signal.SIGTERM)
rc = test_utils.wait_for_fork(self.process_pid, raise_error=False)
return (rc, '', '')
def dump_log(self, name):
log = logging.getLogger(name)
@ -258,6 +266,7 @@ class ApiServer(Server):
pid_file=None, sock=None, **kwargs):
super(ApiServer, self).__init__(test_dir, port, sock=sock)
self.server_name = 'api'
self.server_program = 'glance-%s' % self.server_name
self.default_store = kwargs.get("default_store", "file")
self.key_file = ""
self.cert_file = ""
@ -297,7 +306,6 @@ class ApiServer(Server):
self.image_cache_driver = 'sqlite'
self.policy_file = policy_file
self.policy_default_rule = 'default'
self.server_control_options = '--capture-output'
self.needs_database = True
default_sql_connection = 'sqlite:////%s/tests.sqlite' % self.test_dir
@ -424,6 +432,7 @@ class RegistryServer(Server):
def __init__(self, test_dir, port, sock=None):
super(RegistryServer, self).__init__(test_dir, port, sock=sock)
self.server_name = 'registry'
self.server_program = 'glance-%s' % self.server_name
self.needs_database = True
default_sql_connection = 'sqlite:////%s/tests.sqlite' % self.test_dir
@ -433,7 +442,6 @@ class RegistryServer(Server):
self.pid_file = os.path.join(self.test_dir, "registry.pid")
self.log_file = os.path.join(self.test_dir, "registry.log")
self.owner_is_tenant = True
self.server_control_options = '--capture-output'
self.workers = 0
self.conf_base = """[DEFAULT]
verbose = %(verbose)s
@ -481,6 +489,7 @@ class ScrubberDaemon(Server):
# NOTE(jkoelker): Set the port to 0 since we actually don't listen
super(ScrubberDaemon, self).__init__(test_dir, 0)
self.server_name = 'scrubber'
self.server_program = 'glance-%s' % self.server_name
self.daemon = daemon
self.image_dir = os.path.join(self.test_dir, "images")
@ -514,6 +523,14 @@ swift_store_container = %(swift_store_container)s
swift_store_auth_version = %(swift_store_auth_version)s
"""
def start(self, expect_exit=True, expected_exitcode=0, **kwargs):
if 'daemon' in kwargs:
expect_exit = False
return super(ScrubberDaemon, self).start(
expect_exit=expect_exit,
expected_exitcode=expected_exitcode,
**kwargs)
class FunctionalTest(test_utils.BaseTestCase):
@ -611,14 +628,19 @@ class FunctionalTest(test_utils.BaseTestCase):
tests are destroyed or spun down
"""
for pid_file in self.pid_files:
if os.path.exists(pid_file):
pid = int(open(pid_file).read().strip())
try:
os.killpg(pid, signal.SIGTERM)
except:
pass # Ignore if the process group is dead
os.unlink(pid_file)
# NOTE(jbresnah) call stop on each of the servers instead of
# checking the pid file. stop() will wait until the child
# server is dead. This eliminates the possibility of a race
# between a child process listening on a port actually dying
# and a new process being started
servers = [self.api_server,
self.registry_server,
self.scrubber_daemon]
for s in servers:
try:
s.stop()
except Exception:
pass
for f in self.files_to_destroy:
if os.path.exists(f):
@ -654,16 +676,14 @@ class FunctionalTest(test_utils.BaseTestCase):
"Failed to spin up the requested server. "
"Got: %s" % err)
self.assertTrue(re.search("Starting glance-[a-z]+ with", out))
self.launched_servers.append(server)
launch_msg = self.wait_for_servers([server], expect_launch)
self.assertTrue(launch_msg is None, launch_msg)
def start_with_retry(self, server, port_name, max_retries,
expect_launch=True, expect_exit=True,
expect_confirmation=True, **kwargs):
expect_launch=True,
**kwargs):
"""
Starts a server, with retries if the server launches but
fails to start listening on the expected port.
@ -675,19 +695,15 @@ class FunctionalTest(test_utils.BaseTestCase):
successfully start
:param expect_exit: true iff the launched process is expected
to exit in a timely fashion
:param expect_confirmation: true iff launch confirmation msg
expected on stdout
"""
launch_msg = None
for i in range(0, max_retries):
exitcode, out, err = server.start(expect_exit=expect_exit,
exitcode, out, err = server.start(expect_exit=not expect_launch,
**kwargs)
name = server.server_name
self.assertEqual(0, exitcode,
"Failed to spin up the %s server. "
"Got: %s" % (name, err))
if expect_confirmation:
self.assertTrue(("Starting glance-%s with" % name) in out)
launch_msg = self.wait_for_servers([server], expect_launch)
if launch_msg:
server.stop()
@ -725,7 +741,6 @@ class FunctionalTest(test_utils.BaseTestCase):
self.assertEqual(0, exitcode,
"Failed to spin up the Scrubber daemon. "
"Got: %s" % err)
self.assertTrue("Starting glance-scrubber with" in out)
def ping_server(self, port):
"""
@ -777,7 +792,7 @@ class FunctionalTest(test_utils.BaseTestCase):
for f in failed:
msg += ('%s, ' % f.server_name)
if os.path.exists(f.pid_file):
pid = int(open(f.pid_file).read().strip())
pid = f.process_pid
trace = f.pid_file.replace('.pid', '.trace')
cmd = 'strace -p %d -o %s' % (pid, trace)
execute(cmd, raise_error=False, expect_exit=False)
@ -834,10 +849,7 @@ class FunctionalTest(test_utils.BaseTestCase):
:param server: the server to stop
"""
# Spin down the requested server
exitcode, out, err = server.stop()
self.assertEqual(0, exitcode,
"Failed to spin down the %s server. Got: %s" %
(err, name))
server.stop()
def stop_servers(self):
"""

View File

@ -1,278 +0,0 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# Copyright 2012 Red Hat, Inc
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""Functional test case for the glance-control --respawn option """
import httplib2
import os
import signal
import socket
import sys
import time
import fixtures
from glance.tests import functional
from glance.tests.utils import skip_if_disabled
class TestGlanceControl(functional.FunctionalTest):
"""Functional test for glance-control"""
def get_versions(self):
path = "http://%s:%d" % ("127.0.0.1", self.api_port)
http = httplib2.Http()
response, content = http.request(path, 'GET')
self.assertEqual(response.status, 300)
def get_pid(self):
return int(open(self.api_server.pid_file).read().strip())
def kill_server(self):
pid = self.get_pid()
os.killpg(pid, signal.SIGKILL)
return pid
def wait_for(self, predicate):
count = 0
while count < 50:
if predicate():
break
else:
time.sleep(0.1)
count += 1
self.assertTrue(predicate())
def connection_unavailable(self, type):
try:
self.get_versions()
self.fail('%s server should not be respawned' % type)
except socket.error:
exc_value = sys.exc_info()[1]
self.assertTrue('Connection refused' in exc_value or
'ECONNREFUSED' in exc_value)
def _do_test_fallback_pidfile(self, pid_file):
self.cleanup()
self.api_server.pid_file = pid_file
exitcode, out, err = self.api_server.start(expect_exit=True,
**self.__dict__.copy())
lines = out.split('\n')
warn = ('Falling back to a temp file, '
'you can stop glance-api service using:')
self.assertTrue(warn in lines)
fallback = lines[lines.index(warn) + 1].split()[-1]
self.assertTrue(os.path.exists(fallback))
self.api_server.pid_file = fallback
self.assertTrue(os.path.exists('/proc/%s' % self.get_pid()))
self.stop_server(self.api_server, 'API server')
@skip_if_disabled
def test_fallback_pidfile_uncreateable_dir(self):
"""
We test that glance-control falls back to a temporary pid file
for non-existent pid file directory that cannot be created.
"""
if os.getuid() is 0:
self.skipTest("User root can always create directories")
parent = self.useFixture(fixtures.TempDir()).path
os.chmod(parent, 0)
pid_file = os.path.join(parent, 'pids', 'api.pid')
self._do_test_fallback_pidfile(pid_file)
@skip_if_disabled
def test_fallback_pidfile_unwriteable_dir(self):
"""
We test that glance-control falls back to a temporary pid file
for unwriteable pid file directory.
"""
if os.getuid() is 0:
self.skipTest("User root can always write inside directories")
parent = self.useFixture(fixtures.TempDir()).path
os.chmod(parent, 0)
pid_file = os.path.join(parent, 'api.pid')
self._do_test_fallback_pidfile(pid_file)
@skip_if_disabled
def test_respawn(self):
"""
We test that the '--respawn' option causes the API server
to be respawned after death but not after a deliberate stop
"""
self.cleanup()
self.api_server.server_control_options += ' --respawn'
# because glance_api_control will be restarting the services
# we cannot pass the socket through in fork
self.api_server.fork_socket = False
# start API server, allowing glance-control to continue running
self.start_with_retry(self.api_server,
'api_port',
3,
expect_launch=True,
expect_exit=False,
expect_confirmation=False,
**self.__dict__.copy())
# ensure the service pid has been cached
pid_cached = lambda: os.path.exists(self.api_server.pid_file)
self.wait_for(pid_cached)
# ensure glance-control has had a chance to waitpid on child
time.sleep(1)
# verify server health with version negotiation
self.get_versions()
# server is killed ungracefully
old_pid = self.kill_server()
# ... but should be respawned
# wait for pid to cycle
pid_changed = lambda: old_pid != self.get_pid()
self.wait_for(pid_changed)
# ensure API service port is re-activated
launch_msg = self.wait_for_servers([self.api_server])
self.assertTrue(launch_msg is None, launch_msg)
# verify server health with version negotiation
self.get_versions()
# deliberately stop server, it should not be respawned
proc_file = '/proc/%d' % self.get_pid()
self.stop_server(self.api_server, 'API server')
# ensure last server process has gone away
process_died = lambda: not os.path.exists(proc_file)
self.wait_for(process_died)
# deliberately stopped server should not be respawned
launch_msg = self.wait_for_servers([self.api_server], False)
self.assertTrue(launch_msg is None, launch_msg)
# ensure the server has not been respawned
self.connection_unavailable('deliberately stopped')
@skip_if_disabled
def test_bouncing(self):
"""
We test that the '--respawn' option doesn't cause bouncing
API server to be respawned
"""
self.cleanup()
self.api_server.server_control_options += ' --respawn'
self.api_server.default_store = 'shouldnotexist'
exitcode, out, err = self.api_server.start(**self.__dict__.copy())
# ensure the service pid has been cached
pid_cached = lambda: os.path.exists(self.api_server.pid_file)
self.wait_for(pid_cached)
# ensure glance-control has had a chance to waitpid on child
time.sleep(1)
# bouncing server should not be respawned
launch_msg = self.wait_for_servers([self.api_server], False)
self.assertTrue(launch_msg is None, launch_msg)
# ensure server process has gone away
process_died = lambda: not os.path.exists('/proc/%d' % self.get_pid())
self.wait_for(process_died)
# ensure the server has not been respawned
self.connection_unavailable('bouncing')
@skip_if_disabled
def test_reload(self):
"""Exercise `glance-control api reload`"""
self.cleanup()
# start API server, allowing glance-control to continue running
self.start_with_retry(self.api_server,
'api_port',
3,
expect_launch=True,
expect_exit=False,
expect_confirmation=False,
**self.__dict__.copy())
# ensure the service pid has been cached
pid_cached = lambda: os.path.exists(self.api_server.pid_file)
self.wait_for(pid_cached)
# ensure glance-control has had a chance to waitpid on child
time.sleep(1)
# verify server health with version negotiation
self.get_versions()
self.reload_server(self.api_server, True)
# ensure API service port is re-activated
launch_msg = self.wait_for_servers([self.api_server])
self.assertTrue(launch_msg is None, launch_msg)
# verify server health with version negotiation
self.get_versions()
# deliberately stop server
proc_file = '/proc/%d' % self.get_pid()
self.stop_server(self.api_server, 'API server')
# ensure last server process has gone away
process_died = lambda: not os.path.exists(proc_file)
self.wait_for(process_died)
# deliberately stopped server should not be respawned
launch_msg = self.wait_for_servers([self.api_server], False)
self.assertTrue(launch_msg is None, launch_msg)
# ensure the server has not been respawned
self.connection_unavailable('deliberately stopped')
@skip_if_disabled
def test_infinite_child_death(self):
"""Ensure child processes don't respawn forever if they can't start"""
self.cleanup()
self.api_server.default_store = 'shouldnotexist'
self.api_server.workers = 2
exitcode, out, err = self.api_server.start(**self.__dict__.copy())
# ensure the service pid has been cached
pid_cached = lambda: os.path.exists(self.api_server.pid_file)
self.wait_for(pid_cached)
# ensure glance-control has had a chance to waitpid on child
time.sleep(1)
# bouncing server should not be respawned
launch_msg = self.wait_for_servers([self.api_server], False)
self.assertTrue(launch_msg is None, launch_msg)
# ensure server process has gone away
process_died = lambda: not os.path.exists('/proc/%d' % self.get_pid())
self.wait_for(process_died)
# ensure the server has not been respawned
self.connection_unavailable('bouncing')

View File

@ -68,8 +68,6 @@ class TestGlanceManage(functional.FunctionalTest):
self._assert_tables()
self.stop_servers()
@depends_on_exe('sqlite3')
@skip_if_disabled
def test_db_creation_auto_create_overridden(self):
@ -77,5 +75,3 @@ class TestGlanceManage(functional.FunctionalTest):
self._sync_db(False)
self._assert_tables()
self.stop_servers()

View File

@ -1234,11 +1234,6 @@ class TestApi(functional.FunctionalTest):
self.cleanup()
self.default_store = 'shouldnotexist'
# ensure failure exit code is available to assert on
# -- on slower machines this needs a few seconds or
# the unit test will fail
self.api_server.server_control_options += ' --await-child=3'
# ensure that the API server fails to launch
self.start_server(self.api_server,
expect_launch=False,

View File

@ -43,7 +43,7 @@ class TestMultiprocessing(functional.FunctionalTest):
self.stop_servers()
def _get_children(self):
api_pid = open(self.api_server.pid_file).read().strip()
api_pid = self.api_server.process_pid
cmd = ("ps --no-headers --ppid %s -o pid,cmd | "
"grep python | " # NOTE(markwash): ignore non-python procs
"awk '{print $1; print >> \"/dev/stderr\"}'" % api_pid)

View File

@ -20,9 +20,11 @@
import errno
import functools
import os
import shlex
import socket
import StringIO
import subprocess
import sys
from oslo.config import cfg
import stubout
@ -122,6 +124,76 @@ def skip_if_disabled(func):
return wrapped
def fork_exec(cmd,
exec_env=None,
logfile=None):
"""
Execute a command using fork/exec.
This is needed for programs system executions that need path
searching but cannot have a shell as their parent process, for
example: glance-api. When glance-api starts it sets itself as
the parent process for its own process group. Thus the pid that
a Popen process would have is not the right pid to use for killing
the process group. This patch gives the test env direct access
to the actual pid.
:param cmd: Command to execute as an array of arguments.
:param exec_env: A dictionary representing the environment with
which to run the command.
:param logile: A path to a file which will hold the stdout/err of
the child process.
"""
env = os.environ.copy()
if exec_env is not None:
for env_name, env_val in exec_env.items():
if callable(env_val):
env[env_name] = env_val(env.get(env_name))
else:
env[env_name] = env_val
pid = os.fork()
if pid == 0:
if logfile:
fds = [1, 2]
with open(logfile, 'r+b') as fptr:
for desc in fds: # close fds
try:
os.dup2(fptr.fileno(), desc)
except OSError:
pass
args = shlex.split(cmd)
os.execvpe(args[0], args, env)
else:
return pid
def wait_for_fork(pid,
raise_error=True,
expected_exitcode=0):
"""
Wait for a process to complete
This function will wait for the given pid to complete. If the
exit code does not match that of the expected_exitcode an error
is raised.
"""
rc = 0
try:
(pid, rc) = os.waitpid(pid, 0)
rc = os.WEXITSTATUS(rc)
if rc != expected_exitcode:
raise RuntimeError('The exit code %d is not %d'
% (rc, expected_exitcode))
except Exception:
if raise_error:
raise
return rc
def execute(cmd,
raise_error=True,
no_venv=False,