From 63a08c6a928d7db9a9f8f154e3da1b3bfda38688 Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Wed, 10 Oct 2018 22:41:05 -0500 Subject: [PATCH] [FIX] mock notes helper and tests for node filter Two testing only changes, one to mock the notes helper during the action_api testing so it doesn't wait for network timeouts silently and take a very long time to run, and secondly enhanced testing around the production of node filters used with deployment groups to validate that rack_names are being handled in the desired way when producing node filters for Drydock. Change-Id: I5439e82333f40e91c270fa52c466731b4bbf1f2f --- .../deployment_group/test_dgm_rack_names.py | 201 ++++++++++++++++++ .../deployment_group/test_node_lookup.py | 46 ++++ .../tests/unit/control/test_actions_api.py | 28 ++- 3 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_dgm_rack_names.py diff --git a/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_dgm_rack_names.py b/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_dgm_rack_names.py new file mode 100644 index 00000000..2881366c --- /dev/null +++ b/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_dgm_rack_names.py @@ -0,0 +1,201 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""An additional test using a "real" use case to ensure that node filters +being produced are as expected before they would be sent to Drydock for +resolution of nodes from the filter. +""" +import logging +import yaml + +from shipyard_airflow.common.deployment_group.deployment_group import ( + Stage +) +from shipyard_airflow.common.deployment_group.deployment_group_manager import ( + DeploymentGroupManager +) +from shipyard_airflow.common.deployment_group.node_lookup import ( + _generate_node_filter, + _validate_selectors +) + + +LOG = logging.getLogger(__name__) + +INPUT_YAML = """ +--- +schema: shipyard/DeploymentStrategy/v1 +metadata: + schema: metadata/Document/v1 + replacement: true + name: deployment-strategy + layeringDefinition: + abstract: false + layer: site + parentSelector: + name: deployment-strategy-global + actions: + - method: replace + path: . + storagePolicy: cleartext + replacement: true +data: + groups: + - name: masters + critical: true + depends_on: [] + selectors: + - node_names: [] + node_labels: [] + node_tags: + - masters + rack_names: [] + success_criteria: + percent_successful_nodes: 100 + - name: worker_group_0 + critical: false + depends_on: + - masters + selectors: + - node_names: [] + node_labels: [] + node_tags: [] + rack_names: + - 'RACK03' + - 'RACK04' + - name: worker_group_1 + critical: false + depends_on: + - masters + selectors: + - node_names: [] + node_labels: [] + node_tags: [] + rack_names: + - 'RACK05' + - 'RACK06' + - name: workers + critical: true + depends_on: + - worker_group_0 + - worker_group_1 + selectors: + - node_names: [] + node_labels: [] + node_tags: + - workers + rack_names: [] + success_criteria: + percent_successful_nodes: 60 +... +""" + +nf_masters = { + 'filter_set_type': + 'union', + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': ['masters'], + 'node_labels': {}, + 'rack_names': [] + }] +} +nf_rack_03_04 = { + 'filter_set_type': + 'union', + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': [], + 'node_labels': {}, + 'rack_names': ['RACK03', 'RACK04'] + }] +} +nf_rack_05_06 = { + 'filter_set_type': + 'union', + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': [], + 'node_labels': {}, + 'rack_names': ['RACK05', 'RACK06'] + }] +} +nf_workers = { + 'filter_set_type': + 'union', + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': ['workers'], + 'node_labels': {}, + 'rack_names': [] + }] +} + + +def node_lookup(selectors): + """Assert things about the input and expected response based on the yaml + above + """ + _validate_selectors(selectors) + nf = _generate_node_filter(selectors) + LOG.info(nf) + nodes = [] + for selector in selectors: + if selector.rack_names: + assert len(selector.rack_names) == 2 + assert len(nf['filter_set'][0]['rack_names']) == 2 + assert len(nf['filter_set'][0]['node_names']) == 0 + assert len(nf['filter_set'][0]['node_tags']) == 0 + assert len(nf['filter_set'][0]['node_labels']) == 0 + if "RACK03" in selector.rack_names: + assert nf == nf_rack_03_04 + if "RACK05" in selector.rack_names: + assert nf == nf_rack_05_06 + for rack in selector.rack_names: + nodes.append(rack + "node1") + if 'masters' in selector.node_tags: + assert len(nf['filter_set'][0]['rack_names']) == 0 + assert len(nf['filter_set'][0]['node_names']) == 0 + assert len(nf['filter_set'][0]['node_tags']) == 1 + assert len(nf['filter_set'][0]['node_labels']) == 0 + assert nf == nf_masters + nodes.append("master1") + nodes.append("master2") + if 'workers' in selector.node_tags: + assert len(nf['filter_set'][0]['rack_names']) == 0 + assert len(nf['filter_set'][0]['node_names']) == 0 + assert len(nf['filter_set'][0]['node_tags']) == 1 + assert len(nf['filter_set'][0]['node_labels']) == 0 + assert nf == nf_workers + nodes.append("RACK06node1") + nodes.append("RACK05node1") + nodes.append("RACK04node1") + nodes.append("RACK03node1") + return nodes + + +class TestDeploymentGroupManagerRackNames: + def test_dgm(self): + all_yaml_dict = yaml.safe_load(INPUT_YAML) + group_dict_list = all_yaml_dict['data']['groups'] + + dgm = DeploymentGroupManager(group_dict_list, node_lookup) + assert dgm is not None + # topological sort doesn't guarantee a specific order. + assert dgm.get_next_group(Stage.PREPARED).name == 'masters' + assert len(dgm._all_groups) == 4 + assert len(dgm._all_nodes) == 6 diff --git a/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_node_lookup.py b/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_node_lookup.py index 698aedae..538bc51f 100644 --- a/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_node_lookup.py +++ b/src/bin/shipyard_airflow/tests/unit/common/deployment_group/test_node_lookup.py @@ -126,6 +126,52 @@ class TestNodeLookup: nf = _generate_node_filter([sel3, sel4]) assert nf is None + def test_generate_node_filter_more_labels(self): + """Tests the _generate_node_filter function""" + sel = GroupNodeSelector({ + 'node_names': [], + 'node_labels': ['label1:label1', + 'label2:enabled', + 'control-plane: bicycle'], + 'node_tags': [], + 'rack_names': [], + }) + nf = _generate_node_filter([sel]) + assert nf == { + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': [], + 'rack_names': [], + 'node_labels': {'label1': 'label1', + 'label2': 'enabled', + 'control-plane': 'bicycle'} + }], + 'filter_set_type': 'union' + } + + def test_generate_node_filter_only_rack(self): + """Tests the _generate_node_filter function""" + sel = GroupNodeSelector({ + 'node_names': [], + 'node_labels': [], + 'node_tags': [], + 'rack_names': ['RACK1', 'RACK2'], + }) + nf = _generate_node_filter([sel]) + assert nf == { + 'filter_set': [{ + 'filter_type': 'intersection', + 'node_names': [], + 'node_tags': [], + 'rack_names': ['RACK1', 'RACK2'], + 'node_labels': {} + }], + 'filter_set_type': 'union' + } + + + @mock.patch('shipyard_airflow.common.deployment_group.node_lookup' '._get_nodes_for_filter', return_value=['node1', 'node2']) def test_NodeLookup_lookup(self, *args): diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py index 77ab421f..04570ec0 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py @@ -257,6 +257,8 @@ def test_on_get(mock_get_all_actions, mock_authorize): assert resp.status == '200 OK' +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch.object(ShipyardPolicy, 'authorize', return_value=True) @mock.patch.object( ActionsResource, @@ -264,7 +266,7 @@ def test_on_get(mock_get_all_actions, mock_authorize): return_value={'id': 'test_id', 'name': 'test_name'}) @patch('logging.Logger.info') -def test_on_post(mock_info, mock_create_action, mock_authorize): +def test_on_post(mock_info, mock_create_action, mock_authorize, *args): act_resource = ActionsResource() context.policy_engine = ShipyardPolicy() json_body = json.dumps({ @@ -289,7 +291,9 @@ def test_on_post(mock_info, mock_create_action, mock_authorize): assert '/api/v1.0/actions/' in resp.location -def test_get_all_actions(): +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) +def test_get_all_actions(*args): """ Tests the main response from get all actions """ @@ -346,13 +350,15 @@ def _gen_action_resource_stubbed(): return action_resource +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_full') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_intermediate_commits') -def test_create_action_invalid_input(ic_val, full_val, basic_val): +def test_create_action_invalid_input(ic_val, full_val, basic_val, *args): action_resource = _gen_action_resource_stubbed() # with invalid input. fail. with pytest.raises(ApiError): @@ -368,6 +374,8 @@ def test_create_action_invalid_input(ic_val, full_val, basic_val): assert not basic_val.called +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -400,6 +408,8 @@ def test_create_action_valid_input_and_params(ic_val, full_val, *args): action=action, configdocs_helper=action_resource.configdocs_helper) +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -429,6 +439,8 @@ def test_create_action_valid_input_no_params(ic_val, full_val, *args): action=action, configdocs_helper=action_resource.configdocs_helper) +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -458,6 +470,8 @@ def test_create_action_validator_error(*args): assert apie.value.title == 'bad' +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -484,6 +498,8 @@ def test_create_targeted_action_valid_input_and_params(basic_val, *args): action=action, configdocs_helper=action_resource.configdocs_helper) +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -508,6 +524,8 @@ def test_create_targeted_action_valid_input_missing_target(basic_val, *args): assert not basic_val.called +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -529,6 +547,8 @@ def test_create_targeted_action_valid_input_missing_param(basic_val, *args): assert not basic_val.called +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.policy.check_auth') @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic') @@ -555,6 +575,8 @@ def test_create_targeted_action_no_committed(basic_val, *args): # Purposefully raising Exception to test only the value passed to auth +@mock.patch('shipyard_airflow.control.action.actions_api.notes_helper', + new=nh) @mock.patch('shipyard_airflow.control.action.action_validators' '.validate_deployment_action_basic', side_effect=Exception('purposeful'))