diff --git a/merlin/server/server_config.py b/merlin/server/server_config.py index 5b89f2f6..9f6047a1 100644 --- a/merlin/server/server_config.py +++ b/merlin/server/server_config.py @@ -136,6 +136,31 @@ def parse_redis_output(redis_stdout: BufferedReader) -> Tuple[bool, str]: return False, "Reached end of redis output without seeing 'Ready to accept connections'" +def write_container_command_files(config_dir: str) -> bool: + """ + Write the yaml files that contain instructions on how to + run certain commands for each container type. + + :param config_dir: The path to the configuration dir where we'll write files + :returns: True if successful. False otherwise. + """ + files = [i + ".yaml" for i in CONTAINER_TYPES] + for file in files: + file_path = os.path.join(config_dir, file) + if os.path.exists(file_path): + LOG.info(f"{file} already exists.") + continue + LOG.info(f"Copying file {file} to configuration directory.") + try: + with resources.path("merlin.server", file) as config_file: + with open(file_path, "w") as outfile, open(config_file, "r") as infile: + outfile.write(infile.read()) + except OSError: + LOG.error(f"Destination location {config_dir} is not writable.") + return False + return True + + def create_server_config() -> bool: """ Create main configuration file for merlin server in the @@ -158,20 +183,8 @@ def create_server_config() -> bool: LOG.error(err) return False - files = [i + ".yaml" for i in CONTAINER_TYPES] - for file in files: - file_path = os.path.join(config_dir, file) - if os.path.exists(file_path): - LOG.info(f"{file} already exists.") - continue - LOG.info(f"Copying file {file} to configuration directory.") - try: - with resources.path("merlin.server", file) as config_file: - with open(file_path, "w") as outfile, open(config_file, "r") as infile: - outfile.write(infile.read()) - except OSError: - LOG.error(f"Destination location {config_dir} is not writable.") - return False + if not write_container_command_files(config_dir): + return False # Load Merlin Server Configuration and apply it to app.yaml with resources.path("merlin.server", MERLIN_SERVER_CONFIG) as merlin_server_config: @@ -189,6 +202,7 @@ def create_server_config() -> bool: return False if not os.path.exists(server_config.container.get_config_dir()): + print("inside get_config_dir if statement") LOG.info("Creating merlin server directory.") os.mkdir(server_config.container.get_config_dir()) diff --git a/tests/unit/server/test_server_config.py b/tests/unit/server/test_server_config.py index 035e70d6..eaac6bc9 100644 --- a/tests/unit/server/test_server_config.py +++ b/tests/unit/server/test_server_config.py @@ -3,11 +3,14 @@ """ import io +import logging +import os import string from typing import Dict, Tuple, Union import pytest +from merlin.server.server_util import CONTAINER_TYPES, ServerConfig from merlin.server.server_config import ( PASSWORD_LENGTH, check_process_file_format, @@ -20,6 +23,7 @@ pull_process_file, pull_server_config, pull_server_image, + write_container_command_files, ) @@ -106,3 +110,182 @@ def test_parse_redis_output_with_vars(lines: bytes, expected_config: Tuple[bool, reader_input = io.BufferedReader(buffer) _, actual_vars = parse_redis_output(reader_input) assert expected_config == actual_vars + + +def test_write_container_command_files_with_existing_files( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Test the `write_container_command_files` function with files that already exist. + This should skip trying to create the files, log 3 "file already exists" messages, + and return True. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + caplog.set_level(logging.INFO) + mocker.patch('os.path.exists', return_value=True) + assert write_container_command_files(server_testing_dir) + file_names = [f"{container}.yaml" for container in CONTAINER_TYPES] + for file in file_names: + assert f"{file} already exists." in caplog.text + + +def test_write_container_command_files_with_nonexisting_files( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Test the `write_container_command_files` function with files that don't already exist. + This should create the files, log messages for each file, and return True + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + caplog.set_level(logging.INFO) + + # Mock the os.path.exists function so it returns False + mocker.patch('os.path.exists', return_value=False) + + # Mock the resources.path context manager + mock_path = mocker.patch("merlin.server.server_config.resources.path") + mock_path.return_value.__enter__.return_value = "mocked_file_path" + + # Mock the open builtin + mock_data = mocker.mock_open(read_data="mocked data") + mocker.patch("builtins.open", mock_data) + + assert write_container_command_files(server_testing_dir) + file_names = [f"{container}.yaml" for container in CONTAINER_TYPES] + for file in file_names: + assert f"Copying file {file} to configuration directory." in caplog.text + + +def test_write_container_command_files_with_oserror( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Test the `write_container_command_files` function with an OSError being raised. + This should log an error message and return False. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + # Mock the open function to raise an OSError + mocker.patch("builtins.open", side_effect=OSError("File not writeable")) + + assert not write_container_command_files(server_testing_dir) + assert f"Destination location {server_testing_dir} is not writable." in caplog.text + + +def test_create_server_config_merlin_config_dir_nonexistent( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Tests the `create_server_config` function with MERLIN_CONFIG_DIR not existing. + This should log an error and return False. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + nonexistent_dir = f"{server_testing_dir}/merlin_config_dir" + mocker.patch('merlin.server.server_config.MERLIN_CONFIG_DIR', nonexistent_dir) + assert not create_server_config() + assert f"Unable to find main merlin configuration directory at {nonexistent_dir}" in caplog.text + + +def test_create_server_config_server_subdir_nonexistent_oserror( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Tests the `create_server_config` function with MERLIN_CONFIG_DIR/MERLIN_SERVER_SUBDIR + not existing and an OSError being raised. This should log an error and return False. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + + # Mock MERLIN_CONFIG_DIR and MERLIN_SERVER_SUBDIR + nonexistent_server_subdir = "test_create_server_config_server_subdir_nonexistent" + mocker.patch('merlin.server.server_config.MERLIN_CONFIG_DIR', server_testing_dir) + mocker.patch('merlin.server.server_config.MERLIN_SERVER_SUBDIR', nonexistent_server_subdir) + + # Mock os.mkdir so it raises an OSError + err_msg = "File not writeable" + mocker.patch("os.mkdir", side_effect=OSError(err_msg)) + assert not create_server_config() + assert err_msg in caplog.text + + +def test_create_server_config_no_server_config( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, +): + """ + Tests the `create_server_config` function with the call to `pull_server_config()` + returning None. This should log an error and return False. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + """ + + # Mock the necessary variables/functions to get us to the pull_server_config call + mocker.patch("merlin.server.server_config.MERLIN_CONFIG_DIR", server_testing_dir) + mocker.patch("merlin.server.server_config.write_container_command_files", return_value=True) + mock_open_func = mocker.mock_open(read_data='key: value') + mocker.patch("builtins.open", mock_open_func) + + # Mock the pull_server_config call (what we're actually testing) and run the test + mocker.patch("merlin.server.server_config.pull_server_config", return_value=None) + assert not create_server_config() + assert 'Try to run "merlin server init" again to reinitialize values.' in caplog.text + + +def test_create_server_config_no_server_dir( + mocker: "Fixture", # noqa: F821 + caplog: "Fixture", # noqa: F821 + server_testing_dir: str, + server_server_config: Dict[str, str], +): + """ + Tests the `create_server_config` function with the call to + `server_config.container.get_config_dir()` returning a non-existent path. This should + log a message and create the directory, then return True. + + :param mocker: A built-in fixture from the pytest-mock library to create a Mock object + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_testing_dir: The path to the the temp output directory for server tests + :param server_server_config: A pytest fixture of test data to pass to the ServerConfig class + """ + caplog.set_level(logging.INFO) + + # Mock the necessary variables/functions to get us to the get_config_dir call + mocker.patch("merlin.server.server_config.MERLIN_CONFIG_DIR", server_testing_dir) + mocker.patch("merlin.server.server_config.write_container_command_files", return_value=True) + mock_open_func = mocker.mock_open(read_data='key: value') + mocker.patch("builtins.open", mock_open_func) + mocker.patch("merlin.server.server_config.pull_server_config", return_value=ServerConfig(server_server_config)) + + # Mock the get_config_dir call to return a directory that doesn't exist yet + nonexistent_dir = f"{server_testing_dir}/merlin_server" + mocker.patch("merlin.server.server_util.ContainerConfig.get_config_dir", return_value=nonexistent_dir) + + assert create_server_config() + assert os.path.exists(nonexistent_dir) + assert "Creating merlin server directory." in caplog.text