From a59df8e49df77e912e6799e0adae461d8ef85ae8 Mon Sep 17 00:00:00 2001 From: Michele Pagot Date: Thu, 29 Aug 2024 17:12:01 +0200 Subject: [PATCH] Drop conf.yaml apiver support (#262) Drop support for apiver <3 --- scripts/qesap/lib/cmds.py | 44 +----- scripts/qesap/lib/config.py | 36 +++-- scripts/qesap/test/conftest.py | 7 +- scripts/qesap/test/e2e/test_2.yaml | 2 +- scripts/qesap/test/unit/test_qesap_ansible.py | 35 +++-- .../qesap/test/unit/test_qesap_configure.py | 48 +++++-- .../test/unit/test_qesap_configure_ansible.py | 125 ------------------ 7 files changed, 94 insertions(+), 203 deletions(-) diff --git a/scripts/qesap/lib/cmds.py b/scripts/qesap/lib/cmds.py index cffb72dd..7b913e30 100644 --- a/scripts/qesap/lib/cmds.py +++ b/scripts/qesap/lib/cmds.py @@ -52,40 +52,13 @@ def create_hana_media(config_ansible, apiver): """ hanamedia_content = {} if apiver < 3: - hana_url_re = r'http.*://(?P.+)\.blob\.core\.windows\.net/(?P.+)/(?P.+)' - match = [] - prev_account = None - prev_container = None - for this_media in config_ansible['hana_urls']: - this_match = re.search(hana_url_re, this_media) - if not this_match: - log.error("[%s] does not match regexp to extract ACCOUNT, CONTAINER or EXE", this_media) - return None, f"Problems in apiver: {apiver} data conversion" - this_account = this_match.group('ACCOUNT') - this_container = this_match.group('CONTAINER') - if prev_account is None: - prev_account = this_account - elif prev_account != this_account: - log.error("ACCOUNT [%s] does not match ACCOUNT [%s] used in previous url", this_account, prev_account) - return None, f"Problems in apiver: {apiver} data conversion" - if prev_container is None: - prev_container = this_match.group('CONTAINER') - elif prev_container != this_container: - log.error("CONTAINER [%s] does not match CONTAINER [%s] used in previous url", this_container, prev_container) - return None, f"Problems in apiver: {apiver} data conversion" - match.append(this_match) - - hanamedia_content['az_storage_account_name'] = match[0].group('ACCOUNT') - hanamedia_content['az_container_name'] = match[0].group('CONTAINER') - hanamedia_content['az_blobs'] = [] - for this_match in match: - hanamedia_content['az_blobs'].append(this_match.group('EXE')) - else: - hanamedia_content['az_storage_account_name'] = config_ansible['az_storage_account_name'] - hanamedia_content['az_container_name'] = config_ansible['az_container_name'] - if 'az_sas_token' in config_ansible: - hanamedia_content['az_sas_token'] = config_ansible['az_sas_token'] - hanamedia_content['az_blobs'] = config_ansible['hana_media'] + log.error("Apiver:%d is no longer supported", apiver) + return None, f"Problems in apiver: {apiver} data conversion" + hanamedia_content['az_storage_account_name'] = config_ansible['az_storage_account_name'] + hanamedia_content['az_container_name'] = config_ansible['az_container_name'] + if 'az_sas_token' in config_ansible: + hanamedia_content['az_sas_token'] = config_ansible['az_sas_token'] + hanamedia_content['az_blobs'] = config_ansible['hana_media'] return hanamedia_content, None @@ -126,9 +99,6 @@ def cmd_configure(configure_data, base_project, dryrun): return Status(err) log.debug("Hana media %s:\n%s", cfg_paths['hana_media_file'], hanamedia_content) - if 'hana_vars' in configure_data['ansible'] and configure_data['apiver'] >= 2: - log.debug("Hana variables %s:\n%s", cfg_paths['hana_vars_file'], configure_data['ansible']['hana_vars']) - if dryrun: print(f"Create {cfg_paths['tfvars_file']} with content {tfvar_content}") print(f"Create {cfg_paths['hana_media_file']} with content {hanamedia_content}") diff --git a/scripts/qesap/lib/config.py b/scripts/qesap/lib/config.py index bcc955bd..013ccb64 100644 --- a/scripts/qesap/lib/config.py +++ b/scripts/qesap/lib/config.py @@ -204,26 +204,24 @@ def validate_ansible_media_config(ansible_conf, apiver): Validate the media part of the ansible configure.yaml """ if apiver < 3: - if 'hana_urls' not in ansible_conf: - log.error("Missing 'hana_urls' in 'ansible' in the config") - return False - else: - if 'hana_media' not in ansible_conf: - log.error("Missing 'hana_media' in 'ansible' in the config") - return False - for media in ansible_conf['hana_media']: - match = re.search(r'^http[s]?://.*', media) - if match: - log.error("Media %s provided as full url. File name expected.", media) - return False - if 'az_storage_account_name' not in ansible_conf: - log.error("Missing 'az_storage_account_name' in 'ansible' in the config") - return False - if 'az_container_name' not in ansible_conf: - log.error("Missing 'az_container_name' in 'ansible' in the config") + log.error("Apiver: %d is no longer supported", apiver) + return False + if 'hana_media' not in ansible_conf or ansible_conf['hana_media'] is None: + log.error("Missing or empty 'hana_media' in 'ansible' in the config") + return False + for media in ansible_conf['hana_media']: + match = re.search(r'^http[s]?://.*', media) + if match: + log.error("Media %s provided as full url. File name expected.", media) return False - if 'az_sas_token' not in ansible_conf: - log.warning("Missing 'az_sas_token' in 'ansible' in the config") + if 'az_storage_account_name' not in ansible_conf: + log.error("Missing 'az_storage_account_name' in 'ansible' in the config") + return False + if 'az_container_name' not in ansible_conf: + log.error("Missing 'az_container_name' in 'ansible' in the config") + return False + if 'az_sas_token' not in ansible_conf: + log.warning("Missing 'az_sas_token' in 'ansible' in the config") return True def has_ansible_playbooks(self, sequence): diff --git a/scripts/qesap/test/conftest.py b/scripts/qesap/test/conftest.py index c13d5464..f74ec110 100644 --- a/scripts/qesap/test/conftest.py +++ b/scripts/qesap/test/conftest.py @@ -172,10 +172,13 @@ def _callback(playbook_list): def ansible_config(): def _callback(provider, playbooks): config_content = f"""--- -apiver: 2 +apiver: 3 provider: {provider} ansible: - hana_urls: somesome""" + az_container_name: pippo + az_storage_account_name: pippo + hana_media: + - pippo""" for seq in ["create", "destroy"]: if seq in playbooks: diff --git a/scripts/qesap/test/e2e/test_2.yaml b/scripts/qesap/test/e2e/test_2.yaml index 88f94ff3..72ea4eee 100644 --- a/scripts/qesap/test/e2e/test_2.yaml +++ b/scripts/qesap/test/e2e/test_2.yaml @@ -1,5 +1,5 @@ --- -apiver: 1 +apiver: 3 provider: fragola terraform: variables: diff --git a/scripts/qesap/test/unit/test_qesap_ansible.py b/scripts/qesap/test/unit/test_qesap_ansible.py index 81c62121..990cb377 100644 --- a/scripts/qesap/test/unit/test_qesap_ansible.py +++ b/scripts/qesap/test/unit/test_qesap_ansible.py @@ -145,7 +145,7 @@ def test_ansible_no_ansible(run, _, base_args, tmpdir): the `qesap.py ... ansible` command invocation has to fail. """ config_content = """--- -apiver: 2 +apiver: 3 provider: grilloparlante""" config_file_name = str(tmpdir / "config.yaml") with open(config_file_name, "w", encoding="utf-8") as file: @@ -267,7 +267,8 @@ def test_ansible_no_bin( # Create the inventory and the playbook file # is needed to avoid the tested function to fails # due to the absence of them. - # This test does not want it: this test has to get a failure due to the lack of binary. + # This test does not want it: this test has to get + # a failure due to the lack of binary. create_inventory(provider) create_playbooks(playbooks["create"]) @@ -375,10 +376,13 @@ def test_ansible_playbook_argument( """ provider = "grilloparlante" config_content = """--- -apiver: 2 +apiver: 3 provider: grilloparlante ansible: - hana_urls: somesome + az_storage_account_name: pippo + az_container_name: pippo + hana_media: + - somesome create: - baboom.yaml -e pim=pam """ @@ -423,10 +427,13 @@ def test_ansible_e_reg( """ provider = "grilloparlante" config_content = """--- -apiver: 2 +apiver: 3 provider: grilloparlante ansible: - hana_urls: somesome + az_storage_account_name: pippo + az_container_name: pippo + hana_media: + - somesome create: - registration.yaml -e reg_code=${reg_code} -e email_address=${email} variables: @@ -478,12 +485,15 @@ def test_ansible_e_sapconf( """ provider = "grilloparlante" config_content = """--- -apiver: 2 +apiver: 3 provider: grilloparlante ansible: - hana_urls: somesome + az_storage_account_name: pippo + az_container_name: pippo + hana_media: + - somesome create: - - sap-hana-preconfigure.yaml -e "use_sapconf=${sapconf}" + - sap-hana-preconfigure.yaml -e "use_sapconf=${sapconf}" variables: sapconf: True """ @@ -731,10 +741,13 @@ def test_ansible_env_roles_path( """ provider = "grilloparlante" config_content = f"""--- -apiver: 2 +apiver: 3 provider: {provider} ansible: - hana_urls: somesome + az_storage_account_name: pippo + az_container_name: pippo + hana_media: + - somesome roles_path: somewhere create: - get_cherry_wood.yaml""" diff --git a/scripts/qesap/test/unit/test_qesap_configure.py b/scripts/qesap/test/unit/test_qesap_configure.py index 6318045a..2cd9b0b4 100644 --- a/scripts/qesap/test/unit/test_qesap_configure.py +++ b/scripts/qesap/test/unit/test_qesap_configure.py @@ -17,7 +17,7 @@ def test_configure(configure_helper, config_yaml_sample): def test_configure_apiver(configure_helper): ''' - The configure has to have a apiver field at top level + The configure has to have an apiver field at top level ''' provider = 'pinocchio' conf = f"""--- @@ -87,7 +87,7 @@ def test_configure_checkfolder(base_args, tmpdir): config_file_name = str(tmpdir / 'config.yaml') with open(config_file_name, 'w', encoding='utf-8') as file: file.write(f"""--- -apiver: 2 +apiver: 3 provider: {provider} ansible: hana_urls: onlyone @@ -147,7 +147,7 @@ def test_configure_fail_at_missing_params(configure_helper): assert main(args) == 1 conf = """--- -apiver: 2 +apiver: 3 provider: terraform: ansible:""" @@ -155,7 +155,7 @@ def test_configure_fail_at_missing_params(configure_helper): assert main(args) == 1 conf = """--- -apiver: 2 +apiver: 3 provider: something terraform: ansible:""" @@ -177,7 +177,7 @@ def test_configure_check_terraform_cloud_provider(base_args, tmpdir): config_file_name = str(tmpdir / 'config.yaml') with open(config_file_name, 'w', encoding='utf-8') as file: file.write(f"""--- -apiver: 2 +apiver: 3 provider: {provider} ansible: hana_urls: onlyone @@ -203,7 +203,6 @@ def test_configure_without_ansible(base_args, tmpdir): config_file_name = str(tmpdir / 'config.yaml') with open(config_file_name, 'w', encoding='utf-8') as file: file.write(config) - os.makedirs(os.path.join(tmpdir, 'terraform')) os.makedirs(os.path.join(tmpdir, 'terraform', 'pinocchio')) args = base_args(base_dir=tmpdir, config_file=config_file_name) @@ -211,7 +210,41 @@ def test_configure_without_ansible(base_args, tmpdir): assert main(args) == 0 -def test_configure_without_ansible_in_older_apiver(base_args, tmpdir): +def test_configure_apiver_older_than_3(base_args, tmpdir): + ''' + Test is using the no more supported apiver:2 + so the configure has just to fail. + ''' + config = """--- +apiver: 2 +provider: pinocchio +terraform: + variables: + az_region: "westeurope" + hana_ips: ["10.0.0.2", "10.0.0.3"] + hana_data_disks_configuration: + disk_type: "hdd,hdd,hdd" + disks_size: "64,64,64" +ansible: pippo +""" + config_file_name = str(tmpdir / 'config.yaml') + with open(config_file_name, 'w', encoding='utf-8') as file: + file.write(config) + os.makedirs(os.path.join(tmpdir, 'terraform', 'pinocchio')) + os.makedirs(os.path.join(tmpdir, 'ansible', 'playbooks', 'vars')) + + args = base_args(base_dir=tmpdir, config_file=config_file_name) + args.append('configure') + assert main(args) == 1 + + +def test_configure_apiver_older_than_3_no_ansible(base_args, tmpdir): + ''' + Test is using the no more supported apiver:2 + but it also does not have the ansible section at all + that is the only impacted one. + Allow this conf.yaml with old apiver in this case. + ''' config = """--- apiver: 2 provider: pinocchio @@ -226,7 +259,6 @@ def test_configure_without_ansible_in_older_apiver(base_args, tmpdir): config_file_name = str(tmpdir / 'config.yaml') with open(config_file_name, 'w', encoding='utf-8') as file: file.write(config) - os.makedirs(os.path.join(tmpdir, 'terraform')) os.makedirs(os.path.join(tmpdir, 'terraform', 'pinocchio')) args = base_args(base_dir=tmpdir, config_file=config_file_name) diff --git a/scripts/qesap/test/unit/test_qesap_configure_ansible.py b/scripts/qesap/test/unit/test_qesap_configure_ansible.py index 083dd16d..dafcb679 100644 --- a/scripts/qesap/test/unit/test_qesap_configure_ansible.py +++ b/scripts/qesap/test/unit/test_qesap_configure_ansible.py @@ -17,106 +17,6 @@ def test_configure_create_ansible_hanamedia(configure_helper, config_yaml_sample assert os.path.isfile(hana_media) -def test_configure_ansible_hanamedia_content_apiver2(configure_helper, validate_hana_media): - """ - Test that an old apiver:2 config.yaml - - ``` - ansible: - hana_urls: - - https://.blob.core.windows.net// - - https://.blob.core.windows.net//IMDB_SERVER - - https://.blob.core.windows.net//IMDB_CLIENT - ``` - - during 'configure', write a hana_media.yaml with - expected content - - ``` - az_storage_account_name: - az_container_name: - az_sas_token: - az_blobs: - - - - - - - ``` - """ - provider = 'pinocchio' - conf = f"""--- -apiver: 2 -provider: {provider} -terraform: - variables: - az_region: "westeurope" -ansible: - hana_urls: - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT""" - args, _, hana_media, _ = configure_helper(provider, conf) - assert main(args) == 0 - - res, msg = validate_hana_media(hana_media, account='SOMEONE', container='SOMETHING', token=None, sapcar='MY_SAPCAR_EXE', imdb_srv='MY_IMDB_SERVER', imdb_cln='MY_IMDB_CLIENT') - assert res, msg - - -def test_configure_ansible_hanamedia_content_apiver2_invalidurl(configure_helper, validate_hana_media): - """ - Test conversion old apiver:2 config.yaml that is using invalid urls - """ - provider = 'pinocchio' - wrong_hana_urls = [ - ( - "MISSING ACCOUNT", - """ - - https://.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT""", - ), - ( - "MISSING CONTAINER", - """ - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONE.blob.core.windows.net//MY_IMDB_CLIENT - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT""", - ), - ( - "MISSING EXE", - """ - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER - - https://SOMEONE.blob.core.windows.net/SOMETHING/""", - ), - ( - "DIFFERENT ACCOUNT", - """ - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONEELSE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER # This is the wrong one DIFFERENT ACCOUNT - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT""", - ), - ( - "DIFFERENT CONTAINER", - """ - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER # This is the wrong one DIFFERENT ACCOUNT - - https://SOMEONE.blob.core.windows.net/SOMETHINGELSE/MY_IMDB_CLIENT""", - ), - ] - for this_set in wrong_hana_urls: - conf = f"""--- -apiver: 2 -provider: {provider} -terraform: - variables: - az_region: "westeurope" -ansible: - hana_urls: -{this_set[1]}""" - args, _, hana_media, _ = configure_helper(provider, conf) - assert main(args) != 0, f"{this_set[0]} error not detected" - - def test_configure_ansible_hanamedia_content_apiver3(configure_helper, validate_hana_media): """ Test that an new apiver:3 config.yaml @@ -240,31 +140,6 @@ def test_configure_create_ansible_hanavars(configure_helper, config_yaml_sample) assert os.path.isfile(hana_vars) -def test_configure_not_create_ansible_hanavars_apiver1(configure_helper, config_yaml_sample): - """ - Test that 'configure' does not write a hana_vars.yaml file in - /ansible/playbooks/vars - if apiver < 2 - """ - this_provider = 'pinocchio' - conf = f"""--- -apiver: 1 -provider: {this_provider} -terraform: - variables: - az_region: "westeurope" -ansible: - hana_urls: - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_SAPCAR_EXE - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_SERVER - - https://SOMEONE.blob.core.windows.net/SOMETHING/MY_IMDB_CLIENT""" - - args, _, _, hana_vars = configure_helper(this_provider, conf) - - assert main(args) == 0 - assert not os.path.isfile(hana_vars) - - def test_configure_ansible_hanavar_content(configure_helper): """ Test that 'configure' write a hana_vars.yaml with