From 330b10a052f8bde70f98ebaa9cab04c6e7fdacf3 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 022ece7..22ea41e 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 4ee4663..e76e109 100644 --- a/fuelclient/cli/arguments.py +++ b/fuelclient/cli/arguments.py @@ -775,3 +775,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 e7a5533..aa80625 100644 --- a/fuelclient/commands/environment.py +++ b/fuelclient/commands/environment.py @@ -720,13 +720,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): @@ -824,6 +820,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): @@ -833,7 +837,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 48d2c10..ed41ac6 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 b87906a..26803a3 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 0990f9f..3d1827b 100644 --- a/fuelclient/tests/unit/v2/cli/test_env.py +++ b/fuelclient/tests/unit/v2/cli/test_env.py @@ -503,8 +503,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' @@ -517,7 +520,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) @@ -530,8 +533,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' @@ -544,7 +550,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) @@ -608,7 +614,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') @@ -641,7 +647,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 b61e760..f995d4d 100644 --- a/fuelclient/tests/unit/v2/lib/test_environment.py +++ b/fuelclient/tests/unit/v2/lib/test_environment.py @@ -487,7 +487,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) @@ -519,7 +519,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 5ff78fa..7a08a32 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 reset(self, env_id, force=False): env = self._entity_wrapper(obj_id=env_id)