From e2c31de9b3f60d55ad6ea35e14af1ace2655c8d1 Mon Sep 17 00:00:00 2001 From: Javier Pena Date: Wed, 7 Mar 2018 16:26:05 +0100 Subject: [PATCH] Initial Python 3 support Packstack needs to get adapted to Python 3. This patch adds initial compatibility fixes and a tox-py36 job (non-voting) to test it. Change-Id: I653454b523224615ea5f0e9f5a7d799031f57649 --- .zuul.yaml | 2 + docs/packstack.rst | 2 +- packstack/installer/core/drones.py | 5 ++- packstack/installer/core/parameters.py | 3 +- packstack/installer/output_messages.py | 2 +- packstack/installer/run_setup.py | 41 +++++++++++---------- packstack/installer/utils/datastructures.py | 2 +- packstack/installer/utils/decorators.py | 2 +- packstack/installer/utils/shell.py | 20 ++++++++-- packstack/installer/utils/shortcuts.py | 3 +- packstack/installer/utils/strings.py | 21 ++++++++--- packstack/modules/documentation.py | 2 +- packstack/plugins/prescript_000.py | 1 + tests/installer/test_sequences.py | 6 +-- tests/installer/test_setup_params.py | 5 ++- tests/test_base.py | 9 +++-- tox.ini | 2 +- 17 files changed, 81 insertions(+), 47 deletions(-) diff --git a/.zuul.yaml b/.zuul.yaml index abeac35b0..37414147b 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -87,6 +87,8 @@ - packstack-integration-scenario002-tempest - packstack-integration-scenario003-tempest - packstack-multinode-scenario002-tempest + - openstack-tox-py36: + voting: false gate: jobs: - packstack-integration-scenario001-tempest diff --git a/docs/packstack.rst b/docs/packstack.rst index f7d9f2277..39f59770d 100755 --- a/docs/packstack.rst +++ b/docs/packstack.rst @@ -1,4 +1,4 @@ -========= +========= Packstack ========= diff --git a/packstack/installer/core/drones.py b/packstack/installer/core/drones.py index 98c5c243e..5f3828d8c 100644 --- a/packstack/installer/core/drones.py +++ b/packstack/installer/core/drones.py @@ -13,6 +13,7 @@ # limitations under the License. import os +import six import stat import uuid import time @@ -75,7 +76,7 @@ class SshTarballTransferMixin(object): dest = self.recipe_dir[len(self.resource_dir):].lstrip('/') else: dest = '' - for marker, recipes in self._recipes.iteritems(): + for marker, recipes in six.iteritems(self._recipes): for path in recipes: _dest = os.path.join(dest, os.path.basename(path)) pack.add(path, arcname=_dest) @@ -278,7 +279,7 @@ class Drone(object): logger = logging.getLogger() skip = skip or [] lastmarker = None - for mark, recipelist in self._recipes.iteritems(): + for mark, recipelist in six.iteritems(self._recipes): if marker and marker != mark: logger.debug('Skipping marker %s for node %s.' % (mark, self.node)) diff --git a/packstack/installer/core/parameters.py b/packstack/installer/core/parameters.py index 0f7fa4934..ea9ed385d 100644 --- a/packstack/installer/core/parameters.py +++ b/packstack/installer/core/parameters.py @@ -17,6 +17,7 @@ Container set for groups and parameters """ from ..utils.datastructures import SortedDict +import six class Parameter(object): @@ -31,7 +32,7 @@ class Parameter(object): defaults = {}.fromkeys(self.allowed_keys) defaults.update(attributes) - for key, value in defaults.iteritems(): + for key, value in six.iteritems(defaults): if key not in self.allowed_keys: raise KeyError('Given attribute %s is not allowed' % key) self.__dict__[key] = value diff --git a/packstack/installer/output_messages.py b/packstack/installer/output_messages.py index 13e5a4806..cd9bc0a86 100644 --- a/packstack/installer/output_messages.py +++ b/packstack/installer/output_messages.py @@ -25,7 +25,7 @@ DONT CHANGE any of the params names (in UPPER-CASE) they are used in the engine-setup.py ''' -import basedefs +from packstack.installer import basedefs INFO_HEADER = "Welcome to the %s setup utility" % basedefs.APP_NAME INFO_INSTALL_SUCCESS = "\n **** Installation completed successfully ******\n" diff --git a/packstack/installer/run_setup.py b/packstack/installer/run_setup.py index 556a70687..4ffb17224 100644 --- a/packstack/installer/run_setup.py +++ b/packstack/installer/run_setup.py @@ -11,15 +11,18 @@ # See the License for the specific language governing permissions and # limitations under the License. -import ConfigParser +from six.moves import configparser +from six.moves import StringIO +from functools import cmp_to_key + import copy import datetime import getpass import logging import os import re +import six import sys -from StringIO import StringIO import traceback import types import textwrap @@ -27,17 +30,17 @@ import textwrap from optparse import OptionGroup from optparse import OptionParser -import basedefs -import validators +from packstack.installer import basedefs +from packstack.installer import validators from . import utils -import processors -import output_messages +from packstack.installer import processors +from packstack.installer import output_messages from .exceptions import FlagValidationError from .exceptions import ParamValidationError from packstack.modules.common import filtered_hosts from packstack.version import version_info -from setup_controller import Controller +from packstack.installer.setup_controller import Controller controller = Controller() commandLineValues = {} @@ -238,21 +241,21 @@ def mask(input): If it finds, it replaces them with '********' """ output = copy.deepcopy(input) - if isinstance(input, types.DictType): + if isinstance(input, dict): for key in input: - if isinstance(input[key], types.StringType): + if isinstance(input[key], six.string_types): output[key] = utils.mask_string(input[key], masked_value_set) - if isinstance(input, types.ListType): + if isinstance(input, list): for item in input: org = item orgIndex = input.index(org) - if isinstance(item, types.StringType): + if isinstance(item, six.string_types): item = utils.mask_string(item, masked_value_set) if item != org: output.remove(org) output.insert(orgIndex, item) - if isinstance(input, types.StringType): + if isinstance(input, six.string_types): output = utils.mask_string(input, masked_value_set) return output @@ -329,7 +332,7 @@ def _handleGroupCondition(config, conditionName, conditionValue): # If the condition is a string - just read it to global conf # We assume that if we get a string as a member it is the name of a member of conf_params - elif isinstance(conditionName, types.StringType): + elif isinstance(conditionName, six.string_types): conditionValue = _loadParamFromFile(config, "general", conditionName) else: # Any other type is invalid @@ -349,14 +352,14 @@ def _loadParamFromFile(config, section, param_name): # Get value from answer file try: value = config.get(section, param_name) - except ConfigParser.NoOptionError: + except configparser.NoOptionError: value = None # Check for deprecated parameters deprecated = param.DEPRECATES if param.DEPRECATES is not None else [] for old_name in deprecated: try: val = config.get(section, old_name) - except ConfigParser.NoOptionError: + except configparser.NoOptionError: continue if not val: # value is empty string @@ -406,7 +409,7 @@ def _handleAnswerFileParams(answerFile): logging.debug("Starting to handle config file") # Read answer file - fconf = ConfigParser.ConfigParser() + fconf = configparser.RawConfigParser() fconf.read(answerFile) # Iterate all the groups and check the pre/post conditions @@ -545,7 +548,7 @@ def _getConditionValue(matchMember): returnValue = False if isinstance(matchMember, types.FunctionType): returnValue = matchMember(controller.CONF) - elif isinstance(matchMember, types.StringType): + elif isinstance(matchMember, six.string_types): # we assume that if we get a string as a member it is the name # of a member of conf_params if matchMember not in controller.CONF: @@ -766,7 +769,7 @@ def validate_answer_file_options(answerfile_path): raise Exception( output_messages.ERR_NO_ANSWER_FILE % answerfile_path) - answerfile = ConfigParser.ConfigParser() + answerfile = configparser.RawConfigParser() answerfile.read(answerfile_path) sections = answerfile._sections general_sections = sections.get('general', None) @@ -912,7 +915,7 @@ def loadPlugins(): sys.path.append(basedefs.DIR_MODULES) fileList = [f for f in os.listdir(basedefs.DIR_PLUGINS) if f[0] != "_"] - fileList = sorted(fileList, cmp=plugin_compare) + fileList = sorted(fileList, key=cmp_to_key(plugin_compare)) for item in fileList: # Looking for files that end with ###.py, example: a_plugin_100.py match = re.search("^(.+\_\d\d\d)\.py$", item) diff --git a/packstack/installer/utils/datastructures.py b/packstack/installer/utils/datastructures.py index a451ff247..b9db4a309 100644 --- a/packstack/installer/utils/datastructures.py +++ b/packstack/installer/utils/datastructures.py @@ -36,7 +36,7 @@ class SortedDict(dict): data = list(data) super(SortedDict, self).__init__(data) if isinstance(data, dict): - self.keyOrder = data.keys() + self.keyOrder = list(data) else: self.keyOrder = [] seen = set() diff --git a/packstack/installer/utils/decorators.py b/packstack/installer/utils/decorators.py index dcf1d7758..7dcf7c9b1 100644 --- a/packstack/installer/utils/decorators.py +++ b/packstack/installer/utils/decorators.py @@ -36,6 +36,6 @@ def retry(count=1, delay=0, retry_on=Exception): if delay: time.sleep(delay) tried += 1 - wrapper.func_name = func.func_name + wrapper.__name__ = func.__name__ return wrapper return decorator diff --git a/packstack/installer/utils/shell.py b/packstack/installer/utils/shell.py index 0a9d3349e..4a32b99e3 100644 --- a/packstack/installer/utils/shell.py +++ b/packstack/installer/utils/shell.py @@ -14,7 +14,7 @@ import re import os -import types +import six import logging import subprocess @@ -38,11 +38,12 @@ def execute(cmd, workdir=None, can_fail=True, mask_list=None, mask_list = mask_list or [] repl_list = [("'", "'\\''")] - if not isinstance(cmd, types.StringType): + if not isinstance(cmd, six.string_types): import pipes masked = ' '.join((pipes.quote(i) for i in cmd)) else: masked = cmd + masked = mask_string(masked, mask_list, repl_list) if log: logging.info("Executing command:\n%s" % masked) @@ -53,6 +54,11 @@ def execute(cmd, workdir=None, can_fail=True, mask_list=None, shell=use_shell, close_fds=True, env=environ) out, err = proc.communicate() + + if not isinstance(out, six.text_type): + out = out.decode('utf-8') + if not isinstance(err, six.text_type): + err = err.decode('utf-8') masked_out = mask_string(out, mask_list, repl_list) masked_err = mask_string(err, mask_list, repl_list) if log: @@ -102,11 +108,17 @@ class ScriptRunner(object): cmd = ["bash", "-x"] environ = os.environ environ['LANG'] = 'en_US.UTF8' + obj = subprocess.Popen(cmd, stdin=_PIPE, stdout=_PIPE, stderr=_PIPE, close_fds=True, shell=False, env=environ) - script = "function t(){ exit $? ; } \n trap t ERR \n" + script - out, err = obj.communicate(script) + script = "function t(){ exit $? ; } \n trap t ERR \n %s" % script + + out, err = obj.communicate(script.encode('utf-8')) + if not isinstance(out, six.text_type): + out = out.decode('utf-8') + if not isinstance(err, six.text_type): + err = err.decode('utf-8') masked_out = mask_string(out, mask_list, repl_list) masked_err = mask_string(err, mask_list, repl_list) if log: diff --git a/packstack/installer/utils/shortcuts.py b/packstack/installer/utils/shortcuts.py index 52b380abb..c5a8d98b8 100644 --- a/packstack/installer/utils/shortcuts.py +++ b/packstack/installer/utils/shortcuts.py @@ -15,10 +15,11 @@ import grp import os import pwd +import six def host_iter(config): - for key, value in config.iteritems(): + for key, value in six.iteritems(config): if key.endswith("_HOST"): host = value.split('/')[0] if host: diff --git a/packstack/installer/utils/strings.py b/packstack/installer/utils/strings.py index b49724bda..12b876d5d 100644 --- a/packstack/installer/utils/strings.py +++ b/packstack/installer/utils/strings.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +from functools import cmp_to_key import re +import six STR_MASK = '*' * 8 @@ -29,6 +31,10 @@ def color_text(text, color): return '%s%s%s' % (COLORS[color], text, COLORS['nocolor']) +def stringcmp(x, y): + return len(y) - len(x) + + def mask_string(unmasked, mask_list=None, replace_list=None): """ Replaces words from mask_list with MASK in unmasked string. @@ -39,14 +45,19 @@ def mask_string(unmasked, mask_list=None, replace_list=None): mask_list = mask_list or [] replace_list = replace_list or [] - masked = unmasked - for word in sorted(mask_list, lambda x, y: len(y) - len(x)): + if isinstance(unmasked, six.text_type): + masked = unmasked.encode('utf-8') + else: + masked = unmasked + + for word in sorted(mask_list, key=cmp_to_key(stringcmp)): if not word: continue + word = word.encode('utf-8') for before, after in replace_list: - word = word.replace(before, after) - masked = masked.replace(word, STR_MASK) - return masked + word = word.replace(before.encode('utf-8'), after.encode('utf-8')) + masked = masked.replace(word, STR_MASK.encode('utf-8')) + return masked.decode('utf-8') def state_format(msg, state, color): diff --git a/packstack/modules/documentation.py b/packstack/modules/documentation.py index 3c651191e..f04c067e3 100644 --- a/packstack/modules/documentation.py +++ b/packstack/modules/documentation.py @@ -69,7 +69,7 @@ def update_params_usage(path, params, opt_title='OPTIONS', sectioned=True): if not _rst_cache: tree = core.publish_doctree( - source=open(path).read().decode('utf-8'), source_path=path + source=open(path).read(), source_path=path ) for key, value in _iter_options(_get_options(tree, opt_title)): _rst_cache.setdefault(key, value) diff --git a/packstack/plugins/prescript_000.py b/packstack/plugins/prescript_000.py index 39aa902d3..620f99a99 100755 --- a/packstack/plugins/prescript_000.py +++ b/packstack/plugins/prescript_000.py @@ -1160,6 +1160,7 @@ def manage_rdo(host, config): # yum-config-manager returns 0 always, but returns current setup # if succeeds rc, out = server.execute() + match = re.search('enabled\s*=\s*(1|True)', out) if not match: msg = ('Failed to set RDO repo on host %s:\nRPM file seems to be ' diff --git a/tests/installer/test_sequences.py b/tests/installer/test_sequences.py index d00cc768e..3d8a7a755 100644 --- a/tests/installer/test_sequences.py +++ b/tests/installer/test_sequences.py @@ -15,8 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +import six import sys -import StringIO from unittest import TestCase from packstack.installer import utils @@ -29,7 +29,7 @@ class StepTestCase(PackstackTestCaseMixin, TestCase): def setUp(self): super(StepTestCase, self).setUp() self._stdout = sys.stdout - sys.stdout = StringIO.StringIO() + sys.stdout = six.StringIO() def tearDown(self): super(StepTestCase, self).tearDown() @@ -57,7 +57,7 @@ class SequenceTestCase(PackstackTestCaseMixin, TestCase): def setUp(self): super(SequenceTestCase, self).setUp() self._stdout = sys.stdout - sys.stdout = StringIO.StringIO() + sys.stdout = six.StringIO() self.steps = [{'name': '1', 'function': lambda x, y: True, 'title': 'Step 1'}, diff --git a/tests/installer/test_setup_params.py b/tests/installer/test_setup_params.py index 5dd943d10..81c3f48b8 100644 --- a/tests/installer/test_setup_params.py +++ b/tests/installer/test_setup_params.py @@ -19,6 +19,7 @@ Test cases for packstack.installer.core.parameters module. """ +import six from unittest import TestCase from ..test_base import PackstackTestCaseMixin @@ -49,7 +50,7 @@ class ParameterTestCase(PackstackTestCaseMixin, TestCase): initialization """ param = Parameter(self.data) - for key, value in self.data.iteritems(): + for key, value in six.iteritems(self.data): self.assertEqual(getattr(param, key), value) def test_default_attribute(self): @@ -80,7 +81,7 @@ class GroupTestCase(PackstackTestCaseMixin, TestCase): Test packstack.installer.core.parameters.Group initialization """ group = Group(attributes=self.attrs, parameters=self.params) - for key, value in self.attrs.iteritems(): + for key, value in six.iteritems(self.attrs): self.assertEqual(getattr(group, key), value) for param in self.params: self.assertIn(param['CONF_NAME'], group.parameters) diff --git a/tests/test_base.py b/tests/test_base.py index eb5f076af..156889db2 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -49,7 +49,8 @@ class FakePopen(object): '''Register a fake script.''' if isinstance(args, list): args = '\n'.join(args) - prefix = "function t(){ exit $? ; } \n trap t ERR \n" + + prefix = "function t(){ exit $? ; } \n trap t ERR \n " args = prefix + args cls.script_registry[args] = {'stdout': stdout, 'stderr': stderr, @@ -86,8 +87,8 @@ class FakePopen(object): def communicate(self, input=None): if self._is_script: - if input in self.script_registry: - this = self.script_registry[input] + if input.decode('utf-8') in self.script_registry: + this = self.script_registry[input.decode('utf-8')] else: LOG.warning('call to unregistered script: %s', input) this = {'stdout': '', 'stderr': '', 'returncode': 0} @@ -128,7 +129,7 @@ class PackstackTestCaseMixin(object): raise AssertionError(_msg) def assertListEqual(self, list1, list2, msg=None): - f, s = len(list1), len(list2) + f, s = len(list(list1)), len(list(list2)) _msg = msg or ('Element counts were not equal. First has %s, ' 'Second has %s' % (f, s)) self.assertEqual(f, s, msg=_msg) diff --git a/tox.ini b/tox.ini index d5529b179..fb02855fa 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] minversion = 1.6 -envlist = py27,pep8,releasenotes +envlist = py27,py36,pep8,releasenotes skipsdist = True [testenv]