From 5759ec0b1cbb9153558d52f55b874ab8b45880bb Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Fri, 4 Jan 2019 10:07:57 +0000 Subject: [PATCH] glance Windows support This change will allow glance services to run on Windows, using eventlet wsgi for API services. This change will: * avoid monkey patching the os module on Windows (which causes Popen to fail) * avoiding unavailable signals * avoid renaming in-use files or leaking handles * update the check that ensures that just one scrubber process may run at a time. We can't rely on process names as there might be wrapper processes that have similar names (no she-bangs on Windows, so the scripts are called a bit differently). We'll use a global named mutex instead. A subsequent change will leverage Windows job objects as a replacement for process groups, also avoiding forking when spawning workers. At the moment, some Glance tests cannot run on Windows, which is also covered by subsequent patches. DocImpact blueprint windows-support Change-Id: I3bca69638685ceb11a1a316511ad9a298c630ad5 --- glance/async_/flows/convert.py | 4 +- glance/async_/flows/ovf_process.py | 25 ++-- glance/cmd/api.py | 13 +- glance/cmd/registry.py | 14 ++- glance/cmd/scrubber.py | 114 ++++++++++++------ glance/common/wsgi.py | 33 +++-- .../tests/functional/serial/test_scrubber.py | 5 +- .../tests/unit/async_/flows/test_convert.py | 3 +- lower-constraints.txt | 1 + .../windows-support-f4aae61681dba569.yaml | 4 + requirements.txt | 2 + 11 files changed, 147 insertions(+), 71 deletions(-) create mode 100644 releasenotes/notes/windows-support-f4aae61681dba569.yaml diff --git a/glance/async_/flows/convert.py b/glance/async_/flows/convert.py index d1b9f1aa42..48732004e4 100644 --- a/glance/async_/flows/convert.py +++ b/glance/async_/flows/convert.py @@ -91,6 +91,7 @@ class _Convert(task.Task): # specified. There's no "sane" default for this # because the dest format may work differently depending # on the environment OpenStack is running in. + abs_file_path = file_path.split("file://")[-1] conversion_format = CONF.taskflow_executor.conversion_format if conversion_format is None: if not _Convert.conversion_missing_warned: @@ -123,7 +124,8 @@ class _Convert(task.Task): if stderr: raise RuntimeError(stderr) - os.rename(dest_path, file_path.split("file://")[-1]) + os.unlink(abs_file_path) + os.rename(dest_path, abs_file_path) return file_path def revert(self, image_id, result=None, **kwargs): diff --git a/glance/async_/flows/ovf_process.py b/glance/async_/flows/ovf_process.py index 79357de11b..eea581fc9b 100644 --- a/glance/async_/flows/ovf_process.py +++ b/glance/async_/flows/ovf_process.py @@ -82,6 +82,7 @@ class _OVF_Process(task.Task): :param file_path: Path to the OVA package """ + file_abs_path = file_path.split("file://")[-1] image = self.image_repo.get(image_id) # Expect 'ova' as image container format for OVF_Process task if image.container_format == 'ova': @@ -92,17 +93,23 @@ class _OVF_Process(task.Task): # the context as a short-cut. if image.context and image.context.is_admin: extractor = OVAImageExtractor() - data_iter = self._get_ova_iter_objects(file_path) - disk, properties = extractor.extract(data_iter) - image.extra_properties.update(properties) - image.container_format = 'bare' - self.image_repo.save(image) - dest_path = self._get_extracted_file_path(image_id) - with open(dest_path, 'wb') as f: - shutil.copyfileobj(disk, f, 4096) + data_iter = None + try: + data_iter = self._get_ova_iter_objects(file_path) + disk, properties = extractor.extract(data_iter) + image.extra_properties.update(properties) + image.container_format = 'bare' + self.image_repo.save(image) + dest_path = self._get_extracted_file_path(image_id) + with open(dest_path, 'wb') as f: + shutil.copyfileobj(disk, f, 4096) + finally: + if data_iter: + data_iter.close() # Overwrite the input ova file since it is no longer needed - os.rename(dest_path, file_path.split("file://")[-1]) + os.unlink(file_abs_path) + os.rename(dest_path, file_abs_path) else: raise RuntimeError(_('OVA extract is limited to admin')) diff --git a/glance/cmd/api.py b/glance/cmd/api.py index 89fe1b730b..4c35b21bf4 100644 --- a/glance/cmd/api.py +++ b/glance/cmd/api.py @@ -20,6 +20,10 @@ """ Glance API Server """ + +import os +import sys + import eventlet # NOTE(jokke): As per the eventlet commit # b756447bab51046dfc6f1e0e299cc997ab343701 there's circular import happening @@ -27,10 +31,13 @@ import eventlet # before calling monkey_patch(). This is solved in eventlet 0.22.0 but we # need to address it before that is widely used around. eventlet.hubs.get_hub() -eventlet.patcher.monkey_patch() -import os -import sys +if os.name == 'nt': + # eventlet monkey patching the os module causes subprocess.Popen to fail + # on Windows when using pipes due to missing non-blocking IO support. + eventlet.patcher.monkey_patch(os=False) +else: + eventlet.patcher.monkey_patch() from oslo_utils import encodeutils diff --git a/glance/cmd/registry.py b/glance/cmd/registry.py index 929f825ec1..c222e81e9c 100644 --- a/glance/cmd/registry.py +++ b/glance/cmd/registry.py @@ -20,6 +20,10 @@ """ Reference implementation server for Glance Registry """ + +import os +import sys + import eventlet # NOTE(jokke): As per the eventlet commit # b756447bab51046dfc6f1e0e299cc997ab343701 there's circular import happening @@ -27,11 +31,13 @@ import eventlet # before calling monkey_patch(). This is solved in eventlet 0.22.0 but we # need to address it before that is widely used around. eventlet.hubs.get_hub() -eventlet.patcher.monkey_patch() - -import os -import sys +if os.name == 'nt': + # eventlet monkey patching the os module causes subprocess.Popen to fail + # on Windows when using pipes due to missing non-blocking IO support. + eventlet.patcher.monkey_patch(os=False) +else: + eventlet.patcher.monkey_patch() from oslo_utils import encodeutils diff --git a/glance/cmd/scrubber.py b/glance/cmd/scrubber.py index 3a08d558f9..deebdd6de7 100644 --- a/glance/cmd/scrubber.py +++ b/glance/cmd/scrubber.py @@ -18,6 +18,10 @@ """ Glance Scrub Service """ + +import os +import sys + import eventlet # NOTE(jokke): As per the eventlet commit # b756447bab51046dfc6f1e0e299cc997ab343701 there's circular import happening @@ -25,11 +29,15 @@ import eventlet # before calling monkey_patch(). This is solved in eventlet 0.22.0 but we # need to address it before that is widely used around. eventlet.hubs.get_hub() -eventlet.patcher.monkey_patch() -import os +if os.name == 'nt': + # eventlet monkey patching the os module causes subprocess.Popen to fail + # on Windows when using pipes due to missing non-blocking IO support. + eventlet.patcher.monkey_patch(os=False) +else: + eventlet.patcher.monkey_patch() + import subprocess -import sys # If ../glance/__init__.py exists, add ../ to Python search path, so that # it will override what happens to be installed in /usr/(local/)lib/python... @@ -40,6 +48,7 @@ if os.path.exists(os.path.join(possible_topdir, 'glance', '__init__.py')): sys.path.insert(0, possible_topdir) import glance_store +from os_win import utilsfactory as os_win_utilsfactory from oslo_config import cfg from oslo_log import log as logging @@ -54,10 +63,21 @@ CONF.set_default(name='use_stderr', default=True) def main(): - CONF.register_cli_opts(scrubber.scrubber_cmd_cli_opts) - CONF.register_opts(scrubber.scrubber_cmd_opts) + # Used on Window, ensuring that a single scrubber can run at a time. + mutex = None + mutex_acquired = False try: + if os.name == 'nt': + # We can't rely on process names on Windows as there may be + # wrappers with the same name. + mutex = os_win_utilsfactory.get_mutex( + name='Global\\glance-scrubber') + mutex_acquired = mutex.acquire(timeout_ms=0) + + CONF.register_cli_opts(scrubber.scrubber_cmd_cli_opts) + CONF.register_opts(scrubber.scrubber_cmd_opts) + config.parse_args() logging.setup(CONF, 'glance') @@ -65,45 +85,24 @@ def main(): glance_store.create_stores(config.CONF) glance_store.verify_default_store() - app = scrubber.Scrubber(glance_store) - if CONF.restore and CONF.daemon: sys.exit("ERROR: The restore and daemon options should not be set " "together. Please use either of them in one request.") + + app = scrubber.Scrubber(glance_store) + if CONF.restore: - # Try to check the glance-scrubber is running or not. - # 1. Try to find the pid file if scrubber is controlled by - # glance-control - # 2. Try to check the process name. - error_str = ("ERROR: The glance-scrubber process is running under " - "daemon. Please stop it first.") - pid_file = '/var/run/glance/glance-scrubber.pid' - if os.path.exists(os.path.abspath(pid_file)): - sys.exit(error_str) + if os.name == 'nt': + scrubber_already_running = not mutex_acquired + else: + scrubber_already_running = scrubber_already_running_posix() - for glance_scrubber_name in ['glance-scrubber', - 'glance.cmd.scrubber']: - cmd = subprocess.Popen( - ['/usr/bin/pgrep', '-f', glance_scrubber_name], - stdout=subprocess.PIPE, shell=False) - pids, _ = cmd.communicate() + if scrubber_already_running: + already_running_msg = ( + "ERROR: glance-scrubber is already running. " + "Please ensure that the daemon is stopped.") + sys.exit(already_running_msg) - # The response format of subprocess.Popen.communicate() is - # diffderent between py2 and py3. It's "string" in py2, but - # "bytes" in py3. - if isinstance(pids, bytes): - pids = pids.decode() - self_pid = os.getpid() - - if pids.count('\n') > 1 and str(self_pid) in pids: - # One process is self, so if the process number is > 1, it - # means that another glance-scrubber process is running. - sys.exit(error_str) - elif pids.count('\n') > 0 and str(self_pid) not in pids: - # If self is not in result and the pids number is still - # > 0, it means that the another glance-scrubber process is - # running. - sys.exit(error_str) app.revert_image_status(CONF.restore) elif CONF.daemon: server = scrubber.Daemon(CONF.wakeup_time) @@ -115,6 +114,45 @@ def main(): sys.exit("ERROR: %s" % e) except RuntimeError as e: sys.exit("ERROR: %s" % e) + finally: + if mutex and mutex_acquired: + mutex.release() + + +def scrubber_already_running_posix(): + # Try to check the glance-scrubber is running or not. + # 1. Try to find the pid file if scrubber is controlled by + # glance-control + # 2. Try to check the process name. + pid_file = '/var/run/glance/glance-scrubber.pid' + if os.path.exists(os.path.abspath(pid_file)): + return True + + for glance_scrubber_name in ['glance-scrubber', + 'glance.cmd.scrubber']: + cmd = subprocess.Popen( + ['/usr/bin/pgrep', '-f', glance_scrubber_name], + stdout=subprocess.PIPE, shell=False) + pids, _ = cmd.communicate() + + # The response format of subprocess.Popen.communicate() is + # diffderent between py2 and py3. It's "string" in py2, but + # "bytes" in py3. + if isinstance(pids, bytes): + pids = pids.decode() + self_pid = os.getpid() + + if pids.count('\n') > 1 and str(self_pid) in pids: + # One process is self, so if the process number is > 1, it + # means that another glance-scrubber process is running. + return True + elif pids.count('\n') > 0 and str(self_pid) not in pids: + # If self is not in result and the pids number is still + # > 0, it means that the another glance-scrubber process is + # running. + return True + + return False if __name__ == '__main__': diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index bda6f03a2f..dae1e2e5b7 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -495,8 +495,10 @@ class Server(object): try: # NOTE(flaper87): Make sure this process # runs in its own process group. + # NOTE(lpetrut): This isn't available on Windows, so we're going + # to use job objects instead. os.setpgid(self.pgid, self.pgid) - except OSError: + except (OSError, AttributeError): # NOTE(flaper87): When running glance-control, # (glance's functional tests, for example) # setpgid fails with EPERM as glance-control @@ -508,18 +510,25 @@ class Server(object): # shouldn't raise any error here. self.pgid = 0 + @staticmethod + def set_signal_handler(signal_name, handler): + # Some signals may not be available on this platform. + sig = getattr(signal, signal_name, None) + if sig is not None: + signal.signal(sig, handler) + def hup(self, *args): """ Reloads configuration files with zero down time """ - signal.signal(signal.SIGHUP, signal.SIG_IGN) + self.set_signal_handler("SIGHUP", signal.SIG_IGN) raise exception.SIGHUPInterrupt def kill_children(self, *args): """Kills the entire process group.""" - signal.signal(signal.SIGTERM, signal.SIG_IGN) - signal.signal(signal.SIGINT, signal.SIG_IGN) - signal.signal(signal.SIGCHLD, signal.SIG_IGN) + self.set_signal_handler("SIGTERM", signal.SIG_IGN) + self.set_signal_handler("SIGINT", signal.SIG_IGN) + self.set_signal_handler("SIGCHLD", signal.SIG_IGN) self.running = False os.killpg(self.pgid, signal.SIGTERM) @@ -544,9 +553,9 @@ class Server(object): return else: LOG.info(_LI("Starting %d workers"), workers) - signal.signal(signal.SIGTERM, self.kill_children) - signal.signal(signal.SIGINT, self.kill_children) - signal.signal(signal.SIGHUP, self.hup) + self.set_signal_handler("SIGTERM", self.kill_children) + self.set_signal_handler("SIGINT", self.kill_children) + self.set_signal_handler("SIGHUP", self.hup) while len(self.children) < workers: self.run_child() @@ -655,18 +664,18 @@ class Server(object): def run_child(self): def child_hup(*args): """Shuts down child processes, existing requests are handled.""" - signal.signal(signal.SIGHUP, signal.SIG_IGN) + self.set_signal_handler("SIGHUP", signal.SIG_IGN) eventlet.wsgi.is_accepting = False self.sock.close() pid = os.fork() if pid == 0: - signal.signal(signal.SIGHUP, child_hup) - signal.signal(signal.SIGTERM, signal.SIG_DFL) + self.set_signal_handler("SIGHUP", child_hup) + self.set_signal_handler("SIGTERM", signal.SIG_DFL) # ignore the interrupt signal to avoid a race whereby # a child worker receives the signal before the parent # and is respawned unnecessarily as a result - signal.signal(signal.SIGINT, signal.SIG_IGN) + self.set_signal_handler("SIGINT", signal.SIG_IGN) # The child has no need to stash the unwrapped # socket, and the reference prevents a clean # exit on sighup diff --git a/glance/tests/functional/serial/test_scrubber.py b/glance/tests/functional/serial/test_scrubber.py index 2737776d9b..f00504bf61 100644 --- a/glance/tests/functional/serial/test_scrubber.py +++ b/glance/tests/functional/serial/test_scrubber.py @@ -351,8 +351,7 @@ class TestScrubber(functional.FunctionalTest): cmd = ("%s --restore fake_image_id" % exe_cmd) exitcode, out, err = execute(cmd, raise_error=False) self.assertEqual(1, exitcode) - self.assertIn('The glance-scrubber process is running under daemon', - str(err)) + self.assertIn('glance-scrubber is already running', str(err)) self.stop_server(self.scrubber_daemon) @@ -363,7 +362,7 @@ class TestScrubber(functional.FunctionalTest): # Sometimes the glance-scrubber process which is setup by the # previous test can't be shutdown immediately, so if we get the "daemon # running" message we sleep and try again. - not_down_msg = 'The glance-scrubber process is running under daemon' + not_down_msg = 'glance-scrubber is already running' total_wait = 15 for _ in range(total_wait): exitcode, out, err = func() diff --git a/glance/tests/unit/async_/flows/test_convert.py b/glance/tests/unit/async_/flows/test_convert.py index 8b141e24ba..1b8d9ab23e 100644 --- a/glance/tests/unit/async_/flows/test_convert.py +++ b/glance/tests/unit/async_/flows/test_convert.py @@ -78,7 +78,8 @@ class TestImportTask(test_utils.BaseTestCase): group='taskflow_executor') glance_store.create_stores(CONF) - def test_convert_success(self): + @mock.patch.object(os, 'unlink') + def test_convert_success(self, mock_unlink): image_convert = convert._Convert(self.task.task_id, self.task_type, self.img_repo) diff --git a/lower-constraints.txt b/lower-constraints.txt index 6beaebec40..8884a61ef2 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -61,6 +61,7 @@ openstackdocstheme==1.18.1 os-api-ref==1.4.0 os-client-config==1.29.0 os-testr==1.0.0 +os-win==3.0.0 oslo.cache==1.29.0 oslo.concurrency==3.26.0 oslo.config==5.2.0 diff --git a/releasenotes/notes/windows-support-f4aae61681dba569.yaml b/releasenotes/notes/windows-support-f4aae61681dba569.yaml new file mode 100644 index 0000000000..47e74038d8 --- /dev/null +++ b/releasenotes/notes/windows-support-f4aae61681dba569.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Glance services can now run on Windows. diff --git a/requirements.txt b/requirements.txt index 407a00b193..eef2c9aa9c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -56,3 +56,5 @@ cursive>=0.2.1 # Apache-2.0 # timeutils iso8601>=0.1.11 # MIT + +os-win>=3.0.0 # Apache-2.0