From 088ccf774410331349cca54785edfeb6d59186ae Mon Sep 17 00:00:00 2001 From: Saravanan KR Date: Mon, 16 Jul 2018 21:25:47 +0530 Subject: [PATCH] Generate additional roles with defined roles file NFV cluster deployment should have the parameters closely associated with hardware and NUMA spec, in which case, the same role has to be duplicated to associate the hardware groups. This patch adds the support to generate the roles_data file with additional roles from the existing defined roles by only changing few properties. openstack overcloud roles generate ComputeOvsDpdk:ComputeOvsDpdkDell \ ComputeOvsDpdk:ComptueOvsDpdkHp -o roles_data.yaml This command allows to use the defined role (ComputeOvsDpdk) to be generated as 2 different roles by changing the 'name' alone. This reduces the copy paste mistakes in generating multiple roles in a cluster with same services. Closes-Bug: #1782322 Change-Id: Ifa60eae1ad09b2ceac207114c40c714a6fc67cbc --- ...ate-roles-with-colon-c903826db084b8a6.yaml | 9 ++ tripleo_common/tests/utils/test_roles.py | 94 +++++++++++++++++++ tripleo_common/utils/roles.py | 58 +++++++++++- 3 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/generate-roles-with-colon-c903826db084b8a6.yaml diff --git a/releasenotes/notes/generate-roles-with-colon-c903826db084b8a6.yaml b/releasenotes/notes/generate-roles-with-colon-c903826db084b8a6.yaml new file mode 100644 index 000000000..36bfde92a --- /dev/null +++ b/releasenotes/notes/generate-roles-with-colon-c903826db084b8a6.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Generating roles_data.yaml file has been enhanced to generate the defined + roles's properties with a differnet name, so that a cluster can have + multiple roles with same set of service, without manual edit. Adds the + support to provide role name input as ``Compute:ComputeA`` so that the + role ``ComputeA`` can be generated from the defined role ``Compute``, by + only chaning the name. diff --git a/tripleo_common/tests/utils/test_roles.py b/tripleo_common/tests/utils/test_roles.py index d01428dbb..b924f40a0 100644 --- a/tripleo_common/tests/utils/test_roles.py +++ b/tripleo_common/tests/utils/test_roles.py @@ -34,6 +34,19 @@ SAMPLE_ROLE = """ ServicesDefault: - OS::TripleO::Services::Ntp """ +SAMPLE_GENERATED_ROLE = """ +############################################################################### +# Role: sample # +############################################################################### +- name: sampleA + description: | + Sample! + networks: + - InternalApi + HostnameFormatDefault: '%stackname%-sample-%index%' + ServicesDefault: + - OS::TripleO::Services::Ntp +""" SAMPLE_ROLE_OBJ = { 'HostnameFormatDefault': '%stackname%-sample-%index%', 'ServicesDefault': ['OS::TripleO::Services::Ntp'], @@ -119,3 +132,84 @@ class TestRolesUtils(base.TestCase): role[0]['CountDefault'] = 'should not be a string' self.assertRaises(RoleMetadataError, rolesutils.validate_role_yaml, yaml.safe_dump(role)) + + @mock.patch('tripleo_common.utils.roles.check_role_exists') + @mock.patch('tripleo_common.utils.roles.get_roles_list_from_directory') + def test_generate_roles_with_one_role_generated(self, get_roles_mock, + check_mock): + get_roles_mock.return_value = ['sample', 'bar', 'baz'] + m = mock.mock_open(read_data=SAMPLE_ROLE) + with mock.patch('tripleo_common.utils.roles.open', m) as open_mock: + r = rolesutils.generate_roles_data_from_directory( + '/roles', ['sample:sampleA']) + open_mock.assert_any_call('/roles/sample.yaml', 'r') + + header = '\n'.join(["#" * 79, + "# File generated by TripleO", + "#" * 79, + ""]) + expected = header + SAMPLE_GENERATED_ROLE + self.assertEqual(expected, r) + get_roles_mock.assert_called_with('/roles') + check_mock.assert_called_with(['sample', 'bar', 'baz'], + ['sample:sampleA']) + + @mock.patch('tripleo_common.utils.roles.check_role_exists') + @mock.patch('tripleo_common.utils.roles.get_roles_list_from_directory') + def test_generate_roles_with_two_same_roles(self, get_roles_mock, + check_mock): + get_roles_mock.return_value = ['sample', 'bar', 'baz'] + m = mock.mock_open(read_data=SAMPLE_ROLE) + with mock.patch('tripleo_common.utils.roles.open', m) as open_mock: + r = rolesutils.generate_roles_data_from_directory( + '/roles', ['sample', 'sample:sampleA']) + open_mock.assert_any_call('/roles/sample.yaml', 'r') + + header = '\n'.join(["#" * 79, + "# File generated by TripleO", + "#" * 79, + ""]) + expected = header + SAMPLE_ROLE + SAMPLE_GENERATED_ROLE + self.assertEqual(expected, r) + get_roles_mock.assert_called_with('/roles') + check_mock.assert_called_with(['sample', 'bar', 'baz'], + ['sample', 'sample:sampleA']) + + @mock.patch('tripleo_common.utils.roles.check_role_exists') + @mock.patch('tripleo_common.utils.roles.get_roles_list_from_directory') + def test_generate_roles_with_wrong_colon_format(self, get_roles_mock, + check_mock): + get_roles_mock.return_value = ['sample', 'bar', 'baz'] + m = mock.mock_open(read_data=SAMPLE_ROLE) + with mock.patch('tripleo_common.utils.roles.open', m) as open_mock: + self.assertRaises(ValueError, + rolesutils.generate_roles_data_from_directory, + '/roles', + ['sample', 'sample:A']) + open_mock.assert_any_call('/roles/sample.yaml', 'r') + + @mock.patch('tripleo_common.utils.roles.check_role_exists') + @mock.patch('tripleo_common.utils.roles.get_roles_list_from_directory') + def test_generate_roles_with_invalid_role_name(self, get_roles_mock, + check_mock): + get_roles_mock.return_value = ['sample', 'bar', 'baz'] + m = mock.mock_open(read_data=SAMPLE_ROLE) + with mock.patch('tripleo_common.utils.roles.open', m) as open_mock: + self.assertRaises(ValueError, + rolesutils.generate_roles_data_from_directory, + '/roles', + ['sample', 'sampleA:sample']) + open_mock.assert_any_call('/roles/sample.yaml', 'r') + + @mock.patch('tripleo_common.utils.roles.check_role_exists') + @mock.patch('tripleo_common.utils.roles.get_roles_list_from_directory') + def test_generate_roles_with_invalid_colon_format(self, get_roles_mock, + check_mock): + get_roles_mock.return_value = ['sample', 'bar', 'baz'] + m = mock.mock_open(read_data=SAMPLE_ROLE) + with mock.patch('tripleo_common.utils.roles.open', m) as open_mock: + self.assertRaises(ValueError, + rolesutils.generate_roles_data_from_directory, + '/roles', + ['sample', 'sample:sample']) + open_mock.assert_any_call('/roles/sample.yaml', 'r') diff --git a/tripleo_common/utils/roles.py b/tripleo_common/utils/roles.py index f7add3ee6..2cd2b08ac 100644 --- a/tripleo_common/utils/roles.py +++ b/tripleo_common/utils/roles.py @@ -44,7 +44,8 @@ def check_role_exists(available_roles, requested_roles): :param requested_roles list of requested role names :exception NotFound if a role in the requested list is not available """ - role_check = set(requested_roles) - set(available_roles) + unique_roles = list(set([r.split(':')[0] for r in requested_roles])) + role_check = set(unique_roles) - set(available_roles) if len(role_check) > 0: msg = "Invalid roles requested: {}\nValid Roles:\n{}".format( ','.join(role_check), '\n'.join(available_roles) @@ -52,6 +53,49 @@ def check_role_exists(available_roles, requested_roles): raise NotFound(msg) +def generate_role_with_colon_format(content, defined_role, generated_role): + """Generate role data with input as Compute:ComputeA + + In Compute:ComputeA, the defined role 'Compute' can be added to + roles_data.yaml by changing the name to 'ComputeA'. This allows duplicating + the defined roles so that hardware specific nodes can be targeted with + specific roles. + + :param content defined role file's content + :param defined_role defined role's name + :param generated_role role's name to generate from defined role + :exception ValueError if generated role name is of invalid format + """ + + # "Compute:Compute" is invalid format + if generated_role == defined_role: + msg = ("Generated role name cannot be same as existing role name (%s) " + "with colon format".format(defined_role)) + raise ValueError(msg) + + # "Compute:A" is invalid format + if not generated_role.startswith(defined_role): + msg = ("Generated role name (%s) name should start with existing role " + "name (%s)".format(generated_role, defined_role)) + raise ValueError(msg) + + name_line = "name:%s" % defined_role + name_line_match = False + processed = [] + for line in content.split('\n'): + stripped_line = line.replace(' ', '') + # Only 'name' need to be replaced in the existing role + if name_line in stripped_line: + line = line.replace(defined_role, generated_role) + name_line_match = True + processed.append(line) + + if not name_line_match: + raise ValueError(" error") + + return '\n'.join(processed) + + def generate_roles_data_from_directory(directory, roles, validate=True): """Generate a roles data file using roles from a local path @@ -71,11 +115,19 @@ def generate_roles_data_from_directory(directory, roles, validate=True): output.write("\n".join(header)) for role in roles: - file_path = os.path.join(directory, "{}.yaml".format(role)) + defined_role = role.split(':')[0] + file_path = os.path.join(directory, "{}.yaml".format(defined_role)) if validate: validate_role_yaml(role_path=file_path) with open(file_path, "r") as f: - shutil.copyfileobj(f, output) + if ':' in role: + generated_role = role.split(':')[1] + content = generate_role_with_colon_format(f.read(), + defined_role, + generated_role) + output.write(content) + else: + shutil.copyfileobj(f, output) return output.getvalue()