From ffc7d359a535b473b6ee1b04de552ab78ca9d93c Mon Sep 17 00:00:00 2001 From: Bulat Gaifullin Date: Thu, 15 Sep 2016 17:56:41 +0300 Subject: [PATCH] Add option to get merged facts for environment Since 9.1 the nailgun returns separated facts for cluster and nodes, so some client expects that node facts also contains cluster fact. this patch adds possibility to get merged facts for each node, by default this flag is true Change-Id: I29f0cc0090abca2826e273dcbe7ac4249cf7a731 Closes-Bug: 1623854 --- fuelclient/cli/actions/fact.py | 9 +++- fuelclient/cli/arguments.py | 10 ++++ fuelclient/commands/environment.py | 19 ++++--- fuelclient/objects/environment.py | 49 +++++++++---------- fuelclient/tests/unit/v1/test_environment.py | 22 +++++++++ fuelclient/tests/unit/v2/cli/test_env.py | 22 ++++++--- .../tests/unit/v2/lib/test_environment.py | 4 +- fuelclient/v1/environment.py | 24 +++------ 8 files changed, 97 insertions(+), 62 deletions(-) diff --git a/fuelclient/cli/actions/fact.py b/fuelclient/cli/actions/fact.py index 022ece76..22ea41ea 100644 --- a/fuelclient/cli/actions/fact.py +++ b/fuelclient/cli/actions/fact.py @@ -47,6 +47,7 @@ class FactAction(Action): Args.get_node_arg( "Node ids." ), + Args.get_not_split_facts_args(), ] self.flag_func_map = ( ("default", self.default), @@ -66,7 +67,9 @@ class FactAction(Action): env = Environment(params.env) dir_name = env.write_facts_to_dir( self.action_name, - env.get_default_facts(self.action_name, nodes=params.node), + env.get_default_facts( + self.action_name, nodes=params.node, split=params.split + ), directory=params.dir, serializer=self.serializer ) @@ -107,7 +110,9 @@ class FactAction(Action): env = Environment(params.env) dir_name = env.write_facts_to_dir( self.action_name, - env.get_facts(self.action_name, nodes=params.node), + env.get_facts( + self.action_name, nodes=params.node, split=params.split + ), directory=params.dir, serializer=self.serializer ) diff --git a/fuelclient/cli/arguments.py b/fuelclient/cli/arguments.py index 025fa9c1..c3e96f8c 100644 --- a/fuelclient/cli/arguments.py +++ b/fuelclient/cli/arguments.py @@ -788,3 +788,13 @@ def get_include_summary_arg(help_msg): "help": help_msg } return get_boolean_arg("include-summary", **default_kwargs) + + +def get_not_split_facts_args(): + kwargs = { + "action": "store_false", + "default": True, + "dest": "split", + "help": "Do not split deployment info for node and cluster parts." + } + return get_arg('no-split', **kwargs) diff --git a/fuelclient/commands/environment.py b/fuelclient/commands/environment.py index cb30a42a..282ed6b8 100644 --- a/fuelclient/commands/environment.py +++ b/fuelclient/commands/environment.py @@ -673,13 +673,9 @@ class FactsMixIn(object): _fact) def download(self, env_id, fact_type, destination_dir, file_format, - nodes=None, default=False): + nodes=None, default=False, split=None): facts = self.client.download_facts( - env_id, fact_type, nodes=nodes, default=default) - if not facts: - raise error.ServerDataException( - "There are no {} facts for this environment!".format( - fact_type)) + env_id, fact_type, nodes=nodes, default=default, split=split) facts_dir = self._get_fact_dir(env_id, fact_type, destination_dir) if os.path.exists(facts_dir): @@ -777,6 +773,14 @@ class BaseEnvFactsDownload(FactsMixIn, EnvMixIn, base.BaseCommand): required=True, help='Format of serialized {} facts'.format(self.fact_type)) + parser.add_argument( + '--no-split', + action='store_false', + dest='split', + default=True, + help='Do not split deployment info for node and cluster parts.' + ) + return parser def take_action(self, parsed_args): @@ -786,7 +790,8 @@ class BaseEnvFactsDownload(FactsMixIn, EnvMixIn, base.BaseCommand): parsed_args.directory, parsed_args.format, nodes=parsed_args.nodes, - default=self.fact_default + default=self.fact_default, + split=parsed_args.split ) self.app.stdout.write( "{0} {1} facts for the environment {2} " diff --git a/fuelclient/objects/environment.py b/fuelclient/objects/environment.py index 2825da2d..53ae69c4 100644 --- a/fuelclient/objects/environment.py +++ b/fuelclient/objects/environment.py @@ -296,37 +296,34 @@ class Environment(BaseObject): os.path.abspath(directory), "{0}_{1}".format(fact_type, self.id)) - def _get_fact_default_url(self, fact_type, nodes=None): - default_url = "clusters/{0}/orchestrator/{1}/defaults".format( - self.id, - fact_type + def _get_fact_url(self, fact_type, default=False): + fact_url = "clusters/{0}/orchestrator/{1}/{2}".format( + self.id, fact_type, 'defaults/' if default else '' ) - if nodes is not None: - default_url += "/?nodes=" + ",".join(map(str, nodes)) - return default_url - - def _get_fact_url(self, fact_type, nodes=None): - fact_url = "clusters/{0}/orchestrator/{1}/".format( - self.id, - fact_type - ) - if nodes is not None: - fact_url += "/?nodes=" + ",".join(map(str, nodes)) return fact_url - def get_default_facts(self, fact_type, nodes=None): - facts = self.connection.get_request( - self._get_fact_default_url(fact_type, nodes=nodes)) - if not facts: - raise error.ServerDataException( - "There is no {0} info for this " - "environment!".format(fact_type) - ) - return facts + def get_default_facts(self, fact_type, **kwargs): + """Gets default facts for cluster. + :param fact_type: the type of facts (deployment, provision) + """ + return self.get_facts(fact_type, default=True, **kwargs) + + def get_facts(self, fact_type, default=False, nodes=None, split=None): + """Gets facts for cluster. + :param fact_type: the type of facts (deployment, provision) + :param default: if True, the default facts will be retrieved + :param nodes: if specified, get facts only for selected nodes + :param split: if True, the node part and common part will be split + """ + params = {} + if nodes is not None: + params['nodes'] = ','.join(str(x) for x in nodes) + if split is not None: + params['split'] = str(int(split)) - def get_facts(self, fact_type, nodes=None): facts = self.connection.get_request( - self._get_fact_url(fact_type, nodes=nodes)) + self._get_fact_url(fact_type, default=default), params=params + ) if not facts: raise error.ServerDataException( "There is no {0} info for this " diff --git a/fuelclient/tests/unit/v1/test_environment.py b/fuelclient/tests/unit/v1/test_environment.py index b87906a3..26803a3d 100644 --- a/fuelclient/tests/unit/v1/test_environment.py +++ b/fuelclient/tests/unit/v1/test_environment.py @@ -170,3 +170,25 @@ class TestEnvironmentOstf(base.UnitTestCase): self.env.get_deployment_tasks(end=end) self.assertEqual(get.last_request.qs, {'end': ['task1']}) + + def test_get_default_facts(self): + legacy_format_facts = [ + {"uid": "1", "a": 1}, {"uid": "2", "a": 1} + ] + new_format_facts = [ + {"uid": "common", "a": 1}, {"uid": "1"}, {"uid": "2"} + ] + self.m_request.get( + '/api/v1/clusters/{0}/orchestrator/deployment/defaults/?split=0' + .format(self.env.id), + json=legacy_format_facts + ) + self.m_request.get( + '/api/v1/clusters/{0}/orchestrator/deployment/defaults/?split=1' + .format(self.env.id), + json=new_format_facts + ) + facts = self.env.get_default_facts("deployment", split=False) + self.assertItemsEqual(legacy_format_facts, facts) + facts = self.env.get_default_facts("deployment", split=True) + self.assertItemsEqual(new_format_facts, facts) diff --git a/fuelclient/tests/unit/v2/cli/test_env.py b/fuelclient/tests/unit/v2/cli/test_env.py index 57f2eaa9..b21285a0 100644 --- a/fuelclient/tests/unit/v2/cli/test_env.py +++ b/fuelclient/tests/unit/v2/cli/test_env.py @@ -476,8 +476,11 @@ class TestEnvCommand(test_engine.BaseCLITest): @mock.patch('json.dump') def _deployment_facts_download_json(self, m_dump, default=False): command = 'get-default' if default else 'download' - args = "env deployment-facts {}" \ - " --env 42 --dir /tmp --nodes 2 --format json".format(command) + args = ( + "env deployment-facts {}" + " --env 42 --dir /tmp --nodes 2 --format json --no-split" + .format(command) + ) data = [{'uid': 2, 'name': 'node'}] expected_path = '/tmp/deployment_42/2.json' @@ -490,7 +493,7 @@ class TestEnvCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('environment', mock.ANY) self.m_client.download_facts.assert_called_once_with( - 42, 'deployment', nodes=[2], default=default) + 42, 'deployment', nodes=[2], default=default, split=False) m_open.assert_called_once_with(expected_path, 'w') m_dump.assert_called_once_with(data[0], mock.ANY, indent=4) @@ -503,8 +506,11 @@ class TestEnvCommand(test_engine.BaseCLITest): @mock.patch('yaml.safe_dump') def _deployment_facts_download_yaml(self, m_safe_dump, default=False): command = 'get-default' if default else 'download' - args = "env deployment-facts {}" \ - " --env 42 --dir /tmp --nodes 2 --format yaml".format(command) + args = ( + "env deployment-facts {}" + " --env 42 --dir /tmp --nodes 2 --format yaml" + .format(command) + ) data = [{'uid': 2, 'name': 'node'}] expected_path = '/tmp/deployment_42/2.yaml' @@ -517,7 +523,7 @@ class TestEnvCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('environment', mock.ANY) self.m_client.download_facts.assert_called_once_with( - 42, 'deployment', nodes=[2], default=default) + 42, 'deployment', nodes=[2], default=default, split=True) m_open.assert_called_once_with(expected_path, 'w') m_safe_dump.assert_called_once_with(data[0], mock.ANY, default_flow_style=False) @@ -581,7 +587,7 @@ class TestEnvCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('environment', mock.ANY) self.m_client.download_facts.assert_called_once_with( - 42, 'provisioning', nodes=[2], default=default) + 42, 'provisioning', nodes=[2], default=default, split=True) m_open.assert_any_call(expected_path_engine, 'w') m_dump.assert_any_call(data['engine'], mock.ANY, indent=4) m_open.assert_any_call(expected_path_node, 'w') @@ -614,7 +620,7 @@ class TestEnvCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('environment', mock.ANY) self.m_client.download_facts.assert_called_once_with( - 42, 'provisioning', nodes=[2], default=default) + 42, 'provisioning', nodes=[2], default=default, split=True) m_open.assert_any_call(expected_path_engine, 'w') m_dump.assert_any_call(data['engine'], mock.ANY, default_flow_style=False) diff --git a/fuelclient/tests/unit/v2/lib/test_environment.py b/fuelclient/tests/unit/v2/lib/test_environment.py index d3869649..d5c1e9d3 100644 --- a/fuelclient/tests/unit/v2/lib/test_environment.py +++ b/fuelclient/tests/unit/v2/lib/test_environment.py @@ -452,7 +452,7 @@ class TestEnvFacade(test_api.BaseLibTest): expected_uri = self.get_object_uri( self.res_uri, env_id, - '/orchestrator/{fact_type}'.format(fact_type=fact_type)) + '/orchestrator/{fact_type}/'.format(fact_type=fact_type)) matcher = self.m_request.delete(expected_uri, json={}) self.client.delete_facts(env_id, fact_type) @@ -484,7 +484,7 @@ class TestEnvFacade(test_api.BaseLibTest): expected_uri = self.get_object_uri( self.res_uri, env_id, - "/orchestrator/{fact_type}".format(fact_type=fact_type)) + "/orchestrator/{fact_type}/".format(fact_type=fact_type)) matcher = self.m_request.put(expected_uri, json={}) self.client.upload_facts(env_id, fact_type, facts) diff --git a/fuelclient/v1/environment.py b/fuelclient/v1/environment.py index 6a785b32..ab201097 100644 --- a/fuelclient/v1/environment.py +++ b/fuelclient/v1/environment.py @@ -163,27 +163,17 @@ class EnvironmentClient(base_v1.BaseV1Client): env = self._entity_wrapper(environment_id) env.set_settings_data(new_configuration, force=force) - @staticmethod - def _get_fact_url(env_id, fact_type, nodes=None, default=False): - return "clusters/{id}/orchestrator/{fact_type}{default}{nodes}".format( - id=env_id, - fact_type=fact_type, - default="/defaults" if default else '', - nodes="/?nodes={}".format( - ",".join(map(str, nodes))) if nodes else '' - ) - def delete_facts(self, env_id, fact_type): - return self.connection.delete_request( - self._get_fact_url(env_id, fact_type)) + env = self._entity_wrapper(env_id) + return env.delete_facts(fact_type) - def download_facts(self, env_id, fact_type, nodes=None, default=False): - return self.connection.get_request(self._get_fact_url( - env_id, fact_type, nodes=nodes, default=default)) + def download_facts(self, env_id, fact_type, default=False, **kwargs): + env = self._entity_wrapper(env_id) + return env.get_facts(fact_type, default=default, **kwargs) def upload_facts(self, env_id, fact_type, facts): - return self.connection.put_request( - self._get_fact_url(env_id, fact_type), facts) + env = self._entity_wrapper(env_id) + return env.upload_facts(fact_type, facts) def get_client(connection):