From 3d47927ad3b71e382ed8e936b5a28d3e7a904dcf Mon Sep 17 00:00:00 2001 From: Borne Mace Date: Tue, 28 Nov 2017 21:52:47 -0800 Subject: [PATCH] Support modification of non string properties Support modification of non string properties. Introduced the concept of types to the AnsibleProperty object and when a property is set if the type is not str or None then the value will be loaded via yaml and processed into the appropriate python type. At write time it is then written out via yaml dump rather than always writing it out as a string. Change-Id: Iab1d10430cb919cf968e4357c93e23ed360bf338 Jira-Issue: OPENSTACK-1698 --- kollacli/api/properties.py | 23 +++++++++++++++++------ kollacli/common/ansible/actions.py | 18 +++++++----------- kollacli/common/properties.py | 23 ++++++----------------- kollacli/common/support.py | 4 ++-- kollacli/common/utils.py | 24 +++++++++++++++++++++--- tests/property.py | 4 ++-- 6 files changed, 55 insertions(+), 41 deletions(-) diff --git a/kollacli/api/properties.py b/kollacli/api/properties.py index 32890eb..a426ffa 100644 --- a/kollacli/api/properties.py +++ b/kollacli/api/properties.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. import logging +import yaml import kollacli.i18n as u @@ -44,21 +45,29 @@ class PropertyApi(object): :type change_set: list of strings """ + ansible_properties = AnsibleProperties() for key, value in property_dict.items(): check_arg(key, u._('Property Key'), str) - check_arg(value, u._('Property Value'), str, empty_ok=True) - if '"' in value: + current_property = ansible_properties.get_property(key) + if current_property is not None: + current_property_type = current_property.value_type + if (current_property_type is not str and + current_property is not None): + value = yaml.safe_load(value) + check_arg(value, u._('Property Value'), + current_property_type, empty_ok=True) + property_dict[key] = value + else: + check_arg(value, u._('Property Value'), str, empty_ok=True) + if type(value) is str and '"' in value: raise InvalidArgument(u._('Cannot use double quotes in ' 'a property value.')) - property_dict = safe_decode(property_dict) self._check_type(property_type) if property_type is not GLOBAL_TYPE: check_arg(change_set, u._('Change Set'), list, none_ok=True) change_set = safe_decode(change_set) - ansible_properties = AnsibleProperties() - if property_type == GLOBAL_TYPE: ansible_properties.set_property(property_dict) elif property_type == GROUP_TYPE: @@ -142,7 +151,7 @@ class Property(object): Members: - name (str): key - - value (str): value + - value (Any): value - file_name (str): name of file property is from - overrides (bool): does the property override some other value - orig_value (str): the value which is overridden or None @@ -151,6 +160,7 @@ class Property(object): - ovr_global (bool): true if property is overridden at global level - ovr_group (bool): true if property is overridden at group level - ovr_host (bool): true if property is overridden at host level + - value_type (type): the python type of the value """ def __init__(self, ansible_property, override_flags): @@ -161,6 +171,7 @@ class Property(object): self.orig_value = ansible_property.orig_value self.target = ansible_property.target self.prop_type = ansible_property.prop_type + self.value_type = ansible_property.value_type if override_flags is not None: self.ovr_global = override_flags.ovr_global diff --git a/kollacli/common/ansible/actions.py b/kollacli/common/ansible/actions.py index ea75809..2335285 100644 --- a/kollacli/common/ansible/actions.py +++ b/kollacli/common/ansible/actions.py @@ -219,8 +219,9 @@ def _run_deploy_rules(playbook): expected_files = ['account.ring.gz', 'container.ring.gz', 'object.ring.gz'] - is_enabled = properties.get_property('enable_swift') - if is_string_true(is_enabled): + is_swift_enabled = _is_service_enabled('swift', inventory, properties) + + if is_swift_enabled: path_pre = os.path.join(get_kolla_etc(), 'config', 'swift') for expected_file in expected_files: path = os.path.join(path_pre, expected_file) @@ -235,15 +236,10 @@ def _run_deploy_rules(playbook): def _is_service_enabled(servicename, inventory, properties): - service_enabled = False - service = inventory.get_service(servicename) if service is not None: enabled_property = 'enable_' + servicename.replace('-', '_') - is_enabled = properties.get_property(enabled_property) - if is_string_true(is_enabled): - service_enabled = True - else: - service_enabled = False - - return service_enabled + is_enabled = properties.get_property_value(enabled_property) + if type(is_enabled) is str: + is_enabled = is_string_true(is_enabled) + return is_enabled diff --git a/kollacli/common/properties.py b/kollacli/common/properties.py index 9b22883..ca61adc 100644 --- a/kollacli/common/properties.py +++ b/kollacli/common/properties.py @@ -14,7 +14,6 @@ import copy import logging import os -import six import yaml import kollacli.i18n as u @@ -220,7 +219,7 @@ class AnsibleProperties(object): prop_list += self.group_props[group.name] return prop_list - def get_property(self, property_name): + def get_property_value(self, property_name): self._load_properties() prop_val = None if property_name in self.unique_global_props: @@ -228,6 +227,10 @@ class AnsibleProperties(object): prop_val = prop.value return prop_val + def get_property(self, property_name): + self._load_properties() + return self.unique_global_props.get(property_name) + def get_all_unique(self): self._load_properties() unique_list = [] @@ -239,21 +242,6 @@ class AnsibleProperties(object): self._load_properties() return self.unique_override_flags - # TODO(bmace) -- if this isn't used for 2.1.x it should be removed - # property listing is still being tweaked so leaving for - # the time being in case we want to use it - def filter_jinja2(self, contents): - new_contents = {} - for key, value in contents.items(): - if not isinstance(value, six.string_types): - LOG.debug('removing non-string: %s', value) - continue - if value and '{{' in value and '}}' in value: - LOG.debug('removing jinja2 value: %s', value) - continue - new_contents[key] = value - return new_contents - def set_property(self, property_dict): change_property(self.globals_path, property_dict, clear=False) @@ -365,6 +353,7 @@ class AnsibleProperty(object): self.overrides = overrides self.orig_value = orig_value self.target = target + self.value_type = type(value) class OverrideFlags(object): diff --git a/kollacli/common/support.py b/kollacli/common/support.py index ccf5edd..cd0a430 100644 --- a/kollacli/common/support.py +++ b/kollacli/common/support.py @@ -62,9 +62,9 @@ class HostLogs(object): ansible_properties = AnsibleProperties() base_distro = \ - ansible_properties.get_property('kolla_base_distro') + ansible_properties.get_property_value('kolla_base_distro') install_type = \ - ansible_properties.get_property('kolla_install_type') + ansible_properties.get_property_value('kolla_install_type') # typically this prefix will be "ol-openstack-" container_prefix = base_distro + '-' + install_type + '-' diff --git a/kollacli/common/utils.py b/kollacli/common/utils.py index 751a0a7..047413a 100644 --- a/kollacli/common/utils.py +++ b/kollacli/common/utils.py @@ -286,7 +286,13 @@ def change_property(file_path, property_dict, clear=False): # clear existing property continue # edit existing property - line = '%s: "%s"' % (split_key, cloned_dict[split_key]) + value = cloned_dict[split_key] + if type(value) is not str: + value = yaml.safe_dump(value).strip() + line = '%s: %s' % (split_key, value) + else: + line = '%s: "%s"' % (split_key, value) + # clear out the key after we are done, all existing keys # will be appended at the end (or for clear, ignored) del cloned_dict[split_key] @@ -294,10 +300,22 @@ def change_property(file_path, property_dict, clear=False): if not clear: # add new properties to file for key, value in cloned_dict.items(): - line = '%s: "%s"' % (key, value) + if type(value) is not str: + value = yaml.safe_dump(value).strip() + line = '%s: %s' % (key, value) + else: + line = '%s: "%s"' % (key, value) + + # when we are doing an append we want to avoid + # blank lines before the new entry + if new_contents[-1:][0] == '': + del new_contents[-1] new_contents.append(line) - write_data = '\n'.join(new_contents) + # if the last line is blank, trim it off + if new_contents[-1:][0] == '': + del new_contents[-1] + write_data = '\n'.join(new_contents) + '\n' sync_write_file(file_path, write_data) diff --git a/tests/property.py b/tests/property.py index 722f2fe..75d9174 100644 --- a/tests/property.py +++ b/tests/property.py @@ -195,8 +195,8 @@ class TestFunctional(KollaCliTest): bad_path = self._is_size_ok(sizes, 0, '=', 3) self.assertIsNone(bad_path, 'Size of file %s is ' % bad_path + 'different from initial size ' - '(%s %s)' - % (switch, targets_csv)) + '(%s %s %s)' + % (switch, targets_csv, str(sizes))) # test setting empty string value = '""'