diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py index 0e2e8aa624..57e0be70dd 100644 --- a/glance/tests/functional/__init__.py +++ b/glance/tests/functional/__init__.py @@ -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): """ diff --git a/glance/tests/functional/test_bin_glance_control.py b/glance/tests/functional/test_bin_glance_control.py deleted file mode 100644 index 3ec9d21815..0000000000 --- a/glance/tests/functional/test_bin_glance_control.py +++ /dev/null @@ -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') diff --git a/glance/tests/functional/test_glance_manage.py b/glance/tests/functional/test_glance_manage.py index 478415ad13..60266f914c 100644 --- a/glance/tests/functional/test_glance_manage.py +++ b/glance/tests/functional/test_glance_manage.py @@ -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() diff --git a/glance/tests/functional/v1/test_api.py b/glance/tests/functional/v1/test_api.py index 1a92470095..1c0e8dc30e 100644 --- a/glance/tests/functional/v1/test_api.py +++ b/glance/tests/functional/v1/test_api.py @@ -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, diff --git a/glance/tests/functional/v1/test_multiprocessing.py b/glance/tests/functional/v1/test_multiprocessing.py index 5c5c0d9272..d8e8cb4f3f 100644 --- a/glance/tests/functional/v1/test_multiprocessing.py +++ b/glance/tests/functional/v1/test_multiprocessing.py @@ -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) diff --git a/glance/tests/utils.py b/glance/tests/utils.py index 3b3ee5662d..be51804985 100644 --- a/glance/tests/utils.py +++ b/glance/tests/utils.py @@ -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,