Skip to content

Commit

Permalink
[Builder] Install mlrun with python requirements (#3673)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alonmr
alonmr committed Jun 1, 2023
1 parent 2a94447 commit 0ee3090
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 23 deletions.
11 changes: 10 additions & 1 deletion mlrun/api/api/endpoints/frontend_spec.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def get_frontend_spec(
function_deployment_target_image_template=function_deployment_target_image_template,
function_deployment_target_image_name_prefix_template=function_target_image_name_prefix_template,
function_deployment_target_image_registries_to_enforce_prefix=registries_to_enforce_prefix,
function_deployment_mlrun_command=mlrun.api.utils.builder.resolve_mlrun_install_command(),
function_deployment_mlrun_command=_resolve_function_deployment_mlrun_command(),
auto_mount_type=config.storage.auto_mount_type,
auto_mount_params=config.get_storage_auto_mount_params(),
default_artifact_path=config.artifact_path,
Expand All @@ -86,6 +86,15 @@ def get_frontend_spec(
)


def _resolve_function_deployment_mlrun_command():
# TODO: When UI adds a requirements section, mlrun should be specified there instead of the commands section i.e.
# frontend spec will contain only the mlrun_version_specifier instead of the full command
mlrun_version_specifier = (
mlrun.api.utils.builder.resolve_mlrun_install_command_version()
)
return f'python -m pip install "{mlrun_version_specifier}"'


def _resolve_jobs_dashboard_url(session: str) -> typing.Optional[str]:
iguazio_client = mlrun.api.utils.clients.iguazio.Client()
grafana_service_url = iguazio_client.try_get_grafana_service_url(session)
Expand Down
19 changes: 11 additions & 8 deletions mlrun/api/utils/builder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,13 @@ def build_image(
image_target, secret_name = resolve_image_target_and_registry_secret(
image_target, registry, secret_name
)

requirements_path = "/empty/requirements.txt"
if requirements and isinstance(requirements, list):
requirements_list = requirements
requirements_path = "/empty/requirements.txt"
else:
requirements_list = None
requirements_path = requirements or ""
requirements_list = []
requirements_path = requirements or requirements_path

commands = commands or []
if with_mlrun:
Expand All @@ -341,11 +342,13 @@ def build_image(
if upgrade_pip_command:
commands.append(upgrade_pip_command)

mlrun_command = resolve_mlrun_install_command(
mlrun_version = resolve_mlrun_install_command_version(
mlrun_version_specifier, client_version, commands
)
if mlrun_command:
commands.append(mlrun_command)

# mlrun must be installed with other python requirements in the same pip command to avoid version conflicts
if mlrun_version:
requirements_list.insert(0, mlrun_version)

if not inline_code and not source and not commands and not requirements:
mlrun.utils.logger.info("skipping build, nothing to add")
Expand Down Expand Up @@ -492,7 +495,7 @@ def get_kaniko_spec_attributes_from_runtime():
]


def resolve_mlrun_install_command(
def resolve_mlrun_install_command_version(
mlrun_version_specifier=None, client_version=None, commands=None
):
commands = commands or []
Expand Down Expand Up @@ -522,7 +525,7 @@ def resolve_mlrun_install_command(
mlrun_version_specifier = (
f"{config.package_path}[complete]=={config.version}"
)
return f'python -m pip install "{mlrun_version_specifier}"'
return mlrun_version_specifier


def resolve_upgrade_pip_command(commands=None):
Expand Down
5 changes: 5 additions & 0 deletions tests/api/api/test_frontend_spec.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import sqlalchemy.orm

import mlrun.api.crud
import mlrun.api.utils.builder
import mlrun.api.utils.clients.iguazio
import mlrun.common.schemas
import mlrun.errors
Expand Down Expand Up @@ -99,6 +100,10 @@ def test_get_frontend_spec(
frontend_spec.allowed_artifact_path_prefixes_list
== mlrun.api.api.utils.get_allowed_path_prefixes_list()
)
assert (
frontend_spec.function_deployment_mlrun_command
== f'python -m pip install "{mlrun.api.utils.builder.resolve_mlrun_install_command_version()}"'
)


def test_get_frontend_spec_jobs_dashboard_url_resolution(
Expand Down
141 changes: 140 additions & 1 deletion tests/api/runtimes/test_kubejob.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,15 +767,154 @@ def test_deploy_upgrade_pip(
expected_str += "\nRUN "
expected_str += "\nRUN ".join(commands)
expected_str += f"\nRUN python -m pip install --upgrade pip{mlrun.mlconf.httpdb.builder.pip_version}"

# assert that mlrun was added to the requirements file
if with_mlrun:
expected_str += '\nRUN python -m pip install "mlrun[complete]'
expected_str += (
"\nRUN echo 'Installing /empty/requirements.txt...'; cat /empty/requirements.txt"
"\nRUN python -m pip install -r /empty/requirements.txt"
)
kaniko_pod_requirements = (
mlrun.api.utils.builder.make_kaniko_pod.call_args[1][
"requirements"
]
)
assert kaniko_pod_requirements == [
"mlrun[complete] @ git+https://github.com/mlrun/mlrun@development"
]
assert expected_str in dockerfile
else:
assert (
f"pip install --upgrade pip{mlrun.mlconf.httpdb.builder.pip_version}"
not in dockerfile
)

@pytest.mark.parametrize(
"with_mlrun, requirements, with_requirements_file, expected_requirements",
[
(
True,
[],
False,
["mlrun[complete] @ git+https://github.com/mlrun/mlrun@development"],
),
(
True,
["pandas"],
False,
[
"mlrun[complete] @ git+https://github.com/mlrun/mlrun@development",
"pandas",
],
),
(
True,
["pandas", "tensorflow"],
False,
[
"mlrun[complete] @ git+https://github.com/mlrun/mlrun@development",
"pandas",
"tensorflow",
],
),
(False, [], True, ["faker", "python-dotenv", "chardet>=3.0.2, <4.0"]),
(False, ["pandas", "tensorflow"], False, ["pandas", "tensorflow"]),
(
False,
["pandas", "tensorflow"],
True,
[
"faker",
"python-dotenv",
"chardet>=3.0.2, <4.0",
"pandas",
"tensorflow",
],
),
(
True,
["pandas", "tensorflow"],
True,
[
"mlrun[complete] @ git+https://github.com/mlrun/mlrun@development",
"faker",
"python-dotenv",
"chardet>=3.0.2, <4.0",
"pandas",
"tensorflow",
],
),
(
True,
[],
True,
[
"mlrun[complete] @ git+https://github.com/mlrun/mlrun@development",
"faker",
"python-dotenv",
"chardet>=3.0.2, <4.0",
],
),
],
)
def test_deploy_with_mlrun(
self,
db: Session,
client: TestClient,
with_mlrun,
requirements,
with_requirements_file,
expected_requirements,
):
mlrun.mlconf.httpdb.builder.docker_registry = "localhost:5000"
with unittest.mock.patch(
"mlrun.api.utils.builder.make_kaniko_pod", unittest.mock.MagicMock()
):
runtime = self._generate_runtime()
runtime.spec.build.base_image = "some/image"

requirements_file = (
"" if not with_requirements_file else self.requirements_file
)
runtime.with_requirements(
requirements=requirements, requirements_file=requirements_file
)

self.deploy(db, runtime, with_mlrun=with_mlrun)
dockerfile = mlrun.api.utils.builder.make_kaniko_pod.call_args[1][
"dockertext"
]

install_requirements_commands = (
"\nRUN echo 'Installing /empty/requirements.txt...'; cat /empty/requirements.txt"
"\nRUN python -m pip install -r /empty/requirements.txt"
)
kaniko_pod_requirements = mlrun.api.utils.builder.make_kaniko_pod.call_args[
1
]["requirements"]
if with_mlrun:
expected_str = f"\nRUN python -m pip install --upgrade pip{mlrun.mlconf.httpdb.builder.pip_version}"
expected_str += install_requirements_commands
assert kaniko_pod_requirements == expected_requirements
assert expected_str in dockerfile

else:
assert (
f"pip install --upgrade pip{mlrun.mlconf.httpdb.builder.pip_version}"
not in dockerfile
)

# assert that install requirements commands are in the dockerfile
if with_requirements_file or requirements:
expected_str = install_requirements_commands
assert expected_str in dockerfile

# assert mlrun is not in the requirements
for requirement in kaniko_pod_requirements:
assert "mlrun" not in requirement

assert kaniko_pod_requirements == expected_requirements

@pytest.mark.parametrize(
"workdir, source, pull_at_runtime, target_dir, expected_workdir",
[
Expand Down
24 changes: 11 additions & 13 deletions tests/api/utils/test_builder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,16 +339,14 @@ def test_function_build_with_default_requests(monkeypatch):
)


def test_resolve_mlrun_install_command():
pip_command = "python -m pip install"
def test_resolve_mlrun_install_command_version():
cases = [
{
"test_description": "when mlrun_version_specifier configured, expected to install mlrun_version_specifier",
"mlrun_version_specifier": "mlrun[complete] @ git+https://github.com/mlrun/mlrun@v0.10.0",
"client_version": "0.9.3",
"server_mlrun_version_specifier": None,
"expected_mlrun_install_command": f"{pip_command} "
f'"mlrun[complete] @ git+https://github.com/mlrun/mlrun@v0.10.0"',
"expected_mlrun_install_command_version": "mlrun[complete] @ git+https://github.com/mlrun/mlrun@v0.10.0",
},
{
"test_description": "when mlrun_version_specifier is not configured and the server_mlrun_version_specifier"
Expand All @@ -357,7 +355,7 @@ def test_resolve_mlrun_install_command():
"mlrun_version_specifier": None,
"client_version": "0.9.3",
"server_mlrun_version_specifier": "mlrun[complete]==0.10.0-server-version",
"expected_mlrun_install_command": f'{pip_command} "mlrun[complete]==0.10.0-server-version"',
"expected_mlrun_install_command_version": "mlrun[complete]==0.10.0-server-version",
},
{
"test_description": "when client_version is specified and stable and mlrun_version_specifier and"
Expand All @@ -366,7 +364,7 @@ def test_resolve_mlrun_install_command():
"mlrun_version_specifier": None,
"client_version": "0.9.3",
"server_mlrun_version_specifier": None,
"expected_mlrun_install_command": f'{pip_command} "mlrun[complete]==0.9.3"',
"expected_mlrun_install_command_version": "mlrun[complete]==0.9.3",
},
{
"test_description": "when client_version is specified and unstable and mlrun_version_specifier and"
Expand All @@ -375,8 +373,8 @@ def test_resolve_mlrun_install_command():
"mlrun_version_specifier": None,
"client_version": "unstable",
"server_mlrun_version_specifier": None,
"expected_mlrun_install_command": f'{pip_command} "mlrun[complete] @ git+'
f'https://github.com/mlrun/mlrun@development"',
"expected_mlrun_install_command_version": "mlrun[complete] @ "
"git+https://github.com/mlrun/mlrun@development",
},
{
"test_description": "when only the config.version is configured and unstable,"
Expand All @@ -385,8 +383,8 @@ def test_resolve_mlrun_install_command():
"client_version": None,
"server_mlrun_version_specifier": None,
"version": "unstable",
"expected_mlrun_install_command": f'{pip_command} "mlrun[complete] @ git+'
f'https://github.com/mlrun/mlrun@development"',
"expected_mlrun_install_command_version": "mlrun[complete] @ "
"git+https://github.com/mlrun/mlrun@development",
},
{
"test_description": "when only the config.version is configured and stable,"
Expand All @@ -395,7 +393,7 @@ def test_resolve_mlrun_install_command():
"client_version": None,
"server_mlrun_version_specifier": None,
"version": "0.9.2",
"expected_mlrun_install_command": f'{pip_command} "mlrun[complete]==0.9.2"',
"expected_mlrun_install_command_version": "mlrun[complete]==0.9.2",
},
]
for case in cases:
Expand All @@ -410,9 +408,9 @@ def test_resolve_mlrun_install_command():

mlrun_version_specifier = case.get("mlrun_version_specifier")
client_version = case.get("client_version")
expected_result = case.get("expected_mlrun_install_command")
expected_result = case.get("expected_mlrun_install_command_version")

result = mlrun.api.utils.builder.resolve_mlrun_install_command(
result = mlrun.api.utils.builder.resolve_mlrun_install_command_version(
mlrun_version_specifier, client_version
)
assert (
Expand Down

0 comments on commit 0ee3090

Please sign in to comment.