Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Runtimes] Support configuring auto mount type #1191

Merged
merged 16 commits into from
Aug 11, 2021

Conversation

theSaarco
Copy link
Member

@theSaarco theSaarco commented Aug 8, 2021

Support the requirements in https://jira.iguazeng.com/browse/ML-867 - perform auto-mount on runtimes, saving the user the need to use func.apply(auto_mount()) for every function deployed remotely (and actually not using auto_mount as it will default to fuse mount on every function, which is usually not needed).
Two configuration parameters were added:

  • storage.auto_mount_type - based on the values in the AutoMountType enum. By default this is auto, and will also be the case if the parameter is empty. Possible values are:
    • none - self explanatory
    • v3io_credentials - add v3io credentials to the deployed runtime (applies v3io_cred)
    • v3io_fuse - mount v3io fuse driver (applies mount_v3io)
    • pvc - configure PVC parameters (applies mount_pvc)
    • auto - for Iguazio system will use v3io_credentials, else will be mount_pvc if pvc is configured or none otherwise
  • storage.auto_mount_params - a string containing kwargs to be passed to the mount function. The exact kwargs to use depend on the type of the auto mount in use and the function it invokes. The format of the string is pairs of values separated by commas: "param1=value1,param2=value2"

Auto mount is activated on func.run, except for Nuclio (RemoteRuntime) where it's applied in func.deploy. Auto mount is always performed on the client side. It is also applied for workflows, immediately after running the user's init_functions code (if exists).
Auto mount is disabled if someone called apply on the runtime with any other modifier. Of course, this also means that if auto-mount was already applied, it won't be applied again.

There are 2 other work-items which are related and are not part of this PR:

  1. Documentation fixes - given that current docs always apply auto-mount to functions prior to their deployment, this will have no effect on the existing docs/demos. However, the docs need to reflect this change.
  2. Properly configure these parameters when deploying MLRun kit - ideally the auto-mount parameters automatically point at the NFS mount. Possibly modify the Helm chart so that the user can control that as part of the values passed to it.

mlrun/config.py Outdated Show resolved Hide resolved
mlrun/runtimes/base.py Show resolved Hide resolved
mlrun/runtimes/pod.py Outdated Show resolved Hide resolved
mlrun/runtimes/pod.py Outdated Show resolved Hide resolved
mlrun/runtimes/pod.py Outdated Show resolved Hide resolved
mlrun/config.py Outdated Show resolved Hide resolved
mlrun/runtimes/utils.py Outdated Show resolved Hide resolved
@Hedingber Hedingber changed the title [Usability] Support auto mount for MLRun runtimes [Runtimes] Support configuring auto mount type Aug 8, 2021
Copy link
Contributor

@Hedingber Hedingber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also reminding ourselves to talk about the mount_pvc thing in the auto mode

mlrun/config.py Outdated
@@ -242,6 +242,14 @@
"channel": "master",
},
},
"storage": {
# What type of auto-mount to use for functions. Can be one of: none, auto, v3io_credentials, v3io_fuse, pvc.
# Default is auto - which is v3io_credentials when running on Iguazio and pvc otherwise (MLRun kit)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it should be else pvc if X is configured (need to think what is that X) else None

mlrun/config.py Outdated Show resolved Hide resolved
mlrun/runtimes/base.py Outdated Show resolved Hide resolved
tests/api/conftest.py Outdated Show resolved Hide resolved
tests/api/runtimes/client_side/test_auto_mount.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Hedingber Hedingber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just please update the comment near the auto_mount_type in config.py and I'll merge

@Hedingber Hedingber merged commit 36bc5ce into development Aug 11, 2021
@Hedingber Hedingber deleted the function_auto_mount branch August 11, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants