From 550537acf44600f4778563077f5f54ec1a866b7d Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Fri, 1 Mar 2019 12:56:50 -0700 Subject: [PATCH] Handle swift interactions are strings as necessary To properly handle the differences between python2/python3 we need to ensure that when we interact with swift and are dealing with string data that we handle the types correctly. This change adds a swift utils helper to call to get the string data from an object out of swift. For example our json and yaml files are all strings so if we try to use something like json.dumps() from data we pull from swift, it errors because we're given a bytes like object in python3. Change-Id: I7996cc08cdd3bddf3f4ba0fb2e48f926f944c0dd Related-Blueprint: python3-support --- tripleo_common/actions/container_images.py | 13 +++++--- tripleo_common/actions/deployment.py | 13 ++++---- tripleo_common/actions/heat_capabilities.py | 16 ++++++---- tripleo_common/actions/logging_to_swift.py | 13 +++++--- tripleo_common/actions/plan.py | 33 ++++++++++++-------- tripleo_common/actions/templates.py | 34 +++++++++++++-------- tripleo_common/tests/utils/test_swift.py | 23 ++++++++++++++ tripleo_common/update.py | 9 ++++-- tripleo_common/utils/plan.py | 12 +++++--- tripleo_common/utils/swift.py | 18 +++++++++++ tripleo_common/utils/validations.py | 4 +-- 11 files changed, 132 insertions(+), 56 deletions(-) diff --git a/tripleo_common/actions/container_images.py b/tripleo_common/actions/container_images.py index 1582f802f..1cb6e296a 100644 --- a/tripleo_common/actions/container_images.py +++ b/tripleo_common/actions/container_images.py @@ -28,6 +28,7 @@ from tripleo_common.actions import heat_capabilities from tripleo_common import constants from tripleo_common.image import kolla_builder from tripleo_common.utils import plan as plan_utils +from tripleo_common.utils import swift as swiftutils LOG = logging.getLogger(__name__) @@ -68,7 +69,8 @@ class PrepareContainerImageEnv(base.TripleOAction): params = default_image_params() swift = self.get_object_client(context) try: - swift.put_object( + swiftutils.put_object_string( + swift, self.container, constants.CONTAINER_DEFAULTS_ENVIRONMENT, yaml.safe_dump( @@ -101,8 +103,10 @@ class PrepareContainerImageParameters(base.TripleOAction): def _get_role_data(self, swift): try: - j2_role_file = swift.get_object( - self.container, constants.OVERCLOUD_J2_ROLES_NAME)[1] + j2_role_file = swiftutils.get_object_string( + swift, + self.container, + constants.OVERCLOUD_J2_ROLES_NAME) role_data = yaml.safe_load(j2_role_file) except swiftexceptions.ClientException: LOG.info("No %s file found, not filtering container images by role" @@ -147,7 +151,8 @@ class PrepareContainerImageParameters(base.TripleOAction): os.remove(f) try: - swift.put_object( + swiftutils.put_object_string( + swift, self.container, constants.CONTAINER_DEFAULTS_ENVIRONMENT, yaml.safe_dump( diff --git a/tripleo_common/actions/deployment.py b/tripleo_common/actions/deployment.py index 81dce9552..cb370e25b 100644 --- a/tripleo_common/actions/deployment.py +++ b/tripleo_common/actions/deployment.py @@ -29,6 +29,7 @@ from tripleo_common import constants from tripleo_common import update from tripleo_common.utils import overcloudrc from tripleo_common.utils import plan as plan_utils +from tripleo_common.utils import swift as swiftutils LOG = logging.getLogger(__name__) @@ -73,10 +74,8 @@ class OrchestrationDeployAction(base.TripleOAction): count_check = 0 swift_client = self.get_object_client(context) while not body: - headers, body = swift_client.get_object( - container_name, - object_name - ) + body = swiftutils.get_object_string(swift_client, container_name, + object_name) count_check += 3 if body or count_check > self.timeout: break @@ -373,9 +372,9 @@ class DeploymentStatusAction(base.TripleOAction): deployment_status=None) try: - headers, body = swift_client.get_object( - '%s-messages' % self.plan, - 'deployment_status.yaml') + body = swiftutils.get_object_string(swift_client, + '%s-messages' % self.plan, + 'deployment_status.yaml') deployment_status = yaml.safe_load(body)['deployment_status'] except swiftexceptions.ClientException: diff --git a/tripleo_common/actions/heat_capabilities.py b/tripleo_common/actions/heat_capabilities.py index c3fd17bc2..44fd1405f 100644 --- a/tripleo_common/actions/heat_capabilities.py +++ b/tripleo_common/actions/heat_capabilities.py @@ -23,6 +23,7 @@ from swiftclient import exceptions as swiftexceptions from tripleo_common.actions import base from tripleo_common import constants from tripleo_common.utils import plan as plan_utils +from tripleo_common.utils import swift as swiftutils LOG = logging.getLogger(__name__) @@ -44,9 +45,10 @@ class GetCapabilitiesAction(base.TripleOAction): def run(self, context): try: swift = self.get_object_client(context) - map_file = swift.get_object( - self.container, 'capabilities-map.yaml') - capabilities = yaml.safe_load(map_file[1]) + map_file = swiftutils.get_object_string(swift, + self.container, + 'capabilities-map.yaml') + capabilities = yaml.safe_load(map_file) except Exception: err_msg = ( "Error parsing capabilities-map.yaml.") @@ -202,9 +204,11 @@ class UpdateCapabilitiesAction(base.TripleOAction): # ordering try: swift = self.get_object_client(context) - map_file = swift.get_object( - self.container, 'capabilities-map.yaml') - capabilities = yaml.safe_load(map_file[1]) + map_file = swiftutils.get_object_string( + swift, + self.container, + 'capabilities-map.yaml') + capabilities = yaml.safe_load(map_file) except swiftexceptions.ClientException as err: err_msg = ("Error retrieving capabilities-map.yaml for " "plan %s: %s" % (self.container, err)) diff --git a/tripleo_common/actions/logging_to_swift.py b/tripleo_common/actions/logging_to_swift.py index 99ec5cbaa..a076eb5c0 100644 --- a/tripleo_common/actions/logging_to_swift.py +++ b/tripleo_common/actions/logging_to_swift.py @@ -129,9 +129,10 @@ class PublishUILogToSwiftAction(base.TripleOAction): self._rotate(swift) try: - old_contents = swift.get_object( + old_contents = swiftutils.get_object_string( + swift, self.logging_container, - constants.TRIPLEO_UI_LOG_FILENAME)[1] + constants.TRIPLEO_UI_LOG_FILENAME) new_contents = "%s\n%s" % (old_contents, self.logging_data) except swiftexceptions.ClientException: LOG.debug( @@ -139,9 +140,11 @@ class PublishUILogToSwiftAction(base.TripleOAction): new_contents = self.logging_data try: - swift.put_object(self.logging_container, - constants.TRIPLEO_UI_LOG_FILENAME, - new_contents) + swiftutils.put_object_string( + swift, + self.logging_container, + constants.TRIPLEO_UI_LOG_FILENAME, + new_contents) except swiftexceptions.ClientException as err: msg = "Failed to publish logs: %s" % err return actions.Result(error=msg) diff --git a/tripleo_common/actions/plan.py b/tripleo_common/actions/plan.py index d66dc7f46..467a50876 100644 --- a/tripleo_common/actions/plan.py +++ b/tripleo_common/actions/plan.py @@ -159,8 +159,8 @@ class ListRolesAction(base.TripleOAction): def run(self, context): try: swift = self.get_object_client(context) - roles_data = yaml.safe_load(swift.get_object( - self.container, self.role_file_name)[1]) + roles_data = yaml.safe_load(swiftutils.get_object_string( + swift, self.container, self.role_file_name)) except Exception as err: err_msg = ("Error retrieving roles data from deployment plan: %s" % err) @@ -244,10 +244,10 @@ class UpdatePlanFromDirAction(base.TripleOAction): tmp_tarball.name, container_tmp) # Get all new templates: - new_templates = swift.get_object(container_tmp, - '')[1].splitlines() - old_templates = swift.get_object(self.container, - '')[1].splitlines() + new_templates = swiftutils.get_object_string(swift, container_tmp, + '').splitlines() + old_templates = swiftutils.get_object_string(swift, self.container, + '').splitlines() exclude_user_data = [constants.PLAN_ENVIRONMENT, constants.OVERCLOUD_J2_ROLES_NAME, constants.OVERCLOUD_J2_NETWORKS_NAME, @@ -256,19 +256,28 @@ class UpdatePlanFromDirAction(base.TripleOAction): for new in new_templates: # if doesn't exist, push it: if new not in old_templates: - swift.put_object( + swiftutils.put_object_string( + swift, self.container, new, - swift.get_object(container_tmp, new)[1]) + swiftutils.get_object_string(swift, container_tmp, new) + ) else: - content_new = swift.get_object(container_tmp, new) - content_old = swift.get_object(self.container, new) + content_new = swiftutils.get_object_string(swift, + container_tmp, + new) + content_old = swiftutils.get_object_string(swift, + self.container, + new) if (not content_new == content_old and new not in exclude_user_data): - swift.put_object( + swiftutils.put_object_string( + swift, self.container, new, - swift.get_object(container_tmp, new)[1]) + swiftutils.get_object_string(swift, container_tmp, + new) + ) except swiftexceptions.ClientException as err: msg = "Error attempting an operation on container: %s" % err LOG.exception(msg) diff --git a/tripleo_common/actions/templates.py b/tripleo_common/actions/templates.py index b8046ae43..dd1484983 100644 --- a/tripleo_common/actions/templates.py +++ b/tripleo_common/actions/templates.py @@ -25,6 +25,7 @@ from swiftclient import exceptions as swiftexceptions from tripleo_common.actions import base from tripleo_common import constants from tripleo_common.utils import plan as plan_utils +from tripleo_common.utils import swift as swiftutils LOG = logging.getLogger(__name__) @@ -56,8 +57,9 @@ class J2SwiftLoader(jinja2.BaseLoader): for searchpath in self.searchpath: template_path = os.path.join(searchpath, *pieces) try: - source = self.swift.get_object( - self.container, template_path)[1] + source = swiftutils.get_object_string(self.swift, + self.container, + template_path) return source, None, False except swiftexceptions.ClientException: pass @@ -122,8 +124,8 @@ class ProcessTemplatesAction(base.TripleOAction): try: # write the template back to the plan container LOG.info("Writing rendered template %s" % yaml_f) - swift.put_object( - self.container, yaml_f, r_template) + swiftutils.put_object_string(swift, self.container, yaml_f, + r_template) except swiftexceptions.ClientException as ex: error_msg = ("Error storing file %s in container %s" % (yaml_f, self.container)) @@ -133,8 +135,8 @@ class ProcessTemplatesAction(base.TripleOAction): def _get_j2_excludes_file(self, context): swift = self.get_object_client(context) try: - j2_excl_file = swift.get_object( - self.container, constants.OVERCLOUD_J2_EXCLUDES)[1] + j2_excl_file = swiftutils.get_object_string( + swift, self.container, constants.OVERCLOUD_J2_EXCLUDES) j2_excl_data = yaml.safe_load(j2_excl_file) if (j2_excl_data is None or j2_excl_data.get('name') is None): j2_excl_data = {"name": []} @@ -176,8 +178,8 @@ class ProcessTemplatesAction(base.TripleOAction): swift = self.get_object_client(context) try: - j2_role_file = swift.get_object( - self.container, constants.OVERCLOUD_J2_ROLES_NAME)[1] + j2_role_file = swiftutils.get_object_string( + swift, self.container, constants.OVERCLOUD_J2_ROLES_NAME) role_data = yaml.safe_load(j2_role_file) except swiftexceptions.ClientException: LOG.info("No %s file found, skipping jinja templating" @@ -185,8 +187,8 @@ class ProcessTemplatesAction(base.TripleOAction): return try: - j2_network_file = swift.get_object( - self.container, constants.OVERCLOUD_J2_NETWORKS_NAME)[1] + j2_network_file = swiftutils.get_object_string( + swift, self.container, constants.OVERCLOUD_J2_NETWORKS_NAME) network_data = yaml.safe_load(j2_network_file) # Allow no networks defined in network_data if network_data is None: @@ -258,7 +260,9 @@ class ProcessTemplatesAction(base.TripleOAction): # and create one file common to all roles if f.endswith('.role.j2.yaml'): LOG.info("jinja2 rendering role template %s" % f) - j2_template = swift.get_object(self.container, f)[1] + j2_template = swiftutils.get_object_string(swift, + self.container, + f) LOG.info("jinja2 rendering roles %s" % "," .join(role_names)) for role in role_names: @@ -300,7 +304,9 @@ class ProcessTemplatesAction(base.TripleOAction): elif (f.endswith('.network.j2.yaml')): LOG.info("jinja2 rendering network template %s" % f) - j2_template = swift.get_object(self.container, f)[1] + j2_template = swiftutils.get_object_string(swift, + self.container, + f) LOG.info("jinja2 rendering networks %s" % ",".join(n_map)) for network in n_map: j2_data = {'network': n_map[network]} @@ -325,7 +331,9 @@ class ProcessTemplatesAction(base.TripleOAction): elif f.endswith('.j2.yaml'): LOG.info("jinja2 rendering %s" % f) - j2_template = swift.get_object(self.container, f)[1] + j2_template = swiftutils.get_object_string(swift, + self.container, + f) j2_data = {'roles': role_data, 'networks': network_data} out_f = f.replace('.j2.yaml', '.yaml') self._j2_render_and_put(j2_template, diff --git a/tripleo_common/tests/utils/test_swift.py b/tripleo_common/tests/utils/test_swift.py index cd268cf85..3c145ea06 100644 --- a/tripleo_common/tests/utils/test_swift.py +++ b/tripleo_common/tests/utils/test_swift.py @@ -63,3 +63,26 @@ class SwiftTest(base.TestCase): def test_create_container(self): swift_utils.create_container(self.swiftclient, 'abc') self.swiftclient.put_container.assert_called() + + def test_get_object_string(self): + self.swiftclient.get_object.return_value = (1, str('foo')) + val = swift_utils.get_object_string(self.swiftclient, 'foo', 'bar') + self.assertEqual(str('foo'), val) + + def test_get_object_string_from_bytes(self): + self.swiftclient.get_object.return_value = (1, b'foo') + val = swift_utils.get_object_string(self.swiftclient, 'foo', 'bar') + self.assertEqual(str('foo'), val) + + def test_put_object_string(self): + put_mock = mock.MagicMock() + self.swiftclient.put_object = put_mock + swift_utils.put_object_string(self.swiftclient, 'foo', 'bar', + str('foo')) + put_mock.assert_called_once_with('foo', 'bar', str('foo')) + + def test_put_object_string_from_bytes(self): + put_mock = mock.MagicMock() + self.swiftclient.put_object = put_mock + swift_utils.put_object_string(self.swiftclient, 'foo', 'bar', b'foo') + put_mock.assert_called_once_with('foo', 'bar', str('foo')) diff --git a/tripleo_common/update.py b/tripleo_common/update.py index d182d799e..a4d2a9ba4 100644 --- a/tripleo_common/update.py +++ b/tripleo_common/update.py @@ -19,6 +19,7 @@ import yaml from heatclient.common import template_utils from tripleo_common import constants +from tripleo_common.utils import swift as swiftutils def add_breakpoints_cleanup_into_env(env): @@ -78,9 +79,11 @@ def check_neutron_mechanism_drivers(env, stack, plan_client, container): # TODO(beagles): we need to look for a better way to # get the current template default value. This is fragile # with respect to changing filenames, etc. - ml2_tmpl = plan_client.get_object( - container, 'puppet/services/neutron-plugin-ml2.yaml') - ml2_def = yaml.safe_load(ml2_tmpl[1]) + ml2_tmpl = swiftutils.get_object_string( + plan_client, + container, + 'puppet/services/neutron-plugin-ml2.yaml') + ml2_def = yaml.safe_load(ml2_tmpl) default_drivers = ml2_def.get('parameters', {}).get(driver_key, {}).get('default') new_driver = get_exclusive_neutron_driver(default_drivers) diff --git a/tripleo_common/utils/plan.py b/tripleo_common/utils/plan.py index ecca83ecc..8e54fcc15 100644 --- a/tripleo_common/utils/plan.py +++ b/tripleo_common/utils/plan.py @@ -22,6 +22,7 @@ import tempfile import yaml from tripleo_common import constants +from tripleo_common.utils import swift as swiftutils def update_in_env(swift, env, key, value='', delete_key=False): @@ -44,7 +45,7 @@ def update_in_env(swift, env, key, value='', delete_key=False): def get_env(swift, name): """Get plan environment from Swift and convert it to a dictionary.""" env = yaml.safe_load( - swift.get_object(name, constants.PLAN_ENVIRONMENT)[1] + swiftutils.get_object_string(swift, name, constants.PLAN_ENVIRONMENT) ) # Ensure the name is correct, as it will be used to update the @@ -57,7 +58,8 @@ def get_env(swift, name): def put_env(swift, env): """Convert given environment to yaml and upload it to Swift.""" - swift.put_object( + swiftutils.put_object_string( + swift, env['name'], constants.PLAN_ENVIRONMENT, yaml.safe_dump(env, default_flow_style=False) @@ -67,12 +69,14 @@ def put_env(swift, env): def get_user_env(swift, container_name): """Get user environment from Swift convert it to a dictionary.""" return yaml.safe_load( - swift.get_object(container_name, constants.USER_ENVIRONMENT)[1]) + swiftutils.get_object_string(swift, container_name, + constants.USER_ENVIRONMENT)) def put_user_env(swift, container_name, env): """Convert given user environment to yaml and upload it to Swift.""" - swift.put_object( + swiftutils.put_object_string( + swift, container_name, constants.USER_ENVIRONMENT, yaml.safe_dump(env, default_flow_style=False) diff --git a/tripleo_common/utils/swift.py b/tripleo_common/utils/swift.py index ae7048593..cfb15ed30 100644 --- a/tripleo_common/utils/swift.py +++ b/tripleo_common/utils/swift.py @@ -166,3 +166,21 @@ def create_and_upload_tarball(swiftservice, LOG.error("%s" % error) except SwiftError as e: LOG.error(e.value) + + +def get_object_string(swift, container, object_name): + """Get the object contents as a string """ + data = swift.get_object(container, object_name)[1] + try: + return data.decode('utf-8') + except AttributeError: + return data + + +def put_object_string(swift, container, object_name, contents): + """Put the object contents as a string """ + try: + contents = contents.decode('utf-8') + except AttributeError: + pass + return swift.put_object(container, object_name, contents) diff --git a/tripleo_common/utils/validations.py b/tripleo_common/utils/validations.py index 4d49af664..0f632123f 100644 --- a/tripleo_common/utils/validations.py +++ b/tripleo_common/utils/validations.py @@ -55,7 +55,7 @@ def _get_validations_from_swift(swift, container, objects, groups, results, if skip_existing and validation_id in existing_ids: continue - contents = swift.get_object(container, obj['name'])[1] + contents = swift_utils.get_object_string(swift, container, obj['name']) validation = yaml.safe_load(contents) validation_groups = get_validation_metadata(validation, 'groups') or [] @@ -130,7 +130,7 @@ def download_validation(swift, plan, validation): # If a custom validation with that name exists, get it from the plan # container and override. Otherwise, the default one will be used. try: - contents = swift.get_object(plan, swift_path)[1] + contents = swift_utils.get_object_string(swift, plan, swift_path) except swiftexceptions.ClientException: pass else: