Using environment variables to update values in worker-spec.yml

[jhamman]

In the dask configuration module, it is possible to use environment variables to update configuration values. It doesn't seem that is currently possible here as the "worker-spec.yml" file is actually a kubernetes spec, rather than a dask config file. I'm curious if there is a way to do that now or if it would be feasible to add such functionality.

somewhat related to #88, #45

Background for my particular use case: jupyterhub/kubespawner#193, jupyterhub/repo2docker#363

[mrocklin]

dask-kubernetes provides two ways to specify the worker template, either as a separate standalone file with a reference to that file in the config, or by placing the entire worker template into the config file. Here is our current blank config file

kubernetes:
  name: "dask-{user}-{uuid}"
  namespace: null
  count:
    start: 0
    max: null
  host: '0.0.0.0'
  port: 0
  env: {}

  worker-template-path: null

  worker-template: {}
    # kind: Pod
    # metadata:
    #   labels:
    #     foo: bar
    #     baz: quux
    # spec:
    #   restartPolicy: Never
    #   containers:
    #   - image: daskdev/dask:latest
    #     args:
    #       - dask-worker
    #       - --nthreads
    #       - '2'
    #       - --no-bokeh
    #       - --memory-limit
    #       - 6GB
    #       - --death-timeout
    #       - '60'
    #     resources:
    #       limits:
    #         cpu: "1.75"
    #         memory: 6G
    #       requests:
    #         cpu: "1.75"
    #         memory: 6G

So one solution would be to just not use worker-spec.yaml, and instead put the worker spec into dask's proper configuration system.

[mrocklin]

We could also merge the two in dask-kubernetes using dask.config.merge(worker_spec_from_file, dask.config.get('kubernetes.worker-template')).

[jhamman]

Okay, thanks. It seems setting DASK_KUBERNETES__WORKER_TEMPLATE__SPEC__IMAGE along with moving the worker-template to the dask config file will suffice.

Upon further reflection, I'm curious if removing the worker-template file might simplify things here. This would emphasize the use of the dask-config tools which do provide a lot of flexibility.

[mrocklin]

The only challenge would, I think, be that users more familiar with kubernetes than dask might not appreciate having to nest their config within dask terms. That's a concern without an active user though, so it's not particularly valid.

…

On Wed, Aug 8, 2018 at 4:47 PM, Joe Hamman ***@***.***> wrote: Okay, thanks. It seems setting DASK_KUBERNETES__WORKER_ TEMPLATE__SPEC__IMAGE along with moving the worker-template to the dask config file will suffice. Upon further reflection, I'm curious if removing the worker-template file might simplify things here. This would emphasize the use of the dask-config tools which do provide a lot of flexibility. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#92 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszHy-ycit6jNFzxkJ_fp3oGNvl0cjks5uO053gaJpZM4Vy4fK> .

[jhamman]

Fair enough. I don't have strong feelings here as it seems I can do everything I needed to with existing tooling. Happy to see this closed as is unless others have thoughts on the last topics.

[jhamman]

So dask/dask#3893 was just merged. How do we feel about expanding environment variables when loading the dask configs from dask-kubernetes?

[mrocklin]

I think that one question is "are there any fields that might have environment variables that we *don't *want to expand?"

…

On Wed, Aug 22, 2018 at 5:24 PM, Joe Hamman ***@***.***> wrote: So dask/dask#3893 <dask/dask#3893> was just merged. How do we feel about expanding environment variables when loading the dask configs from dask-kubernetes? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#92 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszOPny1S8leAIQe_WD4GUTfLRNgGuks5uTcwdgaJpZM4Vy4fK> .

[jhamman]

I think that one question is "are there any fields that might have environment variables that we *don't *want to expand?"

Not that I'm aware of in this space. @jacobtomlinson, any thoughts here?

[jacobtomlinson]

I'm trying to think of a scenario where this would be a problem.

I guess there is potential for another package to update the environment and then dask to expand something nasty into the config. e.g updating the docker image with a crypto miner.

Personally I wouldn't be concerned about things like this though. If we are worrying about the user executing malicious code I can think of much worse things than this.

[jhamman]

On the security point, dask is already vulnerable to that sort of environment variable manipulation. The reason we are discussing this is because the image specification in the worker template is in a list of arguments and dask's use of environment variables only works for nested dictionaries.

[jhamman]

But I agree with your final point, I'm not particularly concerned about this potential.