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

[Core feature] Injection of env vars at run time [discussion needed] #2447

Closed
2 tasks done
wild-endeavor opened this issue May 4, 2022 · 6 comments
Closed
2 tasks done
Assignees
Labels
enhancement New feature or request needs discussion

Comments

@wild-endeavor
Copy link
Contributor

Motivation: Why do you think this is important?

A user requested that Flyte be able to inject environment variables at execution launch time to a workflow run that would get propagated into all downstream tasks, subworkflows, and launch plans.

I think this feature is possibly reasonable, but warrants discussion. The reason is that this breaks referential transparency kinda. The injected environment variable are designed to alter the behavior of the task. However. the idea is that these env vars will not affect caching at all, so that is a bit strange.

The original use-case for this would be to direct service queries. One may run the same task against production services (non-Flyte stuff) or development services for example.

Goal: What should the final outcome look like, ideally?

Not entirely clear, but basically at all the points where users can run workflows (flytectl, console, flyte-cli, flyteremote, etc.) the user would be able to pass along a dict that would get set at the top execution level, and this propagate through all nodes/subexecutions.

Describe alternatives you've considered

It feels more natural to use either a task's domain or the execution's domain to alter behavior, but perhaps not.

Another alternative is to add an explicit service environment variable to all the tasks but this is cumbersome and inelegant.

Propose: Link/Inline OR Additional context

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@wild-endeavor wild-endeavor added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers needs discussion labels May 4, 2022
@wild-endeavor wild-endeavor changed the title [Core feature] [wip] Injection of env vars at run time. [Core feature] Injection of env vars at run time [discussion needed] May 4, 2022
@wild-endeavor
Copy link
Contributor Author

From Gigi on channel:

Our use case is a bit different than Dylan's in that our configured domains match our external service ecosystem environments, so we generally want workflows in the "development" domain to use the "development" service ecosystem and matching service urls.
However we have a bunch of commonly used external services within various projects and workflows. We would love a way to have something like a list of "development" service urls that could get injected for workflows in the development domain, for a bunch of projects/workflows.
Currently every project and workflow keys off the internal env var to get the domain, and has a hard coded map of service urls for that domain ie ZAUTH_UTL, ZREST_URL etc. Mostly it's a lot of repeated boilerplate config that different project teams are maintaining. We could put it in a Python lib to DRY it up a bit, but it's the kind of thing we often put in configmaps in our other k8s ecosystems

@mucahitkantepe
Copy link
Contributor

We also need this feature to decide the environment(dev/staging/prod) at runtime so that our application can determine what configuration file to load for example. Most people use flyte domain to decide the environment but we don't use domains for the environment as we have isolated clusters across environments. As a workaround, we use Docker arg/env but this forces us to build multiple

It would be great if we can pass these properties via flytekit or flytectl along with the packaging/registering

@wild-endeavor
Copy link
Contributor Author

this is not directly related to this thread but there was a request from a user on how to support the injection of different environment variables by domain (that do not change from one workflow execution to another). this can now be achieved through pod templates and the pod template fallback logic in flyte propeller.

Bring up the demo environment with flytectl demo start
After it’s ready, place a file mypodtemplate.yaml in ~/.flyte/sandbox/ that contains

apiVersion: v1
kind: PodTemplate
metadata:
  name: flyte-template
  namespace: '{{ namespace }}'
template:
  metadata:
    annotations:
      bar: custom-value
  spec:
    containers:
      - name: default
        image: docker.io/rwgrim/docker-noop
        terminationMessagePath: "/dev/foo"
        env:
        - name: DOMAIN_BASED_VALUE
          value: '{{ mydomainvalue }}'

Then, place this in ~/.flyte/sandbox/config.yaml

plugins:
  k8s:
    default-pod-template-name: "flyte-template"
cluster_resources:
  customData:
  - production:
    - mydomainvalue:
        value: "iminprod"
  - staging:
    - mydomainvalue:
        value: "iminstg"
  - development:
    - mydomainvalue:
        value: "imindev"

And if you want you can also make a default flyte namespace pod template

apiVersion: v1
kind: PodTemplate
metadata:
  name: flyte-template
  namespace: flyte
template:
  metadata:
    labels:
      foo: foolabel
    annotations:
      foo: initial-value
      bar: initial-value
  spec:
    containers:
      - name: default
        image: docker.io/rwgrim/docker-noop
        terminationMessagePath: "/dev/foo"

@kumare3
Copy link
Contributor

kumare3 commented Mar 31, 2023

I think the original issue, still stands where a user may want to set env vars for containers at runtime.

pyflyte run --env "k=v" --env "k=v" ...

or in flytectl

This can help users set env vars. For non compute plugins, flyte will not be able to set this env vars, as not all plugins support this. But that is ok, will be a per plugin decision

I am thinking, of starting to work on this

@kumare3
Copy link
Contributor

kumare3 commented May 21, 2023

Think this is ready to close?

@pingsutw
Copy link
Member

yes, it's done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion
Projects
None yet
Development

No branches or pull requests

4 participants