From fa22231bc7311184f9667558b7bff068755c5b37 Mon Sep 17 00:00:00 2001 From: John Fulton Date: Fri, 11 May 2018 03:01:48 -0400 Subject: [PATCH] Persist ceph-ansible fetch_directory using mistral When scaling ceph monitors, ceph-ansible uses context from the fetch_directory to prevent new monitors from behaving like they are the only monitors. Save the fetch_directory in swift after each ceph-ansible playbook run; and if there is a fetch directory in swift, restore it before each playbook run. Note that https://review.openstack.org/#/c/582811 only resolves 1769769 for Master where config-download external deploy tasks run ceph-ansible. This change resolves 1769769 when using mistral for Queens/Pike (when backported). Change-Id: I4b576a6e7fbfb18fa13221e2d080bf7876a8303e Depends-On: I995ad32345a39238ffb9cbcf9966dedc60c75ff8 Closes-Bug: #1769769 (cherry picked from commit 2f300dfd906ae747e626efa27f4dc0380aef361a) --- setup.cfg | 2 + tripleo_common/actions/files.py | 117 ++++++++++++++++++++- tripleo_common/tests/actions/test_files.py | 82 +++++++++++++++ tripleo_common/utils/tarball.py | 15 +++ workbooks/ceph-ansible.yaml | 26 ++++- 5 files changed, 236 insertions(+), 6 deletions(-) diff --git a/setup.cfg b/setup.cfg index 6fc8f443d..84b2bdb74 100644 --- a/setup.cfg +++ b/setup.cfg @@ -142,6 +142,8 @@ mistral.actions = tripleo.files.file_exists = tripleo_common.actions.files:FileExists tripleo.files.make_temp_dir = tripleo_common.actions.files:MakeTempDir tripleo.files.remove_temp_dir = tripleo_common.actions.files:RemoveTempDir + tripleo.files.save_temp_dir_to_swift = tripleo_common.actions.files:SaveTempDirToSwift + tripleo.files.restore_temp_dir_from_swift = tripleo_common.actions.files:RestoreTempDirFromSwift tripleo.ansible-generate-inventory = tripleo_common.actions.ansible:AnsibleGenerateInventoryAction tripleo.undercloud.get_free_space = tripleo_common.actions.undercloud:GetFreeSpace tripleo.undercloud.create_backup_dir = tripleo_common.actions.undercloud:CreateBackupDir diff --git a/tripleo_common/actions/files.py b/tripleo_common/actions/files.py index 38133b7c3..aa69a06fb 100644 --- a/tripleo_common/actions/files.py +++ b/tripleo_common/actions/files.py @@ -16,14 +16,19 @@ import os import re import shutil import six +import sys import tempfile - from mistral_lib import actions -from mistral_lib.actions import base +from oslo_concurrency import processutils +from swiftclient import exceptions as swiftexceptions +from tripleo_common.actions import base +from tripleo_common.utils import swift as swiftutils +from tripleo_common.utils import tarball +from tripleo_common.utils import time_functions as timeutils -class FileExists(base.Action): +class FileExists(base.TripleOAction): """Verifies if a path exists on the localhost (undercloud)""" def __init__(self, path): self.path = path @@ -38,7 +43,7 @@ class FileExists(base.Action): return actions.Result(error={"msg": msg}) -class MakeTempDir(base.Action): +class MakeTempDir(base.TripleOAction): """Creates temporary directory on localhost The directory created will match the regular expression @@ -57,7 +62,7 @@ class MakeTempDir(base.Action): return actions.Result(error={"msg": six.text_type(msg)}) -class RemoveTempDir(base.Action): +class RemoveTempDir(base.TripleOAction): """Removes temporary directory on localhost by path. The path must match the regular expression @@ -80,3 +85,105 @@ class RemoveTempDir(base.Action): return actions.Result(data={"msg": msg}) except Exception as msg: return actions.Result(error={"msg": six.text_type(msg)}) + + +class SaveTempDirToSwift(base.TripleOAction): + """Save temporary directory, identified by path, to Swift container + + The path must match the regular expression + ^/tmp/file-mistral-action[A-Za-z0-9_]{6}$ + + The Swift container must exist + + Contents from path will be packaged in a tarball before upload + + Older tarball(s) will be replaced with the one that is uploaded + """ + + def __init__(self, path, container): + super(SaveTempDirToSwift, self).__init__() + self.path = path + self.container = container + + def run(self, context): + swift = self.get_object_client(context) + swift_service = self.get_object_service(context) + tarball_name = 'temporary_dir-%s.tar.gz' \ + % timeutils.timestamp() + # regex from tempfile's _RandomNameSequence characters + _regex = '^/tmp/file-mistral-action[A-Za-z0-9_]{6}$' + if (not isinstance(self.path, six.string_types) or + not re.match(_regex, self.path)): + msg = "Path does not match %s" % _regex + return actions.Result(error={"msg": msg}) + try: + headers, objects = swift.get_container(self.container) + for o in objects: + swift.delete_object(self.container, o['name']) + swiftutils.create_and_upload_tarball( + swift_service, self.path, self.container, + tarball_name, delete_after=sys.maxsize) + except swiftexceptions.ClientException as err: + msg = "Error attempting an operation on container: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except (OSError, IOError) as err: + msg = "Error while writing file: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except processutils.ProcessExecutionError as err: + msg = "Error while creating a tarball: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except Exception as err: + msg = "Error exporting logs: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + msg = "Saved tarball of directory: %s in Swift container: %s" % ( + self.path, self.container) + return actions.Result(data={"msg": msg}) + + +class RestoreTempDirFromSwift(base.TripleOAction): + """Unpack tarball from Swift container into temporary directory at path + + The path must exist and match the regular expression + ^/tmp/file-mistral-action[A-Za-z0-9_]{6}$ + + Container should contain a single tarball object + """ + + def __init__(self, path, container): + super(RestoreTempDirFromSwift, self).__init__() + self.path = path + self.container = container + + def run(self, context): + swift = self.get_object_client(context) + # regex from tempfile's _RandomNameSequence characters + _regex = '^/tmp/file-mistral-action[A-Za-z0-9_]{6}$' + if (not isinstance(self.path, six.string_types) or + not re.match(_regex, self.path)): + msg = "Path does not match %s" % _regex + return actions.Result(error={"msg": msg}) + try: + swiftutils.download_container(swift, self.container, self.path) + filenames = os.listdir(self.path) + if len(filenames) == 1: + tarball.extract_tarball(self.path, filenames[0], remove=True) + else: + msg = "%d objects found in container: %s" \ + % (len(filenames), self.container) + msg += " but one object was expected." + return actions.Result(error={"msg": six.text_type(msg)}) + except swiftexceptions.ClientException as err: + msg = "Error attempting an operation on container: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except (OSError, IOError) as err: + msg = "Error while writing file: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except processutils.ProcessExecutionError as err: + msg = "Error while creating a tarball: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + except Exception as err: + msg = "Error exporting logs: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) + msg = "Swift container: %s has been extracted to directory: %s" % ( + self.container, self.path) + return actions.Result(data={"msg": msg}) diff --git a/tripleo_common/tests/actions/test_files.py b/tripleo_common/tests/actions/test_files.py index 4a6756d71..836c8a1b9 100644 --- a/tripleo_common/tests/actions/test_files.py +++ b/tripleo_common/tests/actions/test_files.py @@ -66,3 +66,85 @@ class RemoveTempDirTest(base.TestCase): self.assertIsNone(action_result.error) self.assertEqual('Deleted directory /tmp/file-mistral-actionxFLfYz', action_result.data['msg']) + + +class SaveTempDirToSwiftTest(base.TestCase): + def setUp(self): + super(SaveTempDirToSwiftTest, self).setUp() + self.path = "/tmp/file-mistral-actionxFLfYz" + self.container = "my_container" + self.tarball = "foo.tar.gz" + + @mock.patch("tripleo_common.utils.swift.create_and_upload_tarball") + @mock.patch("tripleo_common.actions.base.TripleOAction.get_object_service") + @mock.patch("tripleo_common.actions.base.TripleOAction.get_object_client") + def test_save_temp_dir_to_swift(self, mock_get_object_client, + mock_get_object_service, + mock_create_and_upload_tarball): + # Setup context, swift, swift_service, get_container, create_upload + mock_ctx = mock.MagicMock() + + swift = mock.MagicMock(url="http://test.com") + mock_get_object_client.return_value = swift + + swift_service = mock.MagicMock() + mock_get_object_service.return_value = swift_service + + def return_container_files(*args): + return ('headers', []) + + swift.get_container = mock.MagicMock( + side_effect=return_container_files) + mock_get_object_client.return_value = swift + + mock_create_and_upload_tarball.return_value = mock.MagicMock( + swift_service, self.path, self.container, self.tarball) + + # Test + action = files.SaveTempDirToSwift(self.path, self.container) + result = action.run(mock_ctx) + + # Verify + self.assertFalse(result.cancel) + self.assertIsNone(result.error) + msg = "Saved tarball of directory: %s in Swift container: %s" \ + % (self.path, self.container) + self.assertEqual(msg, result.data['msg']) + + +class RestoreTempDirFromSwiftTest(base.TestCase): + def setUp(self): + super(RestoreTempDirFromSwiftTest, self).setUp() + self.path = "/tmp/file-mistral-actionxFLfYz" + self.container = "my_container" + self.tarball = "foo.tar.gz" + + @mock.patch("os.listdir") + @mock.patch("tripleo_common.utils.tarball.extract_tarball") + @mock.patch("tripleo_common.utils.swift.download_container") + @mock.patch("tripleo_common.actions.base.TripleOAction.get_object_client") + def test_restore_temp_dir_from_swift(self, mock_get_object_client, + mock_download_container, + mock_extract_tarball, mock_listdir): + # Setup context, swift, listdir, tarball + mock_ctx = mock.MagicMock() + + swift = mock.MagicMock(url="http://test.com") + mock_get_object_client.return_value = swift + + mock_download_container.return_value = mock.MagicMock( + swift, self.container, self.path) + mock_extract_tarball.return_value = mock.MagicMock( + self.path, self.tarball) + mock_listdir.return_value = [self.tarball] + + # Test + action = files.RestoreTempDirFromSwift(self.path, self.container) + result = action.run(mock_ctx) + + # Verify + self.assertFalse(result.cancel) + self.assertIsNone(result.error) + msg = "Swift container: %s has been extracted to directory: %s" \ + % (self.container, self.path) + self.assertEqual(msg, result.data['msg']) diff --git a/tripleo_common/utils/tarball.py b/tripleo_common/utils/tarball.py index 696e2b02c..7b668c295 100644 --- a/tripleo_common/utils/tarball.py +++ b/tripleo_common/utils/tarball.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. import logging +import os from oslo_concurrency import processutils @@ -38,3 +39,17 @@ def tarball_extract_to_swift_container(object_client, filename, container): query_string='extract-archive=tar.gz', headers={'X-Detect-Content-Type': 'true'} ) + + +def extract_tarball(directory, tarball, options='-xf', remove=False): + """Extracts the tarball contained in the directory.""" + full_path = directory + '/' + tarball + if not os.path.exists(full_path): + LOG.debug('Tarball %s does not exist' % full_path) + else: + LOG.debug('Extracting tarball %s' % full_path) + cmd = ['/usr/bin/tar', '-C', directory, options, full_path] + processutils.execute(*cmd) + if remove: + LOG.debug('Removing tarball %s' % full_path) + os.remove(full_path) diff --git a/workbooks/ceph-ansible.yaml b/workbooks/ceph-ansible.yaml index 815be2be3..a58299f1d 100644 --- a/workbooks/ceph-ansible.yaml +++ b/workbooks/ceph-ansible.yaml @@ -23,9 +23,14 @@ workflows: - ceph_ansible_extra_vars: {} - ceph_ansible_playbook: /usr/share/ceph-ansible/site-docker.yml.sample - node_data_lookup: '{}' + - swift_container: 'ceph_ansible_fetch_dir' tags: - tripleo-common-managed tasks: + set_swift_container: + publish: + swift_container: <% concat(env().get('heat_stack_name', ''), '_', $.swift_container) %> + on-complete: collect_puppet_hieradata collect_puppet_hieradata: on-success: check_hieradata publish: @@ -67,6 +72,19 @@ workflows: action: tripleo.files.make_temp_dir publish: fetch_directory: <% task().result.path %> + on-success: verify_container_exists + verify_container_exists: + action: swift.head_container container=<% $.swift_container %> + on-success: restore_fetch_directory + on-error: create_container + restore_fetch_directory: + action: tripleo.files.restore_temp_dir_from_swift + input: + container: <% $.swift_container %> + path: <% $.fetch_directory %> + on-success: collect_nodes_uuid + create_container: + action: swift.put_container container=<% $.swift_container %> on-success: collect_nodes_uuid collect_nodes_uuid: action: tripleo.ansible-playbook @@ -159,6 +177,12 @@ workflows: ireallymeanit: 'yes' publish: output: <% task().result %> - on-complete: purge_fetch_directory + on-complete: save_fetch_directory + save_fetch_directory: + action: tripleo.files.save_temp_dir_to_swift + input: + container: <% $.swift_container %> + path: <% $.fetch_directory %> + on-success: purge_fetch_directory purge_fetch_directory: action: tripleo.files.remove_temp_dir path=<% $.fetch_directory %>