diff --git a/fuelclient/client.py b/fuelclient/client.py index f39fa94..ff8138c 100644 --- a/fuelclient/client.py +++ b/fuelclient/client.py @@ -160,10 +160,7 @@ class APIClient(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. @@ -178,8 +175,7 @@ class APIClient(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 @@ -224,8 +220,7 @@ class APIClient(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") @@ -236,6 +231,14 @@ class APIClient(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 'APIClient' class, # which intended to implement Singleton design pattern. DefaultAPIClient = APIClient.default_client() diff --git a/fuelclient/commands/graph.py b/fuelclient/commands/graph.py index 25e199f..27ee8be 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,6 +58,34 @@ 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(required=True) @@ -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 d1e2ef8..981aabc 100644 --- a/fuelclient/tests/unit/v2/cli/test_deployment_graph.py +++ b/fuelclient/tests/unit/v2/cli/test_deployment_graph.py @@ -20,7 +20,6 @@ import yaml from fuelclient.tests.unit.v2.cli import test_engine - TASKS_YAML = '''- id: custom-task-1 type: puppet parameters: @@ -31,9 +30,27 @@ 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): - @mock.patch('fuelclient.commands.graph.os') def _test_cmd(self, method, cmd_line, expected_kwargs, os_m): os_m.exists.return_value = True @@ -48,19 +65,19 @@ class TestGraphActions(test_engine.BaseCLITest): **expected_kwargs) def test_upload(self): - self._test_cmd('upload', '--env 1 --file new_graph.yaml', dict( + self._test_cmd('upload', '--env 1 --file new_tasks.yaml', dict( data=yaml.load(TASKS_YAML), related_model='clusters', related_id=1, graph_type=None )) - self._test_cmd('upload', '--release 1 --file new_graph.yaml', dict( + self._test_cmd('upload', '--release 1 --file new_tasks.yaml', dict( data=yaml.load(TASKS_YAML), related_model='releases', related_id=1, graph_type=None )) - self._test_cmd('upload', '--plugin 1 --file new_graph.yaml', dict( + self._test_cmd('upload', '--plugin 1 --file new_tasks.yaml', dict( data=yaml.load(TASKS_YAML), related_model='plugins', related_id=1, @@ -68,7 +85,7 @@ class TestGraphActions(test_engine.BaseCLITest): )) self._test_cmd( 'upload', - '--plugin 1 --file new_graph.yaml --type custom_type', + '--plugin 1 --file tasks.yaml --type custom_type', dict( data=yaml.load(TASKS_YAML), related_model='plugins', @@ -77,6 +94,50 @@ 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' diff --git a/fuelclient/v1/graph.py b/fuelclient/v1/graph.py index 20f9fb3..c29b69b 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 = []