From 7c75a7f3765ac5bc1367f5f65cec4bbc9a7d3255 Mon Sep 17 00:00:00 2001 From: Rodolfo Olivieri Date: Tue, 2 Jul 2024 10:59:14 -0300 Subject: [PATCH] Add support for input parameters (#102) * Add support for input parameters This patch introduces the support for input parameters that will come from the Insights UI. Regarding the environment variables value that will be passed down to us, there is no change that we need to do in order to receive it. The only change that was necessary was to adapt the code to receive the CLI switches. In this case, we are handling it pretty simple, only identifying the two possibilities for now and working with it. In the future, if more options appear, we can improve the code. * Skip environmnet variables that have value as 0 If an environment variable has value as 0, we will skip setting them in the subprocess. The reason for this is that convert2rhel does not handle well the case for values in those env vars. Convert2rhel currently only checks for the presence of the env var in the os.environ object. If it is set, then it will skip the check, otherwise, it will follow the execution normally. --- convert2rhel_insights_tasks/main.py | 67 +++++++++-- playbooks/convert-to-rhel-analysis.yml | 77 ++++++++++-- playbooks/convert-to-rhel-conversion.yml | 76 ++++++++++-- tests/test_environment_variables.py | 75 ++++++++++++ tests/test_run.py | 146 +++++++++++++++++++---- 5 files changed, 386 insertions(+), 55 deletions(-) create mode 100644 tests/test_environment_variables.py diff --git a/convert2rhel_insights_tasks/main.py b/convert2rhel_insights_tasks/main.py index 2593128..584496c 100644 --- a/convert2rhel_insights_tasks/main.py +++ b/convert2rhel_insights_tasks/main.py @@ -657,12 +657,65 @@ def install_or_update_convert2rhel(required_files): return False, None -def run_convert2rhel(): +def prepare_environment_variables(env): + """Prepare environment variables to be used in subprocess + + This metod will prepare any environment variables before they are sent down + to the subprocess that will convert2rhel. Currently, this is meant to be a + workaround since convert2rhel does not parse the value of the environment + variables, but only check the presence of them in os.environ. + + With this function, we are make sure that any variables that have the value + 0 are ignored before setting them in the subprocess env context, this will + prevent convert2rhel to wrongly skipping checks because it was pre-defined + in the insights playbook. + + :param env: The environment variables before setting them in subprocess. + :type env: dict[str, Any] + """ + for variable, value in env.items(): + if variable.startswith("CONVERT2RHEL_") and value == 0: + env.pop(variable) + return env + + +def run_convert2rhel(env): """ Run the convert2rhel tool assigning the correct environment variables. + + :param env: Dictionary of possible environment variables to passed down to + the process. + :type env: dict[str] """ logger.info("Running Convert2RHEL %s", (SCRIPT_TYPE.title())) + command = ["/usr/bin/convert2rhel"] + if IS_ANALYSIS: + command.append("analyze") + + command.append("-y") + + # This will always be represented as either false/true, since this option + # comes from the input parameters through Insights UI. + els_disabled = json.loads(env.pop("ELS_DISABLED", "false").lower()) + if not bool(els_disabled): + command.append("--els") + + optional_repositories = env.pop("OPTIONAL_REPOSITORIES", []) + if optional_repositories: + repositories = optional_repositories.split(",") + repositories = [ + "--enablerepo=%s" % repository.strip() for repository in repositories + ] + command.extend(repositories) + + env = prepare_environment_variables(env) + output, returncode = run_subprocess(command, env=env) + return output, returncode + + +def parse_environment_variables(): + """Read the environment variable from os.environ and return them.""" new_env = {} for key, value in os.environ.items(): valid_prefix = "RHC_WORKER_" @@ -671,14 +724,7 @@ def run_convert2rhel(): new_env[key.replace(valid_prefix, "")] = value else: new_env[key] = value - - command = ["/usr/bin/convert2rhel"] - if IS_ANALYSIS: - command.append("analyze") - - command.append("-y") - output, returncode = run_subprocess(command, env=new_env) - return output, returncode + return new_env def cleanup(required_files): @@ -881,7 +927,8 @@ def main(): YUM_TRANSACTIONS_TO_UNDO.add(transaction_id) check_convert2rhel_inhibitors_before_run() - stdout, returncode = run_convert2rhel() + new_env = parse_environment_variables() + stdout, returncode = run_convert2rhel(new_env) execution_successful = returncode == 0 # Returncode other than 0 can happen in three states in analysis mode: diff --git a/playbooks/convert-to-rhel-analysis.yml b/playbooks/convert-to-rhel-analysis.yml index bf374a3..467792d 100644 --- a/playbooks/convert-to-rhel-analysis.yml +++ b/playbooks/convert-to-rhel-analysis.yml @@ -4,7 +4,7 @@ vars: insights_signature: | ascii_armored gpg signature - insights_signature_exclude: /vars/insights_signature + insights_signature_exclude: /vars/insights_signature,/vars/content_vars interpreter: /usr/bin/python2 content: | import json @@ -666,12 +666,65 @@ return False, None - def run_convert2rhel(): + def prepare_environment_variables(env): + """Prepare environment variables to be used in subprocess + + This metod will prepare any environment variables before they are sent down + to the subprocess that will convert2rhel. Currently, this is meant to be a + workaround since convert2rhel does not parse the value of the environment + variables, but only check the presence of them in os.environ. + + With this function, we are make sure that any variables that have the value + 0 are ignored before setting them in the subprocess env context, this will + prevent convert2rhel to wrongly skipping checks because it was pre-defined + in the insights playbook. + + :param env: The environment variables before setting them in subprocess. + :type env: dict[str, Any] + """ + for variable, value in env.items(): + if variable.startswith("CONVERT2RHEL_") and value == 0: + env.pop(variable) + return env + + + def run_convert2rhel(env): """ Run the convert2rhel tool assigning the correct environment variables. + + :param env: Dictionary of possible environment variables to passed down to + the process. + :type env: dict[str] """ logger.info("Running Convert2RHEL %s", (SCRIPT_TYPE.title())) + command = ["/usr/bin/convert2rhel"] + if IS_ANALYSIS: + command.append("analyze") + + command.append("-y") + + # This will always be represented as either false/true, since this option + # comes from the input parameters through Insights UI. + els_disabled = json.loads(env.pop("ELS_DISABLED", "false").lower()) + if not bool(els_disabled): + command.append("--els") + + optional_repositories = env.pop("OPTIONAL_REPOSITORIES", []) + if optional_repositories: + repositories = optional_repositories.split(",") + repositories = [ + "--enablerepo=%s" % repository.strip() for repository in repositories + ] + command.extend(repositories) + + env = prepare_environment_variables(env) + output, returncode = run_subprocess(command, env=env) + return output, returncode + + + def parse_environment_variables(): + """Read the environment variable from os.environ and return them.""" new_env = {} for key, value in os.environ.items(): valid_prefix = "RHC_WORKER_" @@ -680,14 +733,7 @@ new_env[key.replace(valid_prefix, "")] = value else: new_env[key] = value - - command = ["/usr/bin/convert2rhel"] - if IS_ANALYSIS: - command.append("analyze") - - command.append("-y") - output, returncode = run_subprocess(command, env=new_env) - return output, returncode + return new_env def cleanup(required_files): @@ -890,7 +936,8 @@ YUM_TRANSACTIONS_TO_UNDO.add(transaction_id) check_convert2rhel_inhibitors_before_run() - stdout, returncode = run_convert2rhel() + new_env = parse_environment_variables() + stdout, returncode = run_convert2rhel(new_env) execution_successful = returncode == 0 # Returncode other than 0 can happen in three states in analysis mode: @@ -1033,4 +1080,12 @@ if __name__ == "__main__": main() content_vars: + CONVERT2RHEL_THROUGH_INSIGHTS: 1 SCRIPT_MODE: ANALYSIS + CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS: 0 + CONVERT2RHEL_UNSUPPORTED_SKIP_KERNEL_CURRENCY_CHECK: 0 + CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP: 0 + ELS_DISABLED: false + # Multiple optional repository parameter values will be comma separated, for example: + # rhel-7-server-optional-rpms,rhel-7-server-extras-rpms,rhel-7-server-supplementary-rpms + OPTIONAL_REPOSITORIES: None diff --git a/playbooks/convert-to-rhel-conversion.yml b/playbooks/convert-to-rhel-conversion.yml index dbc8a83..b32273c 100644 --- a/playbooks/convert-to-rhel-conversion.yml +++ b/playbooks/convert-to-rhel-conversion.yml @@ -4,7 +4,7 @@ vars: insights_signature: | ascii_armored gpg signature - insights_signature_exclude: /vars/insights_signature + insights_signature_exclude: /vars/insights_signature,/vars/content_vars interpreter: /usr/bin/python2 content: | import json @@ -666,12 +666,65 @@ return False, None - def run_convert2rhel(): + def prepare_environment_variables(env): + """Prepare environment variables to be used in subprocess + + This metod will prepare any environment variables before they are sent down + to the subprocess that will convert2rhel. Currently, this is meant to be a + workaround since convert2rhel does not parse the value of the environment + variables, but only check the presence of them in os.environ. + + With this function, we are make sure that any variables that have the value + 0 are ignored before setting them in the subprocess env context, this will + prevent convert2rhel to wrongly skipping checks because it was pre-defined + in the insights playbook. + + :param env: The environment variables before setting them in subprocess. + :type env: dict[str, Any] + """ + for variable, value in env.items(): + if variable.startswith("CONVERT2RHEL_") and value == 0: + env.pop(variable) + return env + + + def run_convert2rhel(env): """ Run the convert2rhel tool assigning the correct environment variables. + + :param env: Dictionary of possible environment variables to passed down to + the process. + :type env: dict[str] """ logger.info("Running Convert2RHEL %s", (SCRIPT_TYPE.title())) + command = ["/usr/bin/convert2rhel"] + if IS_ANALYSIS: + command.append("analyze") + + command.append("-y") + + # This will always be represented as either false/true, since this option + # comes from the input parameters through Insights UI. + els_disabled = json.loads(env.pop("ELS_DISABLED", "false").lower()) + if not bool(els_disabled): + command.append("--els") + + optional_repositories = env.pop("OPTIONAL_REPOSITORIES", []) + if optional_repositories: + repositories = optional_repositories.split(",") + repositories = [ + "--enablerepo=%s" % repository.strip() for repository in repositories + ] + command.extend(repositories) + + env = prepare_environment_variables(env) + output, returncode = run_subprocess(command, env=env) + return output, returncode + + + def parse_environment_variables(): + """Read the environment variable from os.environ and return them.""" new_env = {} for key, value in os.environ.items(): valid_prefix = "RHC_WORKER_" @@ -680,14 +733,7 @@ new_env[key.replace(valid_prefix, "")] = value else: new_env[key] = value - - command = ["/usr/bin/convert2rhel"] - if IS_ANALYSIS: - command.append("analyze") - - command.append("-y") - output, returncode = run_subprocess(command, env=new_env) - return output, returncode + return new_env def cleanup(required_files): @@ -890,7 +936,8 @@ YUM_TRANSACTIONS_TO_UNDO.add(transaction_id) check_convert2rhel_inhibitors_before_run() - stdout, returncode = run_convert2rhel() + new_env = parse_environment_variables() + stdout, returncode = run_convert2rhel(new_env) execution_successful = returncode == 0 # Returncode other than 0 can happen in three states in analysis mode: @@ -1036,3 +1083,10 @@ CONVERT2RHEL_THROUGH_INSIGHTS: 1 CONVERT2RHEL_CONFIGURE_HOST_METERING: auto SCRIPT_MODE: CONVERSION + CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS: 0 + CONVERT2RHEL_UNSUPPORTED_SKIP_KERNEL_CURRENCY_CHECK: 0 + CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP: 0 + ELS_DISABLED: false + # Multiple optional repository parameter values will be comma separated, for example: + # rhel-7-server-optional-rpms,rhel-7-server-extras-rpms,rhel-7-server-supplementary-rpms + OPTIONAL_REPOSITORIES: None diff --git a/tests/test_environment_variables.py b/tests/test_environment_variables.py new file mode 100644 index 0000000..cbe582c --- /dev/null +++ b/tests/test_environment_variables.py @@ -0,0 +1,75 @@ +import os +import pytest + +from convert2rhel_insights_tasks import main + + +@pytest.mark.parametrize( + ("env", "expected"), + ( + ({"RHC_WORKER_SCRIPT_MODE": "CONVERSION"}, {"SCRIPT_MODE": "CONVERSION"}), + ( + {"RHC_WORKER_FOO": "BAR", "RHC_WORKER_BAR": "FOO"}, + {"FOO": "BAR", "BAR": "FOO"}, + ), + ( + {"RHC_WORKER_FOO": "BAR", "RHC_BAR": "FOO"}, + {"FOO": "BAR", "RHC_BAR": "FOO"}, + ), + ( + {"FOO": "BAR", "BAR": "FOO"}, + {"FOO": "BAR", "BAR": "FOO"}, + ), + ), +) +def test_parse_environment_variables(env, expected, monkeypatch): + monkeypatch.setattr(os, "environ", env) + result = main.parse_environment_variables() + + assert result == expected + + +def test_parse_environment_variables_empty(monkeypatch): + monkeypatch.setattr(os, "environ", {}) + result = main.parse_environment_variables() + assert not result + + +@pytest.mark.parametrize( + ("env", "expected"), + ( + ( + {}, + {}, + ), + ( + {"CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": 0}, + {}, + ), + ( + {"CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": 1}, + {"CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": 1}, + ), + ( + {"CONVERT2RHEL_CONFIGURE_HOST_METERING": "auto"}, + {"CONVERT2RHEL_CONFIGURE_HOST_METERING": "auto"}, + ), + ( + {"CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": 0, "SCRIPT_MODE": "ANALYSIS"}, + {"SCRIPT_MODE": "ANALYSIS"}, + ), + ( + { + "CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP": 1, + "CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": 0, + "SCRIPT_MODE": "ANALYSIS", + }, + {"CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP": 1, "SCRIPT_MODE": "ANALYSIS"}, + ), + ), +) +def test_prepare_environment_variables(env, expected, monkeypatch): + monkeypatch.setattr(os, "environ", env) + result = main.prepare_environment_variables(env) + + assert result == expected diff --git a/tests/test_run.py b/tests/test_run.py index d202982..c2642ae 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,60 +1,160 @@ +import pytest + from mock import patch +from convert2rhel_insights_tasks import main from convert2rhel_insights_tasks.main import run_convert2rhel -@patch("convert2rhel_insights_tasks.main.SCRIPT_TYPE", "ANALYSIS") -@patch("convert2rhel_insights_tasks.main.IS_ANALYSIS", True) -def test_run_convert2rhel_custom_variables(): - mock_env = {"FOO": "BAR", "BAR": "BAZ", "RHC_WORKER_LALA": "LAND"} +def test_run_convert2rhel_custom_variables(monkeypatch): + monkeypatch.setattr(main, "IS_ANALYSIS", True) + mock_env = {"FOO": "BAR", "BAR": "BAZ", "LALA": "LAND"} - with patch("os.environ", mock_env), patch( + with patch( "convert2rhel_insights_tasks.main.run_subprocess", return_value=(b"", 0) ) as mock_popen: - run_convert2rhel() + run_convert2rhel(mock_env) mock_popen.assert_called_once_with( - ["/usr/bin/convert2rhel", "analyze", "-y"], + ["/usr/bin/convert2rhel", "analyze", "-y", "--els"], env={"FOO": "BAR", "BAR": "BAZ", "LALA": "LAND"}, ) -@patch("convert2rhel_insights_tasks.main.SCRIPT_TYPE", "CONVERSION") -def test_run_convert2rhel_conversion(): +def test_run_convert2rhel_conversion(monkeypatch): + monkeypatch.setattr(main, "SCRIPT_TYPE", "CONVERSION") mock_env = { "PATH": "/fake/path", - "RHC_WORKER_CONVERT2RHEL_DISABLE_TELEMETRY": "1", - "RHC_WORKER_FOO": "1", - "RHC_WORKER_RHC_WORKER_BAR": "1", + "CONVERT2RHEL_DISABLE_TELEMETRY": "1", + "FOO": "1", } - with patch("os.environ", mock_env), patch( + with patch( "convert2rhel_insights_tasks.main.run_subprocess", return_value=(b"", 0) ) as mock_popen: - run_convert2rhel() + run_convert2rhel(mock_env) mock_popen.assert_called_once_with( - ["/usr/bin/convert2rhel", "-y"], + ["/usr/bin/convert2rhel", "-y", "--els"], env={ "PATH": "/fake/path", "CONVERT2RHEL_DISABLE_TELEMETRY": "1", "FOO": "1", - "BAR": "1", }, ) -@patch("convert2rhel_insights_tasks.main.SCRIPT_TYPE", "ANALYSIS") -@patch("convert2rhel_insights_tasks.main.IS_ANALYSIS", True) -def test_run_convert2rhel_analysis(): - mock_env = {"PATH": "/fake/path", "RHC_WORKER_CONVERT2RHEL_DISABLE_TELEMETRY": "1"} +def test_run_convert2rhel_analysis(monkeypatch): + monkeypatch.setattr(main, "IS_ANALYSIS", True) + mock_env = {"PATH": "/fake/path", "CONVERT2RHEL_DISABLE_TELEMETRY": "1"} - with patch("os.environ", mock_env), patch( + with patch( "convert2rhel_insights_tasks.main.run_subprocess", return_value=(b"", 0) ) as mock_popen: - run_convert2rhel() + run_convert2rhel(mock_env) mock_popen.assert_called_once_with( - ["/usr/bin/convert2rhel", "analyze", "-y"], + ["/usr/bin/convert2rhel", "analyze", "-y", "--els"], env={"PATH": "/fake/path", "CONVERT2RHEL_DISABLE_TELEMETRY": "1"}, ) + + +@pytest.mark.parametrize( + ( + "is_analysis", + "els_disabled", + "expected_cmd", + ), + ( + (True, "True", ["/usr/bin/convert2rhel", "analyze", "-y"]), + (True, "true", ["/usr/bin/convert2rhel", "analyze", "-y"]), + (True, "False", ["/usr/bin/convert2rhel", "analyze", "-y", "--els"]), + (True, "false", ["/usr/bin/convert2rhel", "analyze", "-y", "--els"]), + # Make sure that this also pass for conversion + (False, "True", ["/usr/bin/convert2rhel", "-y"]), + (False, "true", ["/usr/bin/convert2rhel", "-y"]), + (False, "False", ["/usr/bin/convert2rhel", "-y", "--els"]), + (False, "false", ["/usr/bin/convert2rhel", "-y", "--els"]), + ), +) +def test_run_convert2rhel_els_option( + is_analysis, els_disabled, expected_cmd, monkeypatch +): + monkeypatch.setattr(main, "IS_ANALYSIS", is_analysis) + mock_env = {"PATH": "/fake/path", "ELS_DISABLED": els_disabled} + + with patch( + "convert2rhel_insights_tasks.main.run_subprocess", return_value=(b"", 0) + ) as mock_popen: + run_convert2rhel(mock_env) + + mock_popen.assert_called_once_with( + expected_cmd, + env={"PATH": "/fake/path"}, + ) + + +@pytest.mark.parametrize( + ( + "is_analysis", + "optional_repositories", + "expected_cmd", + ), + ( + ( + True, + "rhel-7-server-rpm", + [ + "/usr/bin/convert2rhel", + "analyze", + "-y", + "--els", + "--enablerepo=rhel-7-server-rpm", + ], + ), + ( + True, + "rhel-7-server-rpm, rhel-7-server-rpm-extras", + [ + "/usr/bin/convert2rhel", + "analyze", + "-y", + "--els", + "--enablerepo=rhel-7-server-rpm", + "--enablerepo=rhel-7-server-rpm-extras", + ], + ), + # Make sure that this also pass for conversion + ( + False, + "rhel-7-server-rpm", + ["/usr/bin/convert2rhel", "-y", "--els", "--enablerepo=rhel-7-server-rpm"], + ), + ( + False, + "rhel-7-server-rpm, rhel-7-server-rpm-extras", + [ + "/usr/bin/convert2rhel", + "-y", + "--els", + "--enablerepo=rhel-7-server-rpm", + "--enablerepo=rhel-7-server-rpm-extras", + ], + ), + ), +) +def test_run_convert2rhel_optional_repositories( + is_analysis, optional_repositories, expected_cmd, monkeypatch +): + monkeypatch.setattr(main, "IS_ANALYSIS", is_analysis) + mock_env = {"PATH": "/fake/path", "OPTIONAL_REPOSITORIES": optional_repositories} + + with patch( + "convert2rhel_insights_tasks.main.run_subprocess", return_value=(b"", 0) + ) as mock_popen: + run_convert2rhel(mock_env) + + mock_popen.assert_called_once_with( + expected_cmd, + env={"PATH": "/fake/path"}, + )