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

[feature] default to the emissary executor #5718

Closed
10 tasks done
juliusvonkohout opened this issue May 21, 2021 · 25 comments · Fixed by #5926
Closed
10 tasks done

[feature] default to the emissary executor #5718

juliusvonkohout opened this issue May 21, 2021 · 25 comments · Fixed by #5926

Comments

@juliusvonkohout
Copy link
Member

juliusvonkohout commented May 21, 2021

Update - 2021.9.13

edited by @Bobgy

KFP 1.7.0 is released.
documentation is live: https://www.kubeflow.org/docs/components/pipelines/installation/choose-executor/
choosing an executor will be an option starting from KFP 1.7.

Update - 2021.8.6

edited by @Bobgy

Discussed with @kramachandran and decided to delay the default change for at least one KFP minor version, because argo emissary executor is still in Alpha state. There can be other corner cases that we haven't found, so we should give people some time to test it out.

Therefore, I'd like to propose the following:

  • still default to docker executor in 1.7.0 release
  • provide emissary executor as an option and document how to migrate
  • collect feedback from people who starts to try emissary executor

TODOs:

Issues discovered when testing argo v3.1.0 with KFP:

=== the following are the original proposal ===

What feature would you like to see?

Update argo to 3.11 such that we can use the emissary executor as decided by @Bobgy in #4645 (comment)

What is the use case or pain point?

The Docker, Kubelet, PNS and K8sapi executors have severe limitations that will be solved by Argo 3.1 and the emissary executor.

Here is an overview https://argoproj.github.io/argo-workflows/workflow-executors/

In a nutshell docker breaks security completely and is incompatible with kubernetes 1.19
#5714 Kubernetes is also moving to containerd anyway and a lot of users want proper non-docker support for a long time #1654

Kubelet needs some configuration and has the same limitation as k8sapi and rootless PNS: "Output artifacts must be saved on volumes (e.g. emptyDir) and not the base image layer (e.g. /tmp)" so it breaks the leightweight python components if there is no volume mounted or the output directory is inside the base layer #4645

One can use a PNS with root rights (PTRACE and CHROOT) to circumvent this, but this obviously breaks the security again.

So only the emissary executor https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary is feasible which does more ore less what i proposed in #4645 and what was started by Argos @alexec in argoproj/argo-workflows#4766

The Argo update has been done before in #5266 and #4693 by @xinbinhuang @Bobgy

Is there a workaround currently?

# Emmissary executor, needs argo 3.1+
# Sadly visualizations are broken(26.04.2021) for k8sapi and emissary

kubectl -n kubeflow patch configmap/workflow-controller-configmap --type='json' -p='[{"op": "replace", "path": "/data/containerRuntimeExecutor", "value": "emissary"}]'

kubectl edit deployment/workflow-controller -n kubeflow
...
    spec:
      containers:
      - env:
        - name: "LEADER_ELECTION_DISABLE"
       	  value: true
      - args:
	- --configmap
        - workflow-controller-configmap
        - --executor-image
        - docker.io/argoproj/argoexec:latest
        command:
        - workflow-controller
        image: docker.io/argoproj/workflow-controller:latest    
        env:
	- name: "LEADER_ELECTION_DISABLE"
          value: "true"
...
kubectl -n kubeflow patch configmap/workflow-controller-configmap --type='json' -p='[{"op": "replace", "path": "/data/containerRuntimeExecutor", "value": "emissary"}]'
kubectl rollout restart deployment workflow-controller -n kubeflow

or manually patching the sdk and using k8sapi as done in https://github.com/kubeflow/pipelines/pull/4645

Or use kfp-tekton

Love this idea? Give it a 👍. We prioritize fulfilling features with the most 👍.

@Bobgy
Copy link
Contributor

Bobgy commented May 21, 2021

@juliusvonkohout can you clarify how visualization is broken?

Sadly visualizations are broken(26.04.2021) for k8sapi and emissary

@juliusvonkohout
Copy link
Member Author

juliusvonkohout commented May 21, 2021

@juliusvonkohout can you clarify how visualization is broken?

Sadly visualizations are broken(26.04.2021) for k8sapi and emissary

