From 8f955301de8cab6b5eb6a24a292c88091c3bd61f Mon Sep 17 00:00:00 2001 From: Ilya Kutukov Date: Tue, 23 Aug 2016 01:56:23 +0300 Subject: [PATCH] Now it is possilble to upload deployment graph via CLI Also fixed fail in case if post request returns 204 also supported upload from directory. the target dir should contain files: metadata.yaml and tasks.yaml $ fuel2 graph upload --env 1 --file new_graph.yaml $ fuel2 graph upload --release 1 --dir /var/data/graphs/provision -t provision Now accepting two structure formats wrapped into the YAML: 1. {'tasks': [], 'metadata': {}, ....} 2. [{id: 'task1', ...}, {id: 'task2', ...}] Change-Id: I5b5fc8c2733d0bc6bc31fb3a9e3d632b6e429ce7 Closes-Bug: #1615849 --- fuelclient/client.py | 19 ++--- fuelclient/commands/graph.py | 66 +++++++++++++---- .../unit/v2/cli/test_deployment_graph.py | 70 +++++++++++++++++++ fuelclient/v1/graph.py | 17 +++-- 4 files changed, 144 insertions(+), 28 deletions(-) diff --git a/fuelclient/client.py b/fuelclient/client.py index ad08dff..006bd62 100644 --- a/fuelclient/client.py +++ b/fuelclient/client.py @@ -168,10 +168,7 @@ class Client(object): resp = self.session.delete(url) self._raise_for_status_with_info(resp) - if resp.status_code == 204: - return {} - - return resp.json() + return self._decode_content(resp) def put_request(self, api, data, **params): """Make PUT request to specific API with some data. @@ -186,8 +183,7 @@ class Client(object): self.print_debug('PUT {0} data={1}'.format(resp.url, data_json)) self._raise_for_status_with_info(resp) - - return resp.json() + return self._decode_content(resp) def get_request_raw(self, api, ostf=False, params=None): """Make a GET request to specific API and return raw response @@ -232,8 +228,7 @@ class Client(object): """ resp = self.post_request_raw(api, data, ostf=ostf) self._raise_for_status_with_info(resp) - - return resp.json() + return self._decode_content(resp) def get_fuel_version(self): return self.get_request("version") @@ -244,6 +239,14 @@ class Client(object): except requests.exceptions.HTTPError as e: raise error.HTTPError(error.get_full_error_message(e)) + def _decode_content(self, response): + if response.status_code == 204: + return {} + + self.print_debug(response.text) + return response.json() + + # This line is single point of instantiation for 'Client' class, # which intended to implement Singleton design pattern. APIClient = Client.default_client() diff --git a/fuelclient/commands/graph.py b/fuelclient/commands/graph.py index 3caa4eb..0d6de66 100644 --- a/fuelclient/commands/graph.py +++ b/fuelclient/commands/graph.py @@ -20,6 +20,7 @@ from fuelclient.cli import error from fuelclient.cli.serializers import Serializer from fuelclient.commands import base from fuelclient.common import data_utils +from fuelclient.utils import iterfiles class FileMethodsMixin(object): @@ -44,8 +45,8 @@ class GraphUpload(base.BaseCommand, FileMethodsMixin): entity_name = 'graph' @classmethod - def read_tasks_data_from_file(cls, file_path=None, serializer=None): - """Read Tasks data from given path. + def read_data_from_file(cls, file_path=None, serializer=None): + """Read graph data from given path. :param file_path: path :type file_path: str @@ -57,9 +58,37 @@ class GraphUpload(base.BaseCommand, FileMethodsMixin): cls.check_file_path(file_path) return (serializer or Serializer()).read_from_full_path(file_path) + @classmethod + def read_data_from_dir(cls, dir_path=None, serializer=None): + """Read graph data from directory. + + :param dir_path: path + :type dir_path: str + :param serializer: serializer object + :type serializer: object + :return: data + :rtype: list|object + """ + cls.check_dir(dir_path) + serializer = serializer or Serializer() + + metadata_filepath = os.path.join(dir_path, 'metadata.yaml') + if os.path.exists(metadata_filepath): + data = serializer.read_from_full_path(metadata_filepath) + else: + data = {} + + tasks = [] + for file_name in iterfiles(dir_path, 'task.yaml'): + tasks.extend(serializer.read_from_full_path(file_name)) + + if tasks: + data['tasks'] = tasks + return data + def get_parser(self, prog_name): parser = super(GraphUpload, self).get_parser(prog_name) - graph_class = parser.add_mutually_exclusive_group() + graph_class = parser.add_mutually_exclusive_group(required=True) graph_class.add_argument('-e', '--env', @@ -83,13 +112,19 @@ class GraphUpload(base.BaseCommand, FileMethodsMixin): default=None, required=False, help='Type of the deployment graph') - parser.add_argument('-f', - '--file', - type=str, - required=True, - default=None, - help='YAML file that contains ' - 'deployment graph data.') + graph_source = parser.add_mutually_exclusive_group(required=True) + graph_source.add_argument( + '-f', + '--file', + default=None, + help='YAML file that contains deployment graph data.' + ) + graph_source.add_argument( + '-d', + '--dir', + default=None, + help='The directory that includes tasks.yaml and metadata.yaml.' + ) return parser def take_action(self, args): @@ -99,20 +134,23 @@ class GraphUpload(base.BaseCommand, FileMethodsMixin): ('plugin', 'plugins'), ) + if args.file: + data = self.read_data_from_file(args.file) + else: + data = self.read_data_from_dir(args.dir) + for parameter, graph_class in parameters_to_graph_class: model_id = getattr(args, parameter) if model_id: self.client.upload( - data=self.read_tasks_data_from_file(args.file), + data=data, related_model=graph_class, related_id=model_id, graph_type=args.type ) break - self.app.stdout.write( - "Deployment graph was uploaded from {0}\n".format(args.file) - ) + self.app.stdout.write("Deployment graph was successfully uploaded.\n") class GraphExecute(base.BaseCommand): diff --git a/fuelclient/tests/unit/v2/cli/test_deployment_graph.py b/fuelclient/tests/unit/v2/cli/test_deployment_graph.py index 2ded3a8..f8c1eda 100644 --- a/fuelclient/tests/unit/v2/cli/test_deployment_graph.py +++ b/fuelclient/tests/unit/v2/cli/test_deployment_graph.py @@ -31,6 +31,25 @@ TASKS_YAML = '''- id: custom-task-1 param: value ''' +GRAPH_YAML = '''tasks: + - id: custom-task-1 + type: puppet + parameters: + param: value + - id: custom-task-2 + type: puppet + parameters: + param: value +node_filter: $.pending_deletion +''' + +GRAPH_METADATA_YAML = ''' +node_filter: $.pending_addition +on_success: + node_attributes: + status: provisioned +''' + class TestGraphActions(test_engine.BaseCLITest): @@ -77,6 +96,57 @@ class TestGraphActions(test_engine.BaseCLITest): ) ) + @mock.patch('fuelclient.commands.graph.os') + def test_graph_upload_from_file(self, os_m): + os_m.path.exists.return_value = True + self.m_get_client.reset_mock() + self.m_client.get_filtered.reset_mock() + m_open = mock.mock_open(read_data=GRAPH_YAML) + with mock.patch( + 'fuelclient.cli.serializers.open', m_open, create=True): + self.exec_command('graph upload --env 1 --file new_graph.yaml') + self.m_get_client.assert_called_once_with('graph', mock.ANY) + self.m_client.upload.assert_called_once_with( + data=yaml.load(GRAPH_YAML), + related_model='clusters', + related_id=1, + graph_type=None + ) + + @mock.patch('fuelclient.commands.graph.os') + @mock.patch('fuelclient.commands.graph.iterfiles') + @mock.patch('fuelclient.commands.graph.Serializer') + def test_graph_upload_from_dir(self, serializers_m, iterfiles_m, os_m): + tasks = yaml.load(TASKS_YAML) + graph_data = yaml.load(GRAPH_METADATA_YAML) + os_m.path.exists.return_value = True + os_m.path.isdir.return_value = True + serializers_m().read_from_full_path.side_effect = [graph_data, tasks] + iterfiles_m.return_value = ['tasks.yaml'] + + self.m_get_client.reset_mock() + self.m_client.get_filtered.reset_mock() + m_open = mock.mock_open(read_data=GRAPH_YAML) + with mock.patch( + 'fuelclient.cli.serializers.open', m_open, create=True): + self.exec_command( + 'graph upload --release 1 --dir /graph/provision -t provision' + ) + self.m_get_client.assert_called_once_with('graph', mock.ANY) + self.m_client.upload.assert_called_once_with( + data=dict(graph_data, tasks=tasks), + related_model='releases', + related_id=1, + graph_type='provision' + ) + + @mock.patch('sys.stderr') + def test_upload_fail(self, mocked_stderr): + cmd = 'graph upload --file new_graph.yaml' + self.assertRaises(SystemExit, self.exec_command, cmd) + self.assertIn('-e/--env -r/--release -p/--plugin', + mocked_stderr.write.call_args_list[-1][0][0]) + def test_execute(self): self._test_cmd( 'execute', diff --git a/fuelclient/v1/graph.py b/fuelclient/v1/graph.py index 474746d..6550f04 100644 --- a/fuelclient/v1/graph.py +++ b/fuelclient/v1/graph.py @@ -69,15 +69,20 @@ class GraphClient(base_v1.BaseV1Client): def upload(self, data, related_model, related_id, graph_type): # create or update + if not isinstance(data, dict): + data = {'tasks': data} + + method = self.update_graph_for_model + # FIXME(bgaifullin) need to remove loading whole graph only + # for detecting that it does not exist try: - self.get_graph_for_model( - related_model, related_id, graph_type) - self.update_graph_for_model( - {'tasks': data}, related_model, related_id, graph_type) + self.get_graph_for_model(related_model, related_id, graph_type) except error.HTTPError as exc: if '404' in exc.message: - self.create_graph_for_model( - {'tasks': data}, related_model, related_id, graph_type) + method = self.create_graph_for_model + + # could accept {tasks: [], metadata: {}} or just tasks list + method(data, related_model, related_id, graph_type) def execute(self, env_id, nodes, graph_type=None, dry_run=False): put_args = []