From 33e1dc95e93c784b42aea06a0d8a2e0b20b97fbb Mon Sep 17 00:00:00 2001 From: Ratnadeep Debnath Date: Sat, 30 Apr 2016 14:59:12 +0530 Subject: [PATCH 1/3] Initial work on refactoring Nulecule config. #524 - Create a class representation for config data - Implement load() to load config data for a component, taking into account answers and user supplied data - Implement context() method to render flattened data to be consumed in a component artifact. - Improve API for config handling and wire up with Nulecule. - Allow comparing config instances for equality. - Added a placeholder to resolve provider config in Config. - Use ':' as namespace separator in answers and reference namespace separator from constants. - Use self.config if no config supplied in Nulecule load_config. - Store CLI data in Nulecule config. - During run/stop, reference config for provider. We are overriding provider data in config with cli provider, but when referencing provider info, we always do it from a single source of truth, config, in this case. - Update docs for Nulecule config. - Added helper in Config to retrieve resolved global params. - Centralize handling CLI answers in NuleculeManager. - process answers and cli_answers to create a nulecule.config.Config object - Include default provider, namespace in runtime answers, if missing. - Config takes default provider in spec into account. - Fixed unittests for refactored config. --- atomicapp/constants.py | 1 + atomicapp/nulecule/base.py | 51 ++-- atomicapp/nulecule/config.py | 192 ++++++++++++++ atomicapp/nulecule/lib.py | 37 +-- atomicapp/nulecule/main.py | 56 ++--- tests/units/nulecule/test_lib.py | 5 +- tests/units/nulecule/test_nulecule.py | 238 +++++++++++++----- .../units/nulecule/test_nulecule_component.py | 41 +-- 8 files changed, 461 insertions(+), 160 deletions(-) create mode 100644 atomicapp/nulecule/config.py diff --git a/atomicapp/constants.py b/atomicapp/constants.py index 60f4e94c..7887d505 100644 --- a/atomicapp/constants.py +++ b/atomicapp/constants.py @@ -40,6 +40,7 @@ DEFAULTNAME_KEY = "default" PROVIDER_KEY = "provider" NAMESPACE_KEY = "namespace" +NAMESPACE_SEPARATOR = ":" REQUIREMENTS_KEY = "requirements" # Nulecule spec terminology vs the function within /providers diff --git a/atomicapp/nulecule/base.py b/atomicapp/nulecule/base.py index 8283f141..3332ef5b 100644 --- a/atomicapp/nulecule/base.py +++ b/atomicapp/nulecule/base.py @@ -18,7 +18,6 @@ along with Atomic App. If not, see . """ import anymarkup -import copy import logging import os import yaml @@ -38,7 +37,7 @@ NAME_KEY, INHERIT_KEY, ARTIFACTS_KEY, - DEFAULT_PROVIDER) + NAMESPACE_SEPARATOR) from atomicapp.utils import Utils from atomicapp.requirements import Requirements from atomicapp.nulecule.lib import NuleculeBase @@ -76,7 +75,7 @@ def __init__(self, id, specversion, graph, basepath, metadata=None, metadata (dict): Nulecule metadata requirements (dict): Requirements for the Nulecule application params (list): List of params for the Nulecule application - config (dict): Config data for the Nulecule application + config (atomicapp.nulecule.config.Config): Config data namespace (str): Namespace of the current Nulecule application Returns: @@ -88,7 +87,7 @@ def __init__(self, id, specversion, graph, basepath, metadata=None, self.metadata = metadata or {} self.graph = graph self.requirements = requirements - self.config = config or {} + self.config = config @classmethod def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF, @@ -101,7 +100,7 @@ def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF, image (str): A Docker image name. dest (str): Destination path where Nulecule data from Docker image should be extracted. - config (dict): Dictionary, config data for Nulecule application. + config: An instance of atomicapp.nulecule.config.Config namespace (str): Namespace for Nulecule application. nodeps (bool): Don't pull external Nulecule dependencies when True. @@ -115,7 +114,7 @@ def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF, if Utils.running_on_openshift(): # pass general config data containing provider specific data # to Openshift provider - op = OpenshiftProvider(config.get('general', {}), './', False) + op = OpenshiftProvider(config.globals, './', False) op.artifacts = [] op.init() op.extract(image, APP_ENT_PATH, dest, update) @@ -138,7 +137,8 @@ def load_from_path(cls, src, config=None, namespace=GLOBAL_CONF, Args: src (str): Path to load Nulecule application from. - config (dict): Config data for Nulecule application. + config (atomicapp.nulecule.config.Config): Config data for + Nulecule application. namespace (str): Namespace for Nulecule application. nodeps (bool): Do not pull external applications if True. dryrun (bool): Do not make any change to underlying host. @@ -176,8 +176,9 @@ def load_from_path(cls, src, config=None, namespace=GLOBAL_CONF, raise NuleculeException("Failure parsing %s file. Validation error on line %s, column %s:\n%s" % (nulecule_path, line, column, output)) - nulecule = Nulecule(config=config, basepath=src, - namespace=namespace, **nulecule_data) + nulecule = Nulecule(config=config, + basepath=src, namespace=namespace, + **nulecule_data) nulecule.load_components(nodeps, dryrun) return nulecule @@ -231,25 +232,23 @@ def load_config(self, config=None, ask=False, skip_asking=False): It updates self.config. Args: - config (dict): Existing config data, may be from ANSWERS - file or any other source. + config (atomicapp.nulecule.config.Config): Existing config data, + may be from ANSWERS file or any other source. Returns: None """ + if config is None: + config = self.config super(Nulecule, self).load_config( config=config, ask=ask, skip_asking=skip_asking) - if self.namespace == GLOBAL_CONF and self.config[GLOBAL_CONF].get('provider') is None: - self.config[GLOBAL_CONF]['provider'] = DEFAULT_PROVIDER - logger.info("Provider not specified, using default provider - {}". - format(DEFAULT_PROVIDER)) + for component in self.components: # FIXME: Find a better way to expose config data to components. # A component should not get access to all the variables, # but only to variables it needs. - component.load_config(config=copy.deepcopy(self.config), + component.load_config(config=config.clone(component.namespace), ask=ask, skip_asking=skip_asking) - self.merge_config(self.config, component.config) def load_components(self, nodeps=False, dryrun=False): """ @@ -270,8 +269,8 @@ def load_components(self, nodeps=False, dryrun=False): node_name = node[NAME_KEY] source = Utils.getSourceImage(node) component = NuleculeComponent( - node_name, self.basepath, source, - node.get(PARAMS_KEY), node.get(ARTIFACTS_KEY), + self._get_component_namespace(node_name), self.basepath, + source, node.get(PARAMS_KEY), node.get(ARTIFACTS_KEY), self.config) component.load(nodeps, dryrun) components.append(component) @@ -294,6 +293,12 @@ def render(self, provider_key=None, dryrun=False): for component in self.components: component.render(provider_key=provider_key, dryrun=dryrun) + def _get_component_namespace(self, component_name): + current_namespace = '' if self.namespace == GLOBAL_CONF else self.namespace + return ( + '%s%s%s' % (current_namespace, NAMESPACE_SEPARATOR, component_name) + if current_namespace else component_name) + class NuleculeComponent(NuleculeBase): @@ -356,12 +361,13 @@ def load_config(self, config=None, ask=False, skip_asking=False): """ Load config for the Nulecule component. """ + if config is None: + config = self.config super(NuleculeComponent, self).load_config( config, ask=ask, skip_asking=skip_asking) if isinstance(self._app, Nulecule): - self._app.load_config(config=copy.deepcopy(self.config), + self._app.load_config(config=self.config.clone(self.namespace), ask=ask, skip_asking=skip_asking) - self.merge_config(self.config, self._app.config) def load_external_application(self, dryrun=False, update=False): """ @@ -384,7 +390,8 @@ def load_external_application(self, dryrun=False, update=False): 'Found existing external application: %s ' 'Loading: ' % self.name) nulecule = Nulecule.load_from_path( - external_app_path, dryrun=dryrun, update=update) + external_app_path, dryrun=dryrun, update=update, + namespace=self.namespace) elif not dryrun: logger.info('Pulling external application: %s' % self.name) nulecule = Nulecule.unpack( diff --git a/atomicapp/nulecule/config.py b/atomicapp/nulecule/config.py new file mode 100644 index 00000000..f496ce1d --- /dev/null +++ b/atomicapp/nulecule/config.py @@ -0,0 +1,192 @@ +import copy +import logging + +from atomicapp.constants import (GLOBAL_CONF, + LOGGER_COCKPIT, + DEFAULT_PROVIDER, + DEFAULT_ANSWERS, + NAMESPACE_SEPARATOR) +from collections import defaultdict + +cockpit_logger = logging.getLogger(LOGGER_COCKPIT) + + +class Config(object): + """ + Store config data for a Nulecule or Nulecule component. + + It stores config data from different sources (answers, cli, user data) + separately, and exposes high level interfaces to read, write data + from it, with enforced read/write policies. + """ + + def __init__(self, namespace='', answers=None, cli=None, + data=None, is_nulecule=False): + self._namespace = namespace + self._is_nulecule = is_nulecule or False + self._parent_ns, self._current_ns = self._split_namespace(self._namespace) + # Store answers data + self._answers = defaultdict(dict) + self._answers.update(answers or {}) + # Store CLI data + self._cli = cli or {} + # Store data collected during runtime + self._data = data or defaultdict(dict) + self._context = None + self._provider = None + + @property + def globals(self): + """ + Get global config params dict for a Nulecule. + """ + d = self._answers.get(GLOBAL_CONF, {}) + d.update(self._data.get(GLOBAL_CONF, {})) + d.update(self._cli.get(GLOBAL_CONF, {})) + return d + + @property + def provider(self): + """ + Get provider name. + + Returns: + Provider name (str) + """ + if self._provider is None: + self._provider = self._data[GLOBAL_CONF].get('provider') or \ + self._answers[GLOBAL_CONF].get('provider') + if self._provider is None: + self._data[GLOBAL_CONF]['provider'] = DEFAULT_PROVIDER + self._provider = DEFAULT_PROVIDER + + return self._provider + + @property + def providerconfig(self): + """ + Get provider config info taking into account answers and cli data. + """ + pass + + @property + def namespace(self): + """ + Get normalized namespace for this instance. + + Returns: + Current namespace (str). + """ + return self._namespace or GLOBAL_CONF + + def set(self, key, value): + """ + Set value for a key in the current namespace. + + Args: + key (str): Key + value (str): Value. + """ + self._data[self.namespace][key] = value + + def get(self, key): + """ + Get value for a key from data accessible from the current namespace. + + TODO: Improved data inheritance model. It makes sense for a component + to be able to access data from it's sibling namespaces and children + namespaces. + + Args: + key (str): Key + + Returns: + Value for the key, else None. + """ + return ( + self._data[self.namespace].get(key) or + (self._data[self._parent_ns].get(key) if self._parent_ns else None) or + self._data[GLOBAL_CONF].get(key) or + self._answers[self.namespace].get(key) or + (self._answers[self._parent_ns].get(key) if self._parent_ns else None) or + self._answers[GLOBAL_CONF].get(key) + ) + + def context(self): + """ + Get context to render artifact files in a Nulecule component. + + TODO: Improved data inheritance model. Data from siblings and children + namespaces should be available in the context to render an artifact + file in the current namespace. + """ + if self._context is None: + self._context = {} + self._context.update(copy.copy(self._data[GLOBAL_CONF])) + self._context.update(copy.copy(self._data[self.namespace])) + + self._context.update(copy.copy(self._answers[GLOBAL_CONF])) + self._context.update(copy.copy(self._answers[self.namespace])) + return self._context + + def runtime_answers(self): + """ + Get runtime answers. + + Returns: + A defaultdict containing runtime answers data. + """ + answers = defaultdict(dict) + answers.update(copy.deepcopy(DEFAULT_ANSWERS)) + answers['general']['provider'] = self.provider + + for key, value in self._answers.items(): + answers[key].update(value) + + for key, value in self._data.items(): + answers[key].update(value) + + # Remove empty sections for answers + for key, value in answers.items(): + if value is None: + answers.pop(key, None) + + return answers + + def clone(self, namespace): + """ + Create a new config instance in the specified namespace. + + Args: + name (str): Name of the child component + + Returns: + A Config instance. + """ + config = Config(namespace=namespace, + answers=self._answers, + cli=self._cli, + data=self._data) + return config + + def _split_namespace(self, namespace): + """ + Split namespace to get parent and current namespace in a Nulecule. + """ + if self._is_nulecule: + return '', namespace + words = namespace.rsplit(NAMESPACE_SEPARATOR, 1) + parent, current = '', '' + if len(words) == 2: + parent, current = words[0], words[1] + else: + parent, current = '', words[0] + return parent, current + + def __eq__(self, obj): + """ + Check equality of config instances. + """ + if self._namespace == obj._namespace or self._answers == obj._answers or self._data == obj._data or self._cli == obj._cli: + return True + return False diff --git a/atomicapp/nulecule/lib.py b/atomicapp/nulecule/lib.py index 0f2d1539..0a82aeeb 100644 --- a/atomicapp/nulecule/lib.py +++ b/atomicapp/nulecule/lib.py @@ -19,11 +19,9 @@ """ import logging -from atomicapp.constants import (GLOBAL_CONF, - LOGGER_COCKPIT, +from atomicapp.constants import (LOGGER_COCKPIT, NAME_KEY, DEFAULTNAME_KEY, - PROVIDER_KEY, PROVIDERS) from atomicapp.utils import Utils from atomicapp.plugin import Plugin @@ -64,46 +62,21 @@ def load_config(self, config, ask=False, skip_asking=False): None """ for param in self.params: - value = config.get(self.namespace, {}).get(param[NAME_KEY]) or \ - config.get(GLOBAL_CONF, {}).get(param[NAME_KEY]) + value = config.get(param[NAME_KEY]) if value is None and (ask or ( not skip_asking and param.get(DEFAULTNAME_KEY) is None)): cockpit_logger.info("%s is missing in answers.conf." % param[NAME_KEY]) value = Utils.askFor(param[NAME_KEY], param, self.namespace) elif value is None: value = param.get(DEFAULTNAME_KEY) - if config.get(self.namespace) is None: - config[self.namespace] = {} - config[self.namespace][param[NAME_KEY]] = value + config.set(param[NAME_KEY], value) self.config = config - def merge_config(self, to_config, from_config): - """ - Merge values from from_config to to_config. If value for a key - in a group in to_config is missing, then only set it's value from - corresponding key in the same group in from_config. - - Args: - to_config (dict): Dictionary to merge config into - from_config (dict): Dictionary to merge config from - - Returns: - None - """ - for group, group_vars in from_config.items(): - to_config[group] = to_config.get(group) or {} - for key, value in (group_vars or {}).items(): - if to_config[group].get(key) is None: - to_config[group][key] = value - def get_context(self): """ Get context data from config data for rendering an artifact. """ - context = {} - context.update(self.config.get(GLOBAL_CONF) or {}) - context.update(self.config.get(self.namespace) or {}) - return context + return self.config.context() def get_provider(self, provider_key=None, dry=False): """ @@ -118,7 +91,7 @@ def get_provider(self, provider_key=None, dry=False): """ # If provider_key isn't provided via CLI, let's grab it the configuration if provider_key is None: - provider_key = self.config.get(GLOBAL_CONF)[PROVIDER_KEY] + provider_key = self.config.provider provider_class = self.plugin.getProvider(provider_key) if provider_class is None: raise NuleculeException("Invalid Provider - '{}', provided in " diff --git a/atomicapp/nulecule/main.py b/atomicapp/nulecule/main.py index a27577bb..a63ff94e 100644 --- a/atomicapp/nulecule/main.py +++ b/atomicapp/nulecule/main.py @@ -36,11 +36,11 @@ LOGGER_COCKPIT, LOGGER_DEFAULT, MAIN_FILE, - PROVIDER_KEY, __ATOMICAPPVERSION__, __NULECULESPECVERSION__) from atomicapp.nulecule.base import Nulecule from atomicapp.nulecule.exceptions import NuleculeException +from atomicapp.nulecule.config import Config from atomicapp.utils import Utils cockpit_logger = logging.getLogger(LOGGER_COCKPIT) @@ -64,6 +64,7 @@ def __init__(self, app_spec, destination=None, destination: where to unpack a nulecule to if it isn't local cli_answers: some answer file values provided from cli args answers_file: the location of the answers file + cli (dict): CLI data """ self.answers = copy.deepcopy(DEFAULT_ANSWERS) self.cli_answers = cli_answers @@ -240,12 +241,12 @@ def genanswers(self, dryrun=False, answers_format=None, **kwargs): if os.path.exists(answers_file): raise NuleculeException( "Can't generate answers.conf over existing file") + self.config = Config(namespace=GLOBAL_CONF) # Call unpack to get the app code - self.nulecule = self.unpack(update=False, dryrun=dryrun, config=self.answers) + self.nulecule = self.unpack(update=False, dryrun=dryrun, config=self.config) - self.nulecule.load_config(config=self.nulecule.config, - skip_asking=True) + self.nulecule.load_config(skip_asking=True) # Get answers and write them out to answers.conf in cwd answers = self._get_runtime_answers( self.nulecule.config, None) @@ -272,10 +273,9 @@ def fetch(self, nodeps=False, update=False, dryrun=False, # Call unpack. If the app doesn't exist it will be pulled. If # it does exist it will be just be loaded and returned - self.nulecule = self.unpack(update, dryrun, config=self.answers) + self.nulecule = self.unpack(update, dryrun, config=self.config) - self.nulecule.load_config(config=self.nulecule.config, - skip_asking=True) + self.nulecule.load_config(skip_asking=True) runtime_answers = self._get_runtime_answers( self.nulecule.config, None) # write sample answers file @@ -283,7 +283,7 @@ def fetch(self, nodeps=False, update=False, dryrun=False, os.path.join(self.app_path, ANSWERS_FILE_SAMPLE), runtime_answers, answers_format) - cockpit_logger.info("Install Successful.") + cockpit_logger.info("Install Successful.") def run(self, cli_provider, answers_output, ask, answers_format=ANSWERS_FILE_SAMPLE_FORMAT, **kwargs): @@ -308,16 +308,18 @@ def run(self, cli_provider, answers_output, ask, self.answers_format = answers_format or ANSWERS_FILE_SAMPLE_FORMAT dryrun = kwargs.get('dryrun') or False - # Call unpack. If the app doesn't exist it will be pulled. If - # it does exist it will be just be loaded and returned - self.nulecule = self.unpack(dryrun=dryrun, config=self.answers) - # If we didn't find an answers file before then call _process_answers # again just in case the app developer embedded an answers file if not self.answers_file: self._process_answers() - self.nulecule.load_config(config=self.nulecule.config, ask=ask) + # Call unpack. If the app doesn't exist it will be pulled. If + # it does exist it will be just be loaded and returned + self.nulecule = self.unpack(dryrun=dryrun, config=self.config) + + self.nulecule.load_config(ask=ask) + if cli_provider: + self.nulecule.config.set('provider', cli_provider) self.nulecule.render(cli_provider, dryrun) self.nulecule.run(cli_provider, dryrun) runtime_answers = self._get_runtime_answers( @@ -343,10 +345,12 @@ def stop(self, cli_provider, **kwargs): dryrun = kwargs.get('dryrun') or False self.nulecule = Nulecule.load_from_path( - self.app_path, config=self.answers, dryrun=dryrun) - self.nulecule.load_config(config=self.answers) - self.nulecule.render(cli_provider, dryrun=dryrun) - self.nulecule.stop(cli_provider, dryrun) + self.app_path, config=self.config, dryrun=dryrun) + self.nulecule.load_config() + if cli_provider: + self.nulecule.config.set('provider', cli_provider) + self.nulecule.render(self.nulecule.config.provider, dryrun=dryrun) + self.nulecule.stop(self.nulecule.config.provider, dryrun) def clean(self, force=False): # For future use @@ -396,10 +400,13 @@ def _process_answers(self): # Load answers self.answers = Utils.loadAnswers(self.answers_file) + self.config = Config(namespace=GLOBAL_CONF, answers=self.answers, + cli={GLOBAL_CONF: self.cli_answers}) + # If there is answers data from the cli then merge it in now - if self.cli_answers: - for k, v in self.cli_answers.iteritems(): - self.answers[GLOBAL_CONF][k] = v + # if self.cli_answers: + # for k, v in self.cli_answers.iteritems(): + # self.answers[GLOBAL_CONF][k] = v def _write_answers(self, path, answers, answers_format): """ @@ -438,11 +445,4 @@ def _get_runtime_answers(self, config, cli_provider): Returns: dict """ - _config = copy.deepcopy(config) - _config[GLOBAL_CONF] = config.get(GLOBAL_CONF) or {} - - # If a provider is provided via CLI, override the config parameter - if cli_provider: - _config[GLOBAL_CONF][PROVIDER_KEY] = cli_provider - - return _config + return self.nulecule.config.runtime_answers() diff --git a/tests/units/nulecule/test_lib.py b/tests/units/nulecule/test_lib.py index 742738d8..d9e56dac 100644 --- a/tests/units/nulecule/test_lib.py +++ b/tests/units/nulecule/test_lib.py @@ -3,6 +3,7 @@ from atomicapp.nulecule.lib import NuleculeBase from atomicapp.nulecule.exceptions import NuleculeException +from atomicapp.nulecule.config import Config class TestNuleculeBaseGetProvider(unittest.TestCase): @@ -16,7 +17,7 @@ def test_get_provider_success(self): provider_key = u'openshift' # method `get_provider` will read from this config, we give it here # since we have neither provided it before nor it is auto-generated - nb.config = {u'general': {u'provider': provider_key}} + nb.config = Config(answers={u'general': {u'provider': provider_key}}) return_provider = mock.Mock() # mocking return value of method plugin.getProvider,because it returns @@ -35,6 +36,6 @@ def test_get_provider_failure(self): nb = NuleculeBase(params = [], basepath = '', namespace = '') # purposefully give the wrong provider key provider_key = u'mesos' - nb.config = {u'general': {u'provider': provider_key}} + nb.config = Config(answers={u'general': {u'provider': provider_key}}) with self.assertRaises(NuleculeException): nb.get_provider() diff --git a/tests/units/nulecule/test_nulecule.py b/tests/units/nulecule/test_nulecule.py index 1bb60179..0f3a3372 100644 --- a/tests/units/nulecule/test_nulecule.py +++ b/tests/units/nulecule/test_nulecule.py @@ -4,6 +4,7 @@ import os from atomicapp.nulecule.base import Nulecule from atomicapp.nulecule.exceptions import NuleculeException +from atomicapp.nulecule.config import Config class TestNuleculeRun(unittest.TestCase): @@ -15,8 +16,9 @@ def test_run(self): dryrun = False mock_component_1 = mock.Mock() mock_component_2 = mock.Mock() + config = Config(answers={}) - n = Nulecule('some-id', '0.0.2', [{}], 'some/path', {}) + n = Nulecule('some-id', '0.0.2', [{}], 'some/path', {}, config=config) n.components = [mock_component_1, mock_component_2] n.run(provider) @@ -34,7 +36,9 @@ def test_stop(self): mock_component_1 = mock.Mock() mock_component_2 = mock.Mock() - n = Nulecule('some-id', '0.0.2', {}, [], 'some/path') + config = Config(answers={}) + + n = Nulecule('some-id', '0.0.2', {}, [], 'some/path', config=config) n.components = [mock_component_1, mock_component_2] n.stop(provider) @@ -46,74 +50,194 @@ class TestNuleculeLoadConfig(unittest.TestCase): """Test Nulecule load_config""" - def test_load_config_without_specified_provider(self): + def test_load_config_with_default_provider(self): """ - Test Nulecule load_config without specifying a provider. + Test Nulecule load_config with a default provider. """ - config = {'general': {}, 'group1': {'a': 'b'}} - mock_component_1 = mock.Mock() - mock_component_1.config = { - 'group1': {'a': 'c', 'k': 'v'}, - 'group2': {'1': '2'} - } + config = Config(answers={}) + + params = [ + { + "name": "key1", + "default": "val1", + }, + { + "name": "key3", + "default": "val3" + }, + { + "name": "provider", + "default": "docker" + } + ] + + graph = [ + { + "name": "component1", + "params": [ + { + "name": "key1", + }, + { + "name": "key2", + "default": "val2" + } + ], + "artifacts": [] + } + ] - n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, graph=[], basepath='some/path') - n.components = [mock_component_1] + n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, + graph=graph, params=params, basepath='some/path', + config=config) + n.load_components() n.load_config(config) - self.assertEqual(n.config, { - 'general': {'provider': 'kubernetes'}, - 'group1': {'a': 'b', 'k': 'v'}, - 'group2': {'1': '2'} + self.assertEqual(n.config.runtime_answers(), { + 'general': { + 'namespace': 'default', + 'provider': 'docker', + 'key1': 'val1', + 'key3': 'val3' + }, + 'component1': { + 'key2': 'val2', + 'key1': 'val1' + } + }) + + self.assertEqual(n.components[0].config.context(), { + 'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'provider': 'docker' }) - def test_load_config_with_defaultprovider(self): + def test_load_config_without_default_provider(self): """ - Test Nulecule load_config with default provider specified - in global params in Nulecule spec. + Test Nulecule load_config without specifying a default provider. """ - config = {'general': {}, 'group1': {'a': 'b'}} - mock_component_1 = mock.Mock() - mock_component_1.config = { - 'group1': {'a': 'c', 'k': 'v'}, - 'group2': {'1': '2'} - } - - n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, graph=[], - basepath='some/path', - params=[{'name': 'provider', 'default': 'some-provider'}]) - n.components = [mock_component_1] + config = Config(answers={}) + + params = [ + { + "name": "key1", + "default": "val1", + }, + { + "name": "key3", + "default": "val3" + } + ] + + graph = [ + { + "name": "component1", + "params": [ + { + "name": "key1", + }, + { + "name": "key2", + "default": "val2" + } + ], + "artifacts": [] + } + ] + + n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, + graph=graph, params=params, basepath='some/path', + config=config) + n.load_components() n.load_config(config) - self.assertEqual(n.config, { - 'general': {'provider': 'some-provider'}, - 'group1': {'a': 'b', 'k': 'v'}, - 'group2': {'1': '2'} + self.assertEqual(n.config.runtime_answers(), { + 'general': { + 'namespace': 'default', + 'provider': 'kubernetes', + 'key1': 'val1', + 'key3': 'val3' + }, + 'component1': { + 'key2': 'val2', + 'key1': 'val1' + } + }) + + self.assertEqual(n.components[0].config.context(), { + 'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'provider': 'kubernetes' }) - def test_load_config_with_defaultprovider_overridden_by_provider_in_answers(self): + def test_load_config_with_default_provider_overridden_by_answers(self): """ - Test Nulecule load_config with default provider specified - in global params in Nulecule spec, but overridden in answers config. + Test Nulecule load_config with default provider overridden by provider + in answers. """ - config = {'general': {'provider': 'new-provider'}, - 'group1': {'a': 'b'}} - mock_component_1 = mock.Mock() - mock_component_1.config = { - 'group1': {'a': 'c', 'k': 'v'}, - 'group2': {'1': '2'} - } - - n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, graph=[], - basepath='some/path', - params=[{'name': 'provider', 'default': 'some-provider'}]) - n.components = [mock_component_1] + config = Config(answers={ + 'general': { + 'provider': 'openshift' + } + }) + + params = [ + { + "name": "key1", + "default": "val1", + }, + { + "name": "key3", + "default": "val3" + }, + { + "name": "provider", + "default": "docker" + } + ] + + graph = [ + { + "name": "component1", + "params": [ + { + "name": "key1", + }, + { + "name": "key2", + "default": "val2" + } + ], + "artifacts": [] + } + ] + + n = Nulecule(id='some-id', specversion='0.0.2', metadata={}, + graph=graph, params=params, basepath='some/path', + config=config) + n.load_components() n.load_config(config) - self.assertEqual(n.config, { - 'general': {'provider': 'new-provider'}, - 'group1': {'a': 'b', 'k': 'v'}, - 'group2': {'1': '2'} + self.assertEqual(n.config.runtime_answers(), { + 'general': { + 'namespace': 'default', + 'provider': 'openshift', + 'key1': 'val1', + 'key3': 'val3' + }, + 'component1': { + 'key2': 'val2', + 'key1': 'val1' + } + }) + + self.assertEqual(n.components[0].config.context(), { + 'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'provider': 'openshift' }) @@ -137,15 +261,17 @@ def test_load_components(self, MockNuleculeComponent): } ] - n = Nulecule('some-id', '0.0.2', graph, 'some/path', {}) + config = Config(answers={}) + + n = Nulecule('some-id', '0.0.2', graph, 'some/path', config=config) n.load_components() MockNuleculeComponent.assert_any_call( graph[0]['name'], n.basepath, 'somecontainer', - graph[0]['params'], None, {}) + graph[0]['params'], None, config) MockNuleculeComponent.assert_any_call( graph[1]['name'], n.basepath, None, - graph[1].get('params'), graph[1].get('artifacts'), {}) + graph[1].get('params'), graph[1].get('artifacts'), config) class TestNuleculeRender(unittest.TestCase): diff --git a/tests/units/nulecule/test_nulecule_component.py b/tests/units/nulecule/test_nulecule_component.py index be2a8ab7..4bbcb418 100644 --- a/tests/units/nulecule/test_nulecule_component.py +++ b/tests/units/nulecule/test_nulecule_component.py @@ -1,7 +1,7 @@ -import copy import mock import unittest from atomicapp.nulecule.base import NuleculeComponent, Nulecule +from atomicapp.nulecule.config import Config from atomicapp.nulecule.exceptions import NuleculeException @@ -129,49 +129,49 @@ class TestNuleculeComponentLoadConfig(unittest.TestCase): def test_load_config_local_app(self): """Test load config for local app""" params = [ - {'name': 'key1'}, - {'name': 'key2'} + {'name': 'key1', 'description': 'key1'}, + {'name': 'key2', 'description': 'key2'} ] initial_config = { 'general': {'a': 'b', 'key2': 'val2'}, 'some-app': {'key1': 'val1'} } - - nc = NuleculeComponent('some-app', 'some/path', params=params) - nc.load_config(config=copy.deepcopy(initial_config)) - - self.assertEqual(nc.config, { - 'general': {'a': 'b', 'key2': 'val2'}, + conf = Config('some-app', answers=initial_config) + + nc = NuleculeComponent('some-app', 'some/path', + params=params, config=conf) + nc.load_config() + runtime_answers = nc.config.runtime_answers() + self.assertEqual(runtime_answers, { + 'general': {'a': 'b', 'key2': 'val2', 'provider': 'kubernetes', 'namespace': 'default'}, 'some-app': {'key1': 'val1', 'key2': 'val2'} }) - @mock.patch('atomicapp.nulecule.base.NuleculeComponent.merge_config') - def test_load_config_external_app(self, mock_merge_config): + def test_load_config_external_app(self): """Test load config for external app""" mock_nulecule = mock.Mock( name='nulecule', spec=Nulecule('some-id', '0.0.2', {}, [], 'some/path') ) params = [ - {'name': 'key1'}, - {'name': 'key2'} + {'name': 'key1', 'description': 'key1'}, + {'name': 'key2', 'description': 'key2'} ] initial_config = { 'general': {'a': 'b', 'key2': 'val2'}, 'some-app': {'key1': 'val1'} } + config = Config('some-app', answers=initial_config) nc = NuleculeComponent('some-app', 'some/path', params=params) nc._app = mock_nulecule - nc.load_config(config=copy.deepcopy(initial_config)) + nc.load_config(config=config) mock_nulecule.load_config.assert_called_once_with( - config={ - 'general': {'a': 'b', 'key2': 'val2'}, + config=Config('some-app', answers=initial_config, data={ + 'general': {}, 'some-app': {'key1': 'val1', 'key2': 'val2'} - }, ask=False, skip_asking=False) - mock_merge_config.assert_called_once_with( - nc.config, mock_nulecule.config) + }), ask=False, skip_asking=False) class TestNuleculeComponentLoadExternalApplication(unittest.TestCase): @@ -193,7 +193,8 @@ def test_loading_existing_app(self, mock_os_path_isdir, mock_Nulecule): mock_os_path_isdir.assert_called_once_with( expected_external_app_path) mock_Nulecule.load_from_path.assert_called_once_with( - expected_external_app_path, dryrun=dryrun, update=update) + expected_external_app_path, dryrun=dryrun, namespace='some-app', + update=update) # Use http://engineeringblog.yelp.com/2015/02/assert_called_once-threat-or-menace.html # by calling call_count == 1. In order to avoid the return_value = False of Utils.setFileOnwerGroup From d08844910e44b74dee5ff5d8a68a2a1d781724cb Mon Sep 17 00:00:00 2001 From: Ratnadeep Debnath Date: Thu, 21 Jul 2016 17:41:45 +0530 Subject: [PATCH 2/3] Re implememt Config class to be more generic. Fixes #524 - Create single source of truth for config data in NuleculeManager. Get rid of answers and cli_answers instance variables, and use only self.config to look up config data. - Allow ignoring sources when getting data from config. - Allow specifying file format when loading answers. --- atomicapp/cli/main.py | 11 +- atomicapp/nulecule/base.py | 23 +++- atomicapp/nulecule/config.py | 255 ++++++++++++++++------------------- atomicapp/nulecule/lib.py | 35 +++-- atomicapp/nulecule/main.py | 74 ++++------ atomicapp/utils.py | 13 +- 6 files changed, 194 insertions(+), 217 deletions(-) diff --git a/atomicapp/cli/main.py b/atomicapp/cli/main.py index 9d1a9840..47bdafa4 100644 --- a/atomicapp/cli/main.py +++ b/atomicapp/cli/main.py @@ -64,7 +64,8 @@ def cli_fetch(args): nm = NuleculeManager(app_spec=argdict['app_spec'], destination=destination, cli_answers=argdict['cli_answers'], - answers_file=argdict['answers']) + answers_file=argdict['answers'], + answers_format=argdict.get('answers_format')) nm.fetch(**argdict) # Clean up the files if the user asked us to. Otherwise # notify the user where they can manage the application @@ -81,7 +82,8 @@ def cli_run(args): nm = NuleculeManager(app_spec=argdict['app_spec'], destination=destination, cli_answers=argdict['cli_answers'], - answers_file=argdict['answers']) + answers_file=argdict['answers'], + answers_format=argdict.get('answers_format')) nm.run(**argdict) # Clean up the files if the user asked us to. Otherwise # notify the user where they can manage the application @@ -306,7 +308,7 @@ def create_parser(self): help="A file which will contain anwsers provided in interactive mode") run_subparser.add_argument( "--provider", - dest="cli_provider", + dest="provider", choices=PROVIDERS, help="The provider to use. Overrides provider value in answerfile.") run_subparser.add_argument( @@ -511,7 +513,8 @@ def run(self): # and make a dictionary of it to pass along in args. setattr(args, 'cli_answers', {}) for item in ['provider-api', 'provider-cafile', 'provider-auth', - 'provider-config', 'provider-tlsverify', 'namespace']: + 'provider-config', 'provider-tlsverify', 'namespace', + 'provider']: if hasattr(args, item) and getattr(args, item) is not None: args.cli_answers[item] = getattr(args, item) diff --git a/atomicapp/nulecule/base.py b/atomicapp/nulecule/base.py index 3332ef5b..d0d76347 100644 --- a/atomicapp/nulecule/base.py +++ b/atomicapp/nulecule/base.py @@ -176,9 +176,8 @@ def load_from_path(cls, src, config=None, namespace=GLOBAL_CONF, raise NuleculeException("Failure parsing %s file. Validation error on line %s, column %s:\n%s" % (nulecule_path, line, column, output)) - nulecule = Nulecule(config=config, - basepath=src, namespace=namespace, - **nulecule_data) + nulecule = Nulecule(config=config, basepath=src, + namespace=namespace, **nulecule_data) nulecule.load_components(nodeps, dryrun) return nulecule @@ -247,7 +246,7 @@ def load_config(self, config=None, ask=False, skip_asking=False): # FIXME: Find a better way to expose config data to components. # A component should not get access to all the variables, # but only to variables it needs. - component.load_config(config=config.clone(component.namespace), + component.load_config(config=config, ask=ask, skip_asking=skip_asking) def load_components(self, nodeps=False, dryrun=False): @@ -294,6 +293,18 @@ def render(self, provider_key=None, dryrun=False): component.render(provider_key=provider_key, dryrun=dryrun) def _get_component_namespace(self, component_name): + """ + Get a unique namespace for a Nulecule graph item, by concatinating + the namespace of the current Nulecule (which could be the root Nulecule + app or a child or external Nulecule app) and name of the Nulecule + graph item. + + Args: + component_name (str): Name of the Nulecule graph item + + Returns: + A string + """ current_namespace = '' if self.namespace == GLOBAL_CONF else self.namespace return ( '%s%s%s' % (current_namespace, NAMESPACE_SEPARATOR, component_name) @@ -366,7 +377,7 @@ def load_config(self, config=None, ask=False, skip_asking=False): super(NuleculeComponent, self).load_config( config, ask=ask, skip_asking=skip_asking) if isinstance(self._app, Nulecule): - self._app.load_config(config=self.config.clone(self.namespace), + self._app.load_config(config=self.config, ask=ask, skip_asking=skip_asking) def load_external_application(self, dryrun=False, update=False): @@ -443,7 +454,7 @@ def render(self, provider_key=None, dryrun=False): raise NuleculeException( "Data for provider \"%s\" are not part of this app" % provider_key) - context = self.get_context() + context = self.config.context(self.namespace) for provider in self.artifacts: if provider_key and provider != provider_key: continue diff --git a/atomicapp/nulecule/config.py b/atomicapp/nulecule/config.py index f496ce1d..b1d92b31 100644 --- a/atomicapp/nulecule/config.py +++ b/atomicapp/nulecule/config.py @@ -4,8 +4,7 @@ from atomicapp.constants import (GLOBAL_CONF, LOGGER_COCKPIT, DEFAULT_PROVIDER, - DEFAULT_ANSWERS, - NAMESPACE_SEPARATOR) + DEFAULT_ANSWERS) from collections import defaultdict cockpit_logger = logging.getLogger(LOGGER_COCKPIT) @@ -13,121 +12,125 @@ class Config(object): """ - Store config data for a Nulecule or Nulecule component. - - It stores config data from different sources (answers, cli, user data) - separately, and exposes high level interfaces to read, write data - from it, with enforced read/write policies. + This class allows to store config data in different scopes along with + source info for the data. When fetching the value for a key in a scope, + the source info and the PRIORITY order of sources is taken into account. + + Data sources: + cli: Config data coming from the CLI + runtime: Config data resolved during atomic app runtime. For example, + when the value for a parameter in a Nulecule or Nulecule graph + item is missing in answers data, we first try to load the default + value for the parameter. When there's no default value, or when + the user has specified to forcefully ask the user for values, we + ask the user for data. These data collected/resolved during runtime + form the runtime data. + answers: Config data coming from answers file + defaults: Default config data specified in atomicapp/constants.py + + The priority order of the data sources is: + cli > runtime > answers > defaults """ - def __init__(self, namespace='', answers=None, cli=None, - data=None, is_nulecule=False): - self._namespace = namespace - self._is_nulecule = is_nulecule or False - self._parent_ns, self._current_ns = self._split_namespace(self._namespace) - # Store answers data - self._answers = defaultdict(dict) - self._answers.update(answers or {}) - # Store CLI data - self._cli = cli or {} - # Store data collected during runtime - self._data = data or defaultdict(dict) - self._context = None - self._provider = None - - @property - def globals(self): - """ - Get global config params dict for a Nulecule. - """ - d = self._answers.get(GLOBAL_CONF, {}) - d.update(self._data.get(GLOBAL_CONF, {})) - d.update(self._cli.get(GLOBAL_CONF, {})) - return d + PRIORITY = ( + 'cli', + 'runtime', + 'answers', + 'defaults' + ) - @property - def provider(self): + def __init__(self, answers=None, cli=None): """ - Get provider name. - - Returns: - Provider name (str) - """ - if self._provider is None: - self._provider = self._data[GLOBAL_CONF].get('provider') or \ - self._answers[GLOBAL_CONF].get('provider') - if self._provider is None: - self._data[GLOBAL_CONF]['provider'] = DEFAULT_PROVIDER - self._provider = DEFAULT_PROVIDER - - return self._provider + Initialize a Config instance. - @property - def providerconfig(self): - """ - Get provider config info taking into account answers and cli data. - """ - pass + Args: + answers (dict): Answers data + cli (dict): CLI data + """ + answers = answers or {} + cli = cli or {} + # We use a defaultdict of defaultdicts so that we can avoid doing + # redundant checks in a nested dictionary if the value of the keys + # are dictionaries or None. + self._data = defaultdict(defaultdict) + # Initialize default data dict + self._data['defaults'] = defaultdict(defaultdict) + # Initialize answers data dict + self._data['answers'] = defaultdict(defaultdict) + # Initialize cli data dict + self._data['cli'] = defaultdict(defaultdict) + # Initialize runtime data dict + self._data['runtime'] = defaultdict(defaultdict) + + # Load default answers + for scope, data in DEFAULT_ANSWERS.items(): + for key, value in data.items(): + self.set(key, value, scope=scope, source='defaults') + self.set('provider', DEFAULT_PROVIDER, scope=GLOBAL_CONF, source='defaults') + + # Load answers data + for scope, data in answers.items(): + for key, value in data.items(): + self.set(key, value, scope=scope, source='answers') + + # Load cli data + for key, value in cli.items(): + self.set(key, value, scope=GLOBAL_CONF, source='cli') + + def get(self, key, scope=GLOBAL_CONF, ignore_sources=[]): + """ + Get the value of a key in a scope. This takes care of resolving + the value by going through the PRIORITY order of the various + sources of data. - @property - def namespace(self): - """ - Get normalized namespace for this instance. + Args: + key (str): Key + scope (str): Scope from which to fetch the value for the key Returns: - Current namespace (str). + Value for the key. """ - return self._namespace or GLOBAL_CONF + for source in self.PRIORITY: + if source in ignore_sources: + continue + value = self._data[source][scope].get(key) or self._data[source][ + GLOBAL_CONF].get(key) + if value: + return value + return None - def set(self, key, value): + def set(self, key, value, source, scope=GLOBAL_CONF): """ - Set value for a key in the current namespace. + Set the value for a key within a scope along with specifying the + source of the value. Args: key (str): Key - value (str): Value. + value: Value + scope (str): Scope in which to store the value + source (str): Source of the value """ - self._data[self.namespace][key] = value + self._data[source][scope][key] = value - def get(self, key): + def context(self, scope=GLOBAL_CONF): """ - Get value for a key from data accessible from the current namespace. - - TODO: Improved data inheritance model. It makes sense for a component - to be able to access data from it's sibling namespaces and children - namespaces. + Get context data for the scope of Nulecule graph item by aggregating + the data from various sources taking their priority order into + account. This context data, which is a flat dictionary, is used to + render the variables in the artifacts of Nulecule graph item. Args: - key (str): Key - + scope (str): Scope (or namespace) for the Nulecule graph item. Returns: - Value for the key, else None. + A dictionary """ - return ( - self._data[self.namespace].get(key) or - (self._data[self._parent_ns].get(key) if self._parent_ns else None) or - self._data[GLOBAL_CONF].get(key) or - self._answers[self.namespace].get(key) or - (self._answers[self._parent_ns].get(key) if self._parent_ns else None) or - self._answers[GLOBAL_CONF].get(key) - ) - - def context(self): - """ - Get context to render artifact files in a Nulecule component. - - TODO: Improved data inheritance model. Data from siblings and children - namespaces should be available in the context to render an artifact - file in the current namespace. - """ - if self._context is None: - self._context = {} - self._context.update(copy.copy(self._data[GLOBAL_CONF])) - self._context.update(copy.copy(self._data[self.namespace])) - - self._context.update(copy.copy(self._answers[GLOBAL_CONF])) - self._context.update(copy.copy(self._answers[self.namespace])) - return self._context + result = {} + for source in reversed(self.PRIORITY): + source_data = self._data[source] + result.update(copy.deepcopy(source_data.get(GLOBAL_CONF) or {})) + if scope != GLOBAL_CONF: + result.update(copy.deepcopy(source_data.get(scope) or {})) + return result def runtime_answers(self): """ @@ -137,56 +140,34 @@ def runtime_answers(self): A defaultdict containing runtime answers data. """ answers = defaultdict(dict) - answers.update(copy.deepcopy(DEFAULT_ANSWERS)) - answers['general']['provider'] = self.provider - - for key, value in self._answers.items(): - answers[key].update(value) - for key, value in self._data.items(): - answers[key].update(value) + for source in reversed(self.PRIORITY): + for scope, data in (self._data.get(source) or {}).items(): + answers[scope].update(copy.deepcopy(data)) # Remove empty sections for answers for key, value in answers.items(): - if value is None: - answers.pop(key, None) + if not value: + answers.pop(key) return answers - def clone(self, namespace): + def update_source(self, source, data): """ - Create a new config instance in the specified namespace. + Update answers data for a source. Args: - name (str): Name of the child component - - Returns: - A Config instance. + source (str): Source name + data (dict): Answers data """ - config = Config(namespace=namespace, - answers=self._answers, - cli=self._cli, - data=self._data) - return config + data = data or {} + if source not in self._data: + raise - def _split_namespace(self, namespace): - """ - Split namespace to get parent and current namespace in a Nulecule. - """ - if self._is_nulecule: - return '', namespace - words = namespace.rsplit(NAMESPACE_SEPARATOR, 1) - parent, current = '', '' - if len(words) == 2: - parent, current = words[0], words[1] - else: - parent, current = '', words[0] - return parent, current - - def __eq__(self, obj): - """ - Check equality of config instances. - """ - if self._namespace == obj._namespace or self._answers == obj._answers or self._data == obj._data or self._cli == obj._cli: - return True - return False + # clean up source data + for k in self._data[source]: + self._data[source].pop(k) + + for scope, data in data.items(): + for key, value in data.items(): + self.set(key, value, scope=scope, source=source) diff --git a/atomicapp/nulecule/lib.py b/atomicapp/nulecule/lib.py index 0a82aeeb..6157bda5 100644 --- a/atomicapp/nulecule/lib.py +++ b/atomicapp/nulecule/lib.py @@ -19,7 +19,8 @@ """ import logging -from atomicapp.constants import (LOGGER_COCKPIT, +from atomicapp.constants import (GLOBAL_CONF, + LOGGER_COCKPIT, NAME_KEY, DEFAULTNAME_KEY, PROVIDERS) @@ -61,22 +62,20 @@ def load_config(self, config, ask=False, skip_asking=False): Returns: None """ - for param in self.params: - value = config.get(param[NAME_KEY]) - if value is None and (ask or ( - not skip_asking and param.get(DEFAULTNAME_KEY) is None)): - cockpit_logger.info("%s is missing in answers.conf." % param[NAME_KEY]) - value = Utils.askFor(param[NAME_KEY], param, self.namespace) - elif value is None: - value = param.get(DEFAULTNAME_KEY) - config.set(param[NAME_KEY], value) self.config = config - - def get_context(self): - """ - Get context data from config data for rendering an artifact. - """ - return self.config.context() + for param in self.params: + value = config.get(param[NAME_KEY], scope=self.namespace, ignore_sources=['defaults']) + if value is None: + if ask or (not skip_asking and + param.get(DEFAULTNAME_KEY) is None): + cockpit_logger.info( + "%s is missing in answers.conf." % param[NAME_KEY]) + value = config.get(param[NAME_KEY], scope=self.namespace) \ + or Utils.askFor(param[NAME_KEY], param, self.namespace) + else: + value = param.get(DEFAULTNAME_KEY) + config.set(param[NAME_KEY], value, source='runtime', + scope=self.namespace) def get_provider(self, provider_key=None, dry=False): """ @@ -91,7 +90,7 @@ def get_provider(self, provider_key=None, dry=False): """ # If provider_key isn't provided via CLI, let's grab it the configuration if provider_key is None: - provider_key = self.config.provider + provider_key = self.config.get('provider', scope=GLOBAL_CONF) provider_class = self.plugin.getProvider(provider_key) if provider_class is None: raise NuleculeException("Invalid Provider - '{}', provided in " @@ -99,7 +98,7 @@ def get_provider(self, provider_key=None, dry=False): .format(provider_key, ', ' .join(PROVIDERS))) return provider_key, provider_class( - self.get_context(), self.basepath, dry) + self.config.context(), self.basepath, dry) def run(self, provider_key=None, dry=False): raise NotImplementedError diff --git a/atomicapp/nulecule/main.py b/atomicapp/nulecule/main.py index a63ff94e..cf3e4719 100644 --- a/atomicapp/nulecule/main.py +++ b/atomicapp/nulecule/main.py @@ -18,7 +18,6 @@ along with Atomic App. If not, see . """ import anymarkup -import copy import distutils.dir_util import logging import os @@ -27,12 +26,10 @@ import urllib from string import Template -from atomicapp.constants import (GLOBAL_CONF, - ANSWERS_FILE_SAMPLE_FORMAT, +from atomicapp.constants import (ANSWERS_FILE_SAMPLE_FORMAT, ANSWERS_FILE, ANSWERS_FILE_SAMPLE, ANSWERS_RUNTIME_FILE, - DEFAULT_ANSWERS, LOGGER_COCKPIT, LOGGER_DEFAULT, MAIN_FILE, @@ -54,7 +51,8 @@ class NuleculeManager(object): """ def __init__(self, app_spec, destination=None, - cli_answers=None, answers_file=None): + cli_answers=None, answers_file=None, + answers_format=None): """ init function for NuleculeManager. Sets a few instance variables. @@ -64,11 +62,9 @@ def __init__(self, app_spec, destination=None, destination: where to unpack a nulecule to if it isn't local cli_answers: some answer file values provided from cli args answers_file: the location of the answers file - cli (dict): CLI data + answers_format (str): File format for writing sample answers file """ - self.answers = copy.deepcopy(DEFAULT_ANSWERS) - self.cli_answers = cli_answers - self.answers_format = None + self.answers_format = answers_format or ANSWERS_FILE_SAMPLE_FORMAT self.answers_file = None # The path to an answer file self.app_path = None # The path where the app resides or will reside self.image = None # The container image to pull the app from @@ -119,7 +115,7 @@ def __init__(self, app_spec, destination=None, # Process answers. self.answers_file = answers_file - self._process_answers() + self.config = Config(cli=cli_answers) @staticmethod def init(app_name, destination=None, app_version='1.0', @@ -221,27 +217,24 @@ def unpack(self, update=False, return Nulecule.load_from_path( self.app_path, dryrun=dryrun, config=config) - def genanswers(self, dryrun=False, answers_format=None, **kwargs): + def genanswers(self, dryrun=False, **kwargs): """ Renders artifacts and then generates an answer file. Finally copies answer file to the current working directory. Args: dryrun (bool): Do not make any change to the host system if True - answers_format (str): File format for writing sample answers file kwargs (dict): Extra keyword arguments Returns: None """ - self.answers_format = answers_format or ANSWERS_FILE_SAMPLE_FORMAT # Check to make sure an answers.conf file doesn't exist already answers_file = os.path.join(os.getcwd(), ANSWERS_FILE) if os.path.exists(answers_file): raise NuleculeException( "Can't generate answers.conf over existing file") - self.config = Config(namespace=GLOBAL_CONF) # Call unpack to get the app code self.nulecule = self.unpack(update=False, dryrun=dryrun, config=self.config) @@ -252,8 +245,7 @@ def genanswers(self, dryrun=False, answers_format=None, **kwargs): self.nulecule.config, None) self._write_answers(answers_file, answers, self.answers_format) - def fetch(self, nodeps=False, update=False, dryrun=False, - answers_format=ANSWERS_FILE_SAMPLE_FORMAT, **kwargs): + def fetch(self, nodeps=False, update=False, dryrun=False, **kwargs): """ Installs (unpacks) a Nulecule application from a Nulecule image to a target path. @@ -264,13 +256,10 @@ def fetch(self, nodeps=False, update=False, dryrun=False, update (bool): Pull requisite Nulecule image and install or update already installed Nulecule application dryrun (bool): Do not make any change to the host system if True - answers_format (str): File format for writing sample answers file kwargs (dict): Extra keyword arguments Returns: None """ - self.answers_format = answers_format or ANSWERS_FILE_SAMPLE_FORMAT - # Call unpack. If the app doesn't exist it will be pulled. If # it does exist it will be just be loaded and returned self.nulecule = self.unpack(update, dryrun, config=self.config) @@ -281,49 +270,41 @@ def fetch(self, nodeps=False, update=False, dryrun=False, # write sample answers file self._write_answers( os.path.join(self.app_path, ANSWERS_FILE_SAMPLE), - runtime_answers, answers_format) + runtime_answers, self.answers_format) cockpit_logger.info("Install Successful.") - def run(self, cli_provider, answers_output, ask, - answers_format=ANSWERS_FILE_SAMPLE_FORMAT, **kwargs): + def run(self, answers_output, ask, **kwargs): """ Runs a Nulecule application from a local path or a Nulecule image name. Args: answers (dict or str): Answers data or local path to answers file - cli_provider (str): Provider to use to run the Nulecule - application answers_output (str): Path to file to export runtime answers data to ask (bool): Ask for values for params with default values from user, if True - answers_format (str): File format for writing sample answers file kwargs (dict): Extra keyword arguments Returns: None """ - self.answers_format = answers_format or ANSWERS_FILE_SAMPLE_FORMAT dryrun = kwargs.get('dryrun') or False - # If we didn't find an answers file before then call _process_answers - # again just in case the app developer embedded an answers file - if not self.answers_file: - self._process_answers() - # Call unpack. If the app doesn't exist it will be pulled. If # it does exist it will be just be loaded and returned self.nulecule = self.unpack(dryrun=dryrun, config=self.config) + # Process answers file + self._process_answers() + self.nulecule.load_config(ask=ask) - if cli_provider: - self.nulecule.config.set('provider', cli_provider) - self.nulecule.render(cli_provider, dryrun) - self.nulecule.run(cli_provider, dryrun) + provider = self.nulecule.config.get('provider') + self.nulecule.render(provider, dryrun) + self.nulecule.run(provider, dryrun) runtime_answers = self._get_runtime_answers( - self.nulecule.config, cli_provider) + self.nulecule.config, provider) self._write_answers( os.path.join(self.app_path, ANSWERS_RUNTIME_FILE), runtime_answers, self.answers_format) @@ -331,12 +312,11 @@ def run(self, cli_provider, answers_output, ask, self._write_answers(answers_output, runtime_answers, self.answers_format) - def stop(self, cli_provider, **kwargs): + def stop(self, **kwargs): """ Stops a running Nulecule application. Args: - cli_provider (str): Provider running the Nulecule application kwargs (dict): Extra keyword arguments """ # For stop we use the generated answer file from the run @@ -347,10 +327,9 @@ def stop(self, cli_provider, **kwargs): self.nulecule = Nulecule.load_from_path( self.app_path, config=self.config, dryrun=dryrun) self.nulecule.load_config() - if cli_provider: - self.nulecule.config.set('provider', cli_provider) - self.nulecule.render(self.nulecule.config.provider, dryrun=dryrun) - self.nulecule.stop(self.nulecule.config.provider, dryrun) + self.nulecule.render(self.nulecule.config.get('provider'), + dryrun=dryrun) + self.nulecule.stop(self.nulecule.config.get('provider'), dryrun) def clean(self, force=False): # For future use @@ -372,6 +351,7 @@ def _process_answers(self): Returns: None """ + answers = None app_path_answers = os.path.join(self.app_path, ANSWERS_FILE) # If the user didn't provide an answers file then check the app @@ -398,15 +378,9 @@ def _process_answers(self): "Provided answers file doesn't exist: {}".format(self.answers_file)) # Load answers - self.answers = Utils.loadAnswers(self.answers_file) - - self.config = Config(namespace=GLOBAL_CONF, answers=self.answers, - cli={GLOBAL_CONF: self.cli_answers}) + answers = Utils.loadAnswers(self.answers_file, self.answers_format) - # If there is answers data from the cli then merge it in now - # if self.cli_answers: - # for k, v in self.cli_answers.iteritems(): - # self.answers[GLOBAL_CONF][k] = v + self.config.update_source(source='answers', data=answers) def _write_answers(self, path, answers, answers_format): """ diff --git a/atomicapp/utils.py b/atomicapp/utils.py index 8aeddb10..022f3759 100644 --- a/atomicapp/utils.py +++ b/atomicapp/utils.py @@ -360,13 +360,22 @@ def getUniqueUUID(): return data @staticmethod - def loadAnswers(answers_file): + def loadAnswers(answers_file, format=None): if not os.path.isfile(answers_file): raise AtomicAppUtilsException( "Provided answers file does not exist: %s" % answers_file) logger.debug("Loading answers from file: %s", answers_file) - return anymarkup.parse_file(answers_file) + try: + # Try to load answers file with a specified answers file format + # or the default format. + result = anymarkup.parse_file(answers_file, format=format) + except anymarkup.AnyMarkupError: + # if no answers file format is provided and the answers file + # is not a JSON file, try to load it using anymarkup in a + # generic way. + result = anymarkup.parse_file(answers_file) + return result @staticmethod def copy_dir(src, dest, update=False, dryrun=False): From 685c56a6eb2357ead12bf577be2f70370ccd60dc Mon Sep 17 00:00:00 2001 From: Ratnadeep Debnath Date: Wed, 10 Aug 2016 10:36:05 +0530 Subject: [PATCH 3/3] Fixed tests for config refactor. --- tests/units/cli/test_default_provider.py | 2 +- tests/units/nulecule/test_lib.py | 6 +- tests/units/nulecule/test_nulecule.py | 55 ++++++++++--------- .../units/nulecule/test_nulecule_component.py | 54 ++++++++++-------- 4 files changed, 66 insertions(+), 51 deletions(-) diff --git a/tests/units/cli/test_default_provider.py b/tests/units/cli/test_default_provider.py index fa1e4272..c5e56ec7 100644 --- a/tests/units/cli/test_default_provider.py +++ b/tests/units/cli/test_default_provider.py @@ -70,7 +70,7 @@ def test_run_helloapache_app(self, capsys): print stdout # Since this a Docker-only provider test, docker *should* be in it, NOT Kubernetes - assert "u'provider': u'docker'" in stdout + assert "provider: Docker" in stdout assert "Deploying to Kubernetes" not in stdout assert exec_info.value.code == 0 diff --git a/tests/units/nulecule/test_lib.py b/tests/units/nulecule/test_lib.py index d9e56dac..d313429e 100644 --- a/tests/units/nulecule/test_lib.py +++ b/tests/units/nulecule/test_lib.py @@ -25,8 +25,10 @@ def test_get_provider_success(self): nb.plugin.getProvider = mock.Mock(return_value=return_provider) ret_provider_key, ret_provider = nb.get_provider() self.assertEqual(provider_key, ret_provider_key) - return_provider.assert_called_with({u'provider': provider_key}, - '', False) + return_provider.assert_called_with( + {'provider': provider_key, 'namespace': 'default'}, + '', + False) def test_get_provider_failure(self): """ diff --git a/tests/units/nulecule/test_nulecule.py b/tests/units/nulecule/test_nulecule.py index 0f3a3372..13e3e648 100644 --- a/tests/units/nulecule/test_nulecule.py +++ b/tests/units/nulecule/test_nulecule.py @@ -101,23 +101,24 @@ def test_load_config_with_default_provider(self): 'key3': 'val3' }, 'component1': { - 'key2': 'val2', - 'key1': 'val1' + 'key2': 'val2' } }) - self.assertEqual(n.components[0].config.context(), { - 'key3': 'val3', - 'key2': 'val2', - 'key1': 'val1', - 'provider': 'docker' - }) + self.assertEqual( + n.components[0].config.context(scope=n.components[0].namespace), + {'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'provider': 'docker', + 'namespace': 'default'} + ) def test_load_config_without_default_provider(self): """ Test Nulecule load_config without specifying a default provider. """ - config = Config(answers={}) + config = Config() params = [ { @@ -150,7 +151,7 @@ def test_load_config_without_default_provider(self): graph=graph, params=params, basepath='some/path', config=config) n.load_components() - n.load_config(config) + n.load_config() self.assertEqual(n.config.runtime_answers(), { 'general': { @@ -160,17 +161,18 @@ def test_load_config_without_default_provider(self): 'key3': 'val3' }, 'component1': { - 'key2': 'val2', - 'key1': 'val1' + 'key2': 'val2' } }) - self.assertEqual(n.components[0].config.context(), { - 'key3': 'val3', - 'key2': 'val2', - 'key1': 'val1', - 'provider': 'kubernetes' - }) + self.assertEqual( + n.components[0].config.context(n.components[0].namespace), + {'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'namespace': 'default', + 'provider': 'kubernetes'} + ) def test_load_config_with_default_provider_overridden_by_answers(self): """ @@ -228,17 +230,18 @@ def test_load_config_with_default_provider_overridden_by_answers(self): 'key3': 'val3' }, 'component1': { - 'key2': 'val2', - 'key1': 'val1' + 'key2': 'val2' } }) - self.assertEqual(n.components[0].config.context(), { - 'key3': 'val3', - 'key2': 'val2', - 'key1': 'val1', - 'provider': 'openshift' - }) + self.assertEqual( + n.components[0].config.context(n.components[0].namespace), + {'key3': 'val3', + 'key2': 'val2', + 'key1': 'val1', + 'namespace': 'default', + 'provider': 'openshift'} + ) class TestNuleculeLoadComponents(unittest.TestCase): diff --git a/tests/units/nulecule/test_nulecule_component.py b/tests/units/nulecule/test_nulecule_component.py index 4bbcb418..9054a2c4 100644 --- a/tests/units/nulecule/test_nulecule_component.py +++ b/tests/units/nulecule/test_nulecule_component.py @@ -136,23 +136,24 @@ def test_load_config_local_app(self): 'general': {'a': 'b', 'key2': 'val2'}, 'some-app': {'key1': 'val1'} } - conf = Config('some-app', answers=initial_config) + conf = Config(answers=initial_config) nc = NuleculeComponent('some-app', 'some/path', params=params, config=conf) nc.load_config() runtime_answers = nc.config.runtime_answers() self.assertEqual(runtime_answers, { - 'general': {'a': 'b', 'key2': 'val2', 'provider': 'kubernetes', 'namespace': 'default'}, - 'some-app': {'key1': 'val1', 'key2': 'val2'} + 'general': { + 'a': 'b', + 'key2': 'val2', + 'provider': 'kubernetes', + 'namespace': 'default' + }, + 'some-app': {'key1': 'val1'} }) def test_load_config_external_app(self): """Test load config for external app""" - mock_nulecule = mock.Mock( - name='nulecule', - spec=Nulecule('some-id', '0.0.2', {}, [], 'some/path') - ) params = [ {'name': 'key1', 'description': 'key1'}, {'name': 'key2', 'description': 'key2'} @@ -161,17 +162,19 @@ def test_load_config_external_app(self): 'general': {'a': 'b', 'key2': 'val2'}, 'some-app': {'key1': 'val1'} } - config = Config('some-app', answers=initial_config) + config = Config(answers=initial_config) + mock_nulecule = mock.Mock( + name='nulecule', + spec=Nulecule('some-id', '0.0.2', config, [], 'some/path') + ) nc = NuleculeComponent('some-app', 'some/path', params=params) nc._app = mock_nulecule - nc.load_config(config=config) + nc.config = config + nc.load_config() mock_nulecule.load_config.assert_called_once_with( - config=Config('some-app', answers=initial_config, data={ - 'general': {}, - 'some-app': {'key1': 'val1', 'key2': 'val2'} - }), ask=False, skip_asking=False) + config=config, ask=False, skip_asking=False) class TestNuleculeComponentLoadExternalApplication(unittest.TestCase): @@ -260,7 +263,7 @@ def test_render_for_local_app_with_missing_artifacts_for_provider(self): dryrun = False nc = NuleculeComponent(name='some-app', basepath='some/path') - nc.config = {} + nc.config = Config() nc.artifacts = {'x': ['some-artifact']} self.assertRaises(NuleculeException, nc.render, provider_key, dryrun) @@ -276,37 +279,44 @@ def test_render_for_local_app_with_missing_artifacts_from_nulecule(self): with self.assertRaises(NuleculeException): nc.render() - @mock.patch('atomicapp.nulecule.base.NuleculeComponent.get_context') @mock.patch('atomicapp.nulecule.base.NuleculeComponent.' 'get_artifact_paths_for_provider') @mock.patch('atomicapp.nulecule.base.NuleculeComponent.render_artifact') def test_render_for_local_app_with_artifacts_for_provider( - self, mock_render_artifact, mock_get_artifact_paths_for_provider, - mock_get_context): + self, mock_render_artifact, mock_get_artifact_paths_for_provider): """Test rendering artifacts for a local Nulecule component""" provider_key = 'some-provider' dryrun = False expected_rendered_artifacts = [ 'some/path/.artifact1', 'some/path/.artifact2'] - context = {'a': 'b'} mock_get_artifact_paths_for_provider.return_value = [ 'some/path/artifact1', 'some/path/artifact2'] mock_render_artifact.side_effect = lambda path, context, provider: path.replace('artifact', '.artifact') - mock_get_context.return_value = context + # mock_get_context.return_value = context nc = NuleculeComponent(name='some-app', basepath='some/path') - nc.config = {'general': {'key1': 'val1'}, 'some-provider': {'a': 'b'}} + nc.config = Config(answers={ + 'general': {'key1': 'val1'}, + 'some-provider': {'a': 'b'} + }) nc.artifacts = { 'some-provider': ['artifact1', 'artifact2'], 'x': ['foo'] } nc.render(provider_key, dryrun) + expected_context = { + 'key1': 'val1', + 'namespace': 'default', + 'provider': 'kubernetes' + } mock_get_artifact_paths_for_provider.assert_called_once_with( provider_key) - mock_render_artifact.assert_any_call('some/path/artifact1', context, + mock_render_artifact.assert_any_call('some/path/artifact1', + expected_context, 'some-provider') - mock_render_artifact.assert_any_call('some/path/artifact2', context, + mock_render_artifact.assert_any_call('some/path/artifact2', + expected_context, 'some-provider') mock_get_artifact_paths_for_provider.assert_called_once_with( provider_key)