The pipeline finishes, but e.g. no html or markdown visualizations.

It just does not show in the UI. As far as I remember the stuff was in minio but not properly displayed by pipeline-ui. But I would have to check again. Such problems are expected, since I just replaced the Argo images instead of properly updating Argo everywhere in kubeflow pipelines. Since it appeared with emissary AND k8sapi it should be fine with a proper Argo update in kubeflow.

If you need more information you can obtain them with my instructions above or tell me what exactly you want and I will check it on Tuesday.

@Bobgy
Copy link
Contributor

Bobgy commented May 21, 2021

/assign
I believe there is no doubt this is the right way to go.

@NikeNano
Copy link
Member

I can help out updating argo to v3 @Bobgy
/assign

@Ark-kun
Copy link
Contributor

Ark-kun commented May 22, 2021

"Output artifacts must be saved on volumes (e.g. emptyDir) and not the base image layer (e.g. /tmp)" so it breaks the leightweight python components if there is no volume mounted or the output directory is inside the base layer #4645

Each output is placed in it's own directory. And those directories are in /tmp/outputs/, not just in /tmp/. Couldn't Argo mount EmptyDir volumes under the outputs directories? Argo already does that for inputs.

P.S. The SDK could change the paths to /outputs/ without a need to change components or pipelines. It was like that at some point in the past.

@juliusvonkohout
Copy link
Member Author

@Ark-kun this has all been discussed extensively in the links provided above. You will find detailed answers there.

@Bobgy Bobgy added the size/L label May 24, 2021
@juliusvonkohout
Copy link
Member Author

The release of 3.1 is very close. 3.1.0-rc8 is already there https://github.com/argoproj/argo-workflows/releases and 3.0-rc9 https://github.com/argoproj/argo-workflows/releases/tag/v3.0.0-rc9 was the last prerelease for the 3.0 branch. Based on that 3.1.0 should land in 1-2 weeks.

@NikeNano
Copy link
Member

Have started the work to upgrade should be easy to go to the next version when it is out.

@Bobgy
Copy link
Contributor

Bobgy commented Jun 8, 2021

TODO: I'll review argo v3 images for licenses and upgrade the manifests.

google-oss-robot pushed a commit that referenced this issue Jun 8, 2021
* updating argo

* updated fix deps and code

* clean up
@juliusvonkohout
Copy link
Member Author

juliusvonkohout commented Jun 14, 2021

I think we should add a test for a simple pipeline that runs as non-root. That way we know that the emissary executor works properly.
The following tests runasnonroot and kubeflow profile quotas.

python
from kfp.components import create_component_from_func
from typing import NamedTuple

def pod_defaults(op, user = 1000):
    from kubernetes import client as k8s_client
    from kubernetes.client.models import V1SecurityContext
    op.set_security_context(V1SecurityContext(run_as_user=user, run_as_group=0))
    op.set_memory_request('10Mi');# op.set_memory_limit('1000Mi')
    op.set_cpu_request('10m');# op.set_cpu_limit('1000m')
    return op

def produce_markdown() -> NamedTuple('Outputs', [('mlpipeline_ui_metadata', 'UI_metadata'), ('mlpipeline_metrics', 'Metrics')]):
    #https://www.kubeflow.org/docs/components/pipelines/sdk/output-viewer/
    metadata = {"outputs": [
        {"storage": "inline", "source": "*this should be bold*", "type": "markdown"}
    ]}
    # https://www.kubeflow.org/docs/components/pipelines/sdk/pipelines-metrics/
    metrics = {'metrics': [
        {
        'name': 'train-accuracy', # underscores are not allowed... https://github.com/kubeflow/pipelines/issues/3809
        'numberValue':  0.9,
        },
        {
        'name': 'test-accuracy', # underscores are not allowed...
        'numberValue':  0.7,
        }
    ]}
    #return [json.dumps(markdown)]
    from collections import namedtuple; import json
    return namedtuple('output', ['mlpipeline_ui_metadata', 'mlpipeline_metrics'])(json.dumps(metadata), json.dumps(metrics))

...

markdown_component = func_to_container_op(func=produce_markdown, base_image=BASE_IMAGE)()
pod_defaults(markdown_component)

