diff --git a/kfp_notebook/pipeline/_notebook_op.py b/kfp_notebook/pipeline/_notebook_op.py index cb923b2..bbec2a9 100644 --- a/kfp_notebook/pipeline/_notebook_op.py +++ b/kfp_notebook/pipeline/_notebook_op.py @@ -16,6 +16,7 @@ # import os +import string from kfp.dsl import ContainerOp from kfp_notebook import __version__ @@ -52,23 +53,28 @@ class NotebookOp(ContainerOp): def __init__(self, + pipeline_name: str, + experiment_name: str, notebook: str, cos_endpoint: str, cos_bucket: str, cos_directory: str, cos_dependencies_archive: str, + pipeline_version: Optional[str] = '', pipeline_outputs: Optional[List[str]] = None, pipeline_inputs: Optional[List[str]] = None, pipeline_envs: Optional[Dict[str, str]] = None, - requirements_url: str = None, - bootstrap_script_url: str = None, - emptydir_volume_size: str = None, - cpu_request: str = None, - mem_request: str = None, - gpu_limit: str = None, + requirements_url: Optional[str] = None, + bootstrap_script_url: Optional[str] = None, + emptydir_volume_size: Optional[str] = None, + cpu_request: Optional[str] = None, + mem_request: Optional[str] = None, + gpu_limit: Optional[str] = None, **kwargs): """Create a new instance of ContainerOp. Args: + pipeline_name: pipeline that this op belongs to + experiment_name: the experiment where pipeline_name is executed notebook: name of the notebook that will be executed per this operation cos_endpoint: object storage endpoint e.g weaikish1.fyre.ibm.com:30442 cos_bucket: bucket to retrieve archive from @@ -88,6 +94,9 @@ def __init__(self, https://kubeflow-pipelines.readthedocs.io/en/latest/source/kfp.dsl.html#kfp.dsl.ContainerOp """ + self.pipeline_name = pipeline_name + self.pipeline_version = pipeline_version + self.experiment_name = experiment_name self.notebook = notebook self.notebook_name = os.path.basename(notebook) self.cos_endpoint = cos_endpoint @@ -228,6 +237,27 @@ def __init__(self, gpu_vendor = self.pipeline_envs.get('GPU_VENDOR', 'nvidia') self.container.set_gpu_limit(gpu=str(gpu_limit), vendor=gpu_vendor) + # Attach metadata to the pod + # Node type (a static type for this op) + self.add_pod_label('elyra/node-type', + NotebookOp._normalize_label_value( + 'notebook-script')) + # Pipeline name + self.add_pod_label('elyra/pipeline-name', + NotebookOp._normalize_label_value(self.pipeline_name)) + # Pipeline version + self.add_pod_label('elyra/pipeline-version', + NotebookOp._normalize_label_value(self.pipeline_version)) + # Experiment name + self.add_pod_label('elyra/experiment-name', + NotebookOp._normalize_label_value(self.experiment_name)) + # Pipeline node name + self.add_pod_label('elyra/node-name', + NotebookOp._normalize_label_value(kwargs.get('name'))) + # Pipeline node file + self.add_pod_annotation('elyra/node-file-name', + self.notebook) + def _artifact_list_to_str(self, pipeline_array): trimmed_artifact_list = [] for artifact_name in pipeline_array: @@ -236,3 +266,61 @@ def _artifact_list_to_str(self, pipeline_array): ValueError("Illegal character ({}) found in filename '{}'.".format(INOUT_SEPARATOR, artifact_name)) trimmed_artifact_list.append(artifact_name.strip()) return INOUT_SEPARATOR.join(trimmed_artifact_list) + + @staticmethod + def _normalize_label_value(value): + + """Produce a Kubernetes-compliant label from value + + Valid label values must be 63 characters or less and + must be empty or begin and end with an alphanumeric + character ([a-z0-9A-Z]) with dashes (-), underscores + (_), dots (.), and alphanumerics between. + """ + + if value is None or len(value) == 0: + return '' # nothing to do + + max_length = 63 + # This char is added at the front and/or back + # of value, if the first and/or last character + # is invalid. For example a value of "-abc" + # is converted to "a-abc". The specified character + # must meet the label value constraints. + valid_char = 'a' + # This char is used to replace invalid characters + # that are in the "middle" of value. For example + # a value of "abc%def" is converted to "abc_def". + # The specified character must meet the label value + # constraints. + valid_middle_char = '_' + + # must begin with [0-9a-zA-Z] + valid_chars = string.ascii_letters + string.digits + if value[0] not in valid_chars: + value = valid_char + value + + value = value[:max_length] # enforce max length + + # must end with [0-9a-zA-Z] + if value[-1] not in valid_chars: + if len(value) <= max_length - 1: + # append valid character if max length + # would not be exceeded + value = value + valid_char + else: + # replace with valid character + value = value[:-1] + valid_char + + # middle chars must be [0-9a-zA-Z\-_.] + valid_chars = valid_chars + '-_.' + + newstr = '' + for c in range(len(value)): + if value[c] not in valid_chars: + newstr = newstr + valid_middle_char + else: + newstr = newstr + value[c] + value = newstr + + return value diff --git a/kfp_notebook/tests/test_notebook_op.py b/kfp_notebook/tests/test_notebook_op.py index 45139e6..9476a71 100644 --- a/kfp_notebook/tests/test_notebook_op.py +++ b/kfp_notebook/tests/test_notebook_op.py @@ -13,13 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import pytest from kfp_notebook.pipeline import NotebookOp +import pytest +import string @pytest.fixture def notebook_op(): return NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -31,6 +34,8 @@ def notebook_op(): def test_fail_without_cos_endpoint(): with pytest.raises(TypeError): NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_bucket="test_bucket", cos_directory="test_directory", @@ -41,6 +46,8 @@ def test_fail_without_cos_endpoint(): def test_fail_without_cos_bucket(): with pytest.raises(TypeError): NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_directory="test_directory", @@ -51,6 +58,8 @@ def test_fail_without_cos_bucket(): def test_fail_without_cos_directory(): with pytest.raises(TypeError): NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -61,6 +70,8 @@ def test_fail_without_cos_directory(): def test_fail_without_cos_dependencies_archive(): with pytest.raises(TypeError): NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -71,6 +82,8 @@ def test_fail_without_cos_dependencies_archive(): def test_fail_without_runtime_image(): with pytest.raises(ValueError) as error_info: NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -82,6 +95,8 @@ def test_fail_without_runtime_image(): def test_fail_without_notebook(): with pytest.raises(TypeError): NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", cos_directory="test_directory", @@ -91,7 +106,9 @@ def test_fail_without_notebook(): def test_fail_without_name(): with pytest.raises(TypeError): - NotebookOp(notebook="test_notebook.ipynb", + NotebookOp(pipeline_name="test-pipeline", + experiment_name="experiment-name", + notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", cos_directory="test_directory", @@ -102,6 +119,8 @@ def test_fail_without_name(): def test_fail_with_empty_string_as_name(): with pytest.raises(ValueError): NotebookOp(name="", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -113,6 +132,8 @@ def test_fail_with_empty_string_as_name(): def test_fail_with_empty_string_as_notebook(): with pytest.raises(ValueError) as error_info: NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -122,8 +143,34 @@ def test_fail_with_empty_string_as_notebook(): assert "You need to provide a notebook." == str(error_info.value) +def test_fail_without_pipeline_name(): + with pytest.raises(TypeError): + NotebookOp(name="test", + experiment_name="experiment-name", + notebook="test_notebook.ipynb", + cos_endpoint="http://testserver:32525", + cos_bucket="test_bucket", + cos_directory="test_directory", + cos_dependencies_archive="test_archive.tgz", + image="test/image:dev") + + +def test_fail_without_experiment_name(): + with pytest.raises(TypeError): + NotebookOp(name="test", + pipeline_name="test-pipeline", + notebook="test_notebook.ipynb", + cos_endpoint="http://testserver:32525", + cos_bucket="test_bucket", + cos_directory="test_directory", + cos_dependencies_archive="test_archive.tgz", + image="test/image:dev") + + def test_properly_set_notebook_name_when_in_subdirectory(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="foo/test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -135,6 +182,8 @@ def test_properly_set_notebook_name_when_in_subdirectory(): def test_properly_set_python_script_name_when_in_subdirectory(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="foo/test.py", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -146,6 +195,8 @@ def test_properly_set_python_script_name_when_in_subdirectory(): def test_user_crio_volume_creation(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -167,6 +218,8 @@ def test_default_bootstrap_url(notebook_op): def test_override_bootstrap_url(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", bootstrap_script_url="https://test.server.com/bootscript.py", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", @@ -185,6 +238,8 @@ def test_default_requirements_url(notebook_op): def test_override_requirements_url(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", requirements_url="https://test.server.com/requirements.py", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", @@ -197,6 +252,8 @@ def test_override_requirements_url(): def test_construct_with_both_pipeline_inputs_and_outputs(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -214,6 +271,8 @@ def test_construct_with_both_pipeline_inputs_and_outputs(): def test_construct_wiildcard_outputs(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -231,6 +290,8 @@ def test_construct_wiildcard_outputs(): def test_construct_with_only_pipeline_inputs(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -246,6 +307,8 @@ def test_construct_with_only_pipeline_inputs(): def test_construct_with_bad_pipeline_inputs(): with pytest.raises(ValueError) as error_info: NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -259,6 +322,8 @@ def test_construct_with_bad_pipeline_inputs(): def test_construct_with_only_pipeline_outputs(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -274,6 +339,8 @@ def test_construct_with_only_pipeline_outputs(): def test_construct_with_bad_pipeline_outputs(): with pytest.raises(ValueError) as error_info: NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -286,6 +353,8 @@ def test_construct_with_bad_pipeline_outputs(): def test_construct_with_env_variables(): notebook_op = NotebookOp(name="test", + pipeline_name="test-pipeline", + experiment_name="experiment-name", notebook="test_notebook.ipynb", cos_endpoint="http://testserver:32525", cos_bucket="test_bucket", @@ -305,3 +374,129 @@ def test_construct_with_env_variables(): # Verify confirmation values have been drained. assert len(confirmation_names) == 0 assert len(confirmation_values) == 0 + + +def test_normalize_label_value(): + valid_middle_chars = '-_.' + + # test min length + assert NotebookOp._normalize_label_value(None) == '' + assert NotebookOp._normalize_label_value('') == '' + # test max length (63) + assert NotebookOp._normalize_label_value('a' * 63) ==\ + 'a' * 63 + assert NotebookOp._normalize_label_value('a' * 64) ==\ + 'a' * 63 # truncated + # test first and last char + assert NotebookOp._normalize_label_value('1') == '1' + assert NotebookOp._normalize_label_value('22') == '22' + assert NotebookOp._normalize_label_value('3_3') == '3_3' + assert NotebookOp._normalize_label_value('4u4') == '4u4' + assert NotebookOp._normalize_label_value('5$5') == '5_5' + + # test first char + for c in string.printable: + if c in string.ascii_letters + string.digits: + # first char is valid + # no length violation + assert NotebookOp._normalize_label_value(c) == c + assert NotebookOp._normalize_label_value(c + 'B') == c + 'B' + # max length + assert NotebookOp._normalize_label_value(c + 'B' * 62) ==\ + (c + 'B' * 62) + # max length exceeded + assert NotebookOp._normalize_label_value(c + 'B' * 63) ==\ + (c + 'B' * 62) # truncated + else: + # first char is invalid, e.g. '#a', and becomes the + # second char, which might require replacement + rv = c + if c not in valid_middle_chars: + rv = '_' + # no length violation + assert NotebookOp._normalize_label_value(c) == 'a' + rv + 'a' + assert NotebookOp._normalize_label_value(c + 'B') == 'a' + rv + 'B' + # max length + assert NotebookOp._normalize_label_value(c + 'B' * 62) ==\ + ('a' + rv + 'B' * 61) # truncated + # max length exceeded + assert NotebookOp._normalize_label_value(c + 'B' * 63) ==\ + ('a' + rv + 'B' * 61) # truncated + + # test last char + for c in string.printable: + if c in string.ascii_letters + string.digits: + # no length violation + assert NotebookOp._normalize_label_value('b' + c) == 'b' + c + # max length + assert NotebookOp._normalize_label_value('b' * 62 + c) ==\ + ('b' * 62 + c) + # max length exceeded + assert NotebookOp._normalize_label_value('b' * 63 + c) ==\ + ('b' * 63) + else: + # last char is invalid, e.g. 'a#', and requires + # patching + rv = c + if c not in valid_middle_chars: + rv = '_' + # no length violation (char is appended) + assert NotebookOp._normalize_label_value('b' + c) == 'b' + rv + 'a' + # max length (char is replaced) + assert NotebookOp._normalize_label_value('b' * 62 + c) ==\ + ('b' * 62 + 'a') + # max length exceeded (no action required) + assert NotebookOp._normalize_label_value('b' * 63 + c) ==\ + ('b' * 63) + + # test first and last char + for c in string.printable: + if c in string.ascii_letters + string.digits: + # no length violation + assert NotebookOp._normalize_label_value(c + 'b' + c) ==\ + c + 'b' + c # nothing is modified + # max length + assert NotebookOp._normalize_label_value(c + 'b' * 61 + c) ==\ + (c + 'b' * 61 + c) # nothing is modified + # max length exceeded + assert NotebookOp._normalize_label_value(c + 'b' * 62 + c) ==\ + c + 'b' * 62 # truncate only + else: + # first and last characters are invalid, e.g. '#a#' + rv = c + if c not in valid_middle_chars: + rv = '_' + # no length violation + assert NotebookOp._normalize_label_value(c + 'b' + c) ==\ + 'a' + rv + 'b' + rv + 'a' + # max length + assert NotebookOp._normalize_label_value(c + 'b' * 59 + c) ==\ + ('a' + rv + 'b' * 59 + rv + 'a') + # max length exceeded after processing, scenario 1 + # resolved by adding char before first, replace last + assert NotebookOp._normalize_label_value(c + 'b' * 60 + c) ==\ + ('a' + rv + 'b' * 60 + 'a') + # max length exceeded after processing, scenario 2 + # resolved by adding char before first, appending after last + assert NotebookOp._normalize_label_value(c + 'b' * 59 + c) ==\ + ('a' + rv + 'b' * 59 + rv + 'a') + # max length exceeded before processing, scenario 1 + # resolved by adding char before first, truncating last + assert NotebookOp._normalize_label_value(c + 'b' * 62 + c) ==\ + ('a' + rv + 'b' * 61) + # max length exceeded before processing, scenario 2 + # resolved by adding char before first, replacing last + assert NotebookOp._normalize_label_value(c + 'b' * 60 + c * 3) ==\ + ('a' + rv + 'b' * 60 + 'a') + + # test char in a position other than first and last + # if invalid, the char is replaced with '_' + for c in string.printable: + if c in string.ascii_letters + string.digits + '-_.': + assert NotebookOp._normalize_label_value('A' + c + 'Z') ==\ + 'A' + c + 'Z' + else: + assert NotebookOp._normalize_label_value('A' + c + 'Z') == 'A_Z' + + # encore + assert NotebookOp._normalize_label_value(r'¯\_(ツ)_/¯') == 'a_________a'