Skip to content
This repository has been archived by the owner on Mar 11, 2022. It is now read-only.

ODH crio fix #56

Merged
merged 1 commit into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions etc/docker-scripts/bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ def execute(self) -> None:
try:
OpUtil.log_operation_info(f"executing notebook using 'papermill {notebook} {notebook_output}'")
t0 = time.time()
subprocess.run(['papermill', notebook, notebook_output], check=True)
# Really hate to do this but have to invoke Papermill via library as workaround
import papermill
papermill.execute_notebook(notebook, notebook_output)
duration = time.time() - t0
OpUtil.log_operation_info("notebook execution completed", duration)

Expand Down Expand Up @@ -269,7 +271,6 @@ class OpUtil(object):
"""Utility functions for preparing file execution."""
@classmethod
def package_install(cls, user_volume_path) -> None:

OpUtil.log_operation_info("Installing packages")
t0 = time.time()
elyra_packages = cls.package_list_to_dict("requirements-elyra.txt")
Expand Down Expand Up @@ -298,7 +299,12 @@ def package_install(cls, user_volume_path) -> None:
if to_install_list:
if user_volume_path:
to_install_list.insert(0, '--target=' + user_volume_path)
subprocess.run([sys.executable, '-m', 'pip', 'install'] + to_install_list)
to_install_list.append('--no-cache-dir')

subprocess.run([sys.executable, '-m', 'pip', 'install'] + to_install_list, check=True)

if user_volume_path:
os.environ["PIP_CONFIG_FILE"] = user_volume_path + "/pip.conf"

subprocess.run([sys.executable, '-m', 'pip', 'freeze'])
duration = time.time() - t0
Expand Down
2 changes: 2 additions & 0 deletions etc/pip.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[global]
target=/opt/app-root/src/jupyter-work-dir/python3/
4 changes: 2 additions & 2 deletions etc/tests/test_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import pytest
import mock
import sys
import subprocess
import papermill
import minio
import os

Expand Down Expand Up @@ -220,7 +220,7 @@ def test_fail_bad_notebook_main_method(monkeypatch, s3_setup, tmpdir):
file_path="etc/tests/resources/test-bad-archiveB.tgz")

with tmpdir.as_cwd():
with pytest.raises(subprocess.CalledProcessError):
with pytest.raises(papermill.exceptions.PapermillExecutionError):
bootstrapper.main()


Expand Down
44 changes: 31 additions & 13 deletions kfp_notebook/pipeline/_notebook_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ def __init__(self,
cos_dependencies_archive: archive file name to get from object storage bucket e.g archive1.tar.gz
pipeline_outputs: comma delimited list of files produced by the notebook
pipeline_inputs: comma delimited list of files to be consumed/are required by the notebook
pipeline_envs: dictionary of environmental variables to set in the container prior to execution
requirements_url: URL to a python requirements.txt file to be installed prior to running the notebook
bootstrap_script_url: URL to a custom python bootstrap script to run
emptydir_volume_size: Size(GB) of the volume to create for the workspace when using CRIO container runtime
kwargs: additional key value pairs to pass e.g. name, image, sidecars & is_exit_handler.
See Kubeflow pipelines ContainerOp definition for more parameters or how to use
https://kubeflow-pipelines.readthedocs.io/en/latest/source/kfp.dsl.html#kfp.dsl.ContainerOp
Expand All @@ -75,7 +77,7 @@ def __init__(self,
self.cos_directory = cos_directory
self.cos_dependencies_archive = cos_dependencies_archive
self.container_work_dir_root_path = "./"
self.container_work_dir_name = "jupyter-work-dir"
self.container_work_dir_name = "jupyter-work-dir/"
akchinSTC marked this conversation as resolved.
Show resolved Hide resolved
self.container_work_dir = self.container_work_dir_root_path + self.container_work_dir_name
self.bootstrap_script_url = bootstrap_script_url
self.requirements_url = requirements_url
Expand All @@ -85,19 +87,25 @@ def __init__(self,

argument_list = []

# CRI-o support for kfp pipelines
# We need to attach an emptydir volume for each notebook that runs since CRI-o runtime does not allow
# us to write to the base image layer file system, only to volumes.
""" CRI-o support for kfp pipelines
We need to attach an emptydir volume for each notebook that runs since CRI-o runtime does not allow
us to write to the base image layer file system, only to volumes.
"""
self.emptydir_volume_name = "workspace"
self.emptydir_volume_size = emptydir_volume_size
self.python_user_lib_path = ''
self.python_user_lib_path_target = ''
self.python_pip_config_url = ''

if self.emptydir_volume_size:
self.container_work_dir_root_path = "/mnt/"
self.container_work_dir_root_path = "/opt/app-root/src/"
self.container_python_dir_name = "python3/"
self.container_work_dir = self.container_work_dir_root_path + self.container_work_dir_name
self.python_user_lib_path = self.container_work_dir + '/python3.6/'
self.python_user_lib_path = self.container_work_dir + self.container_python_dir_name
self.python_user_lib_path_target = '--target=' + self.python_user_lib_path
self.python_pip_config_url = 'https://raw.githubusercontent.com/{org}/' \
'kfp-notebook/{branch}/etc/pip.conf'. \
format(org=KFP_NOTEBOOK_ORG, branch=KFP_NOTEBOOK_BRANCH)

if not self.bootstrap_script_url:
self.bootstrap_script_url = 'https://raw.githubusercontent.com/{org}/' \
Expand All @@ -124,18 +132,28 @@ def __init__(self,
argument_list.append('mkdir -p {container_work_dir} && cd {container_work_dir} && '
'curl -H "Cache-Control: no-cache" -L {bootscript_url} --output bootstrapper.py && '
'curl -H "Cache-Control: no-cache" -L {reqs_url} --output requirements-elyra.txt && '
'python3 -m pip install {python_user_lib_path_target} packaging && '
.format(container_work_dir=self.container_work_dir,
bootscript_url=self.bootstrap_script_url,
reqs_url=self.requirements_url)
)

if self.emptydir_volume_size:
argument_list.append('mkdir {container_python_dir} && cd {container_python_dir} && '
'curl -H "Cache-Control: no-cache" -L {python_pip_config_url} '
'--output pip.conf && cd .. &&'
.format(python_pip_config_url=self.python_pip_config_url,
container_python_dir=self.container_python_dir_name)
)

argument_list.append('python3 -m pip install {python_user_lib_path_target} packaging && '
'python3 -m pip freeze > requirements-current.txt && '
'python3 bootstrapper.py '
'--cos-endpoint {cos_endpoint} '
'--cos-bucket {cos_bucket} '
'--cos-directory "{cos_directory}" '
'--cos-dependencies-archive "{cos_dependencies_archive}" '
'--file "{notebook}" '
.format(container_work_dir=self.container_work_dir,
bootscript_url=self.bootstrap_script_url,
reqs_url=self.requirements_url,
cos_endpoint=self.cos_endpoint,
.format(cos_endpoint=self.cos_endpoint,
cos_bucket=self.cos_bucket,
cos_directory=self.cos_directory,
cos_dependencies_archive=self.cos_dependencies_archive,
Expand Down Expand Up @@ -173,12 +191,12 @@ def __init__(self,
size_limit=self.emptydir_volume_size),
name=self.emptydir_volume_name))

self.container.add_volume_mount(V1VolumeMount(mount_path=self.container_work_dir,
self.container.add_volume_mount(V1VolumeMount(mount_path=self.container_work_dir_root_path,
name=self.emptydir_volume_name))

# Append to PYTHONPATH location of elyra dependencies in installed in Volume
self.container.add_env_variable(V1EnvVar(name='PYTHONPATH',
value=self.python_user_lib_path + ':$PYTHONPATH'))
value=self.python_user_lib_path))

def _get_file_name_with_extension(self, name, extension):
"""
Expand Down
10 changes: 6 additions & 4 deletions kfp_notebook/tests/test_notebook_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def test_fail_with_empty_string_as_notebook():
assert "You need to provide a notebook." == str(error_info.value)


def test_user_volume_size():
def test_user_crio_volume_creation():
notebook_op = NotebookOp(name="test",
notebook="test_notebook.ipynb",
cos_endpoint="http://testserver:32525",
Expand All @@ -132,13 +132,15 @@ def test_user_volume_size():
image="test/image:dev",
emptydir_volume_size='20Gi')
assert notebook_op.emptydir_volume_size == '20Gi'
assert notebook_op.container_work_dir_root_path == '/mnt/'
assert notebook_op.container_work_dir_root_path == '/opt/app-root/src/'
assert notebook_op.container.volume_mounts.__len__() == 1
assert notebook_op.container.env.__len__() == 1


@pytest.mark.skip(reason="not sure if we should even test this")
def test_default_bootstrap_url(notebook_op):
assert notebook_op.bootstrap_script_url == \
'https://raw.githubusercontent.com/elyra-ai/kfp-notebook/v0.9.1/etc/docker-scripts/bootstrapper.py'
'https://raw.githubusercontent.com/elyra-ai/kfp-notebook/v0.9.1/etc/docker-scripts/bootstrapper.py'


def test_override_bootstrap_url():
Expand All @@ -156,7 +158,7 @@ def test_override_bootstrap_url():
@pytest.mark.skip(reason="not sure if we should even test this")
def test_default_requirements_url(notebook_op):
assert notebook_op.requirements_url == \
'https://raw.githubusercontent.com/elyra-ai/kfp-notebook/v0.9.1/etc/requirements-elyra.txt'
'https://raw.githubusercontent.com/elyra-ai/kfp-notebook/v0.9.1/etc/requirements-elyra.txt'


def test_override_requirements_url():
Expand Down