But you have to use PSPs or consider https://argoproj.github.io/argo-workflows/workflow-pod-security-context/ and use a kubeflow profile with quotas.

@Bobgy
Copy link
Contributor

Bobgy commented Jun 28, 2021

Note, there's a breaking change. When using emissary executor, your components must specify the command explicitly.

step group deemed errored due to child integration-test-vzl9f[0].build-initialization-test-image error: when using the emissary executor you must either explicitly specify the command, or list the image's command in the index: https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary
└───⚠ build-initialization-test-image(0) build-image integration-test-vzl9f-2173864179 0s when using the emissary executor you must either explicitly specify the command, or list the image's command in the index: https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary

@Bobgy
Copy link
Contributor

Bobgy commented Jun 28, 2021

The breaking changes is also required for bit.ly/kfp-v2-compatible and bit.ly/kfp-v2. So I think we can introduce it now.

See https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary

@Bobgy
Copy link
Contributor

Bobgy commented Jun 28, 2021

user facing documentation candidate

Breaking change

Argo emissary executor requires that each container has explicit command and args defined, refer to https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary.

Why we decide to adopt the breaking change? We'd have the same breaking change in bit.ly/kfp-v2-compatible and bit.ly/kfp-v2. So I think it aligns with KFP's long term goal, and it's OK to introduce the breaking change now.

google-oss-robot pushed a commit that referenced this issue Jul 22, 2021
* feat: use argo v3.1.2-patch

* upgrade to argo v3.1.2-patch.2

* update versions to v3.1.2
@Bobgy
Copy link
Contributor

Bobgy commented Aug 4, 2021

Discussed with @kramachandran and decided to delay the default change for at least one KFP minor version, because argo emissary executor is still in Alpha state, despite we verified it in our test infra. There can be other corner cases too, so we should give people some time to test it out.

Action Items:

  • document how to use docker executor in GKE by configuring nodepools to use OS images with docker
  • document how to switch to emissary executor
  • document the breaking changes made in emissary executor

@Bobgy Bobgy reopened this Aug 4, 2021
@Bobgy Bobgy changed the title [feature] update to Argo 3.1 and default to the emissary executor [feature] default to the emissary executor Aug 4, 2021
@Bobgy
Copy link
Contributor

Bobgy commented Aug 4, 2021

Renaming to just default to emissary executor, because we've finished the upgrade.

Bobgy added a commit that referenced this issue Aug 4, 2021
@Bobgy Bobgy reopened this Aug 4, 2021
google-oss-robot pushed a commit to kubeflow/website that referenced this issue Aug 11, 2021
#2857)

* docs(kfp): choose a workflow executor

* clean up

* address feedback

* add deprecation pointers from different pages
@sharon-codefresh
Copy link

Is there any estimation of when the emissary is going to be the default executed?

@juliusvonkohout
Copy link
Member Author

Is there any estimation of when the emissary is going to be the default executed?

I think there are some small runasbonnroot bugs for V2 pipelines, but V1 and V2-Compatible should be fine. Feel free to help there... So I hope that they change the default to emissary in 1.9. (kubeflow 1.5 most likely) . I bet that @Bobgy wants to update argo argo to >= 3.2.2 to get the most mature emissary executor. There is also an emissary feedback issue somewhere here where you can ask.

google-oss-prow bot pushed a commit that referenced this issue Jan 10, 2022
#5718 (#7137)

* feat!: Use Argo Emissary Executor instead of docker by default.

* marketplace presubmit
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Mar 2, 2022
@Bobgy
Copy link
Contributor

Bobgy commented Mar 2, 2022

Fixed by #7137

@stale stale bot removed the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Mar 2, 2022
@Bobgy Bobgy closed this as completed Mar 2, 2022
abaland pushed a commit to abaland/pipelines that referenced this issue May 29, 2022
kubeflow#5718 (kubeflow#7137)

* feat!: Use Argo Emissary Executor instead of docker by default.

* marketplace presubmit
@kotalakshman
Copy link

when using the emissary executor you must either explicitly specify the command, or list the image's command in the index: https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary

kubeflow version:1.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants