Dask Prefect Integration queries

[leej3]

I had some general questions about the Dask Prefect integration and making a little more configurable/easier to manage. There seem to be a few things that would be useful to have configurable for the Prefect Agent. In addition to the fields here
I think many of the fields in the chart could benefit from the same templating. I could probably take a stab at this. I’m wondering is this the best way to do this? @aktech did you envisage this list of templated variables expanding much beyond the current count? @danlester do you have a sense of whether the pending additions with keycloak/vault make this approach to secrets management obsolete?
Regarding easier to manage… because we do not have access to docker on our client systems we have resorted to a rather convoluted approach on CI to registering flows. One thing that seems like it would be a really nice addition for the Prefect integration would be to bind mount the conda-store nfs into the prefect runner. That way all environments available on QHub can be considered for flow execution. Has this been considered? Would it be feasible? I don’t have a sense of the gotchas that would arise in pursuing this.

[leej3]

This seems as good a place as any to throw this.
cc: @costrouc , @viniciusdc, @trallard

I am attempting to build on #768 with the addition that the prefect.yaml values are more extensively configurable. I had a brief chat with @aktech about this and he though it would be good to use the yaml dictionary in the qhub config yaml to provide structure to the configuration.

The thought is that the prefect agent could be configured in the config yaml using something like:

prefect:
  enabled: true
  agent:
    prefect__cloud__api: " a string"
    image_pull_secrets: " a string"
    prefect__cloud__agent__labels: " a string"
    job_mem_request: " a string"
    job_mem_limit: " a string"
    job_cpu_request: " a string"
    job_cpu_limit: " a string"
    image_pull_policy: " a string"
    service_account_name: " a string"
    prefect__backend: " a string"
    prefect__cloud__agent__agent_address: " a string"
    prefect__cloud__agent__env_vars: 
      agent_env_var_1: " a string"

The corresponding agent setting would be configured in the helm chart prefect.yaml file with something along the lines of:

...
        env:
...
            - name: JOB_MEM_REQUEST
              value: {{ .Values.prefectAgent.job_mem_request }} 
...

There is a basic implementation of this here pair-coded by myself and @viniciusdc . Overall the main problem we are having is that the main.tf file seems to want to get strings as input. The most recent commit shows when we set the type to an experimental object with defined attributes. The error is the following:

[terraform]: ╷
[terraform]: │ Error: Invalid reference
[terraform]: │ 
[terraform]: │   on modules/kubernetes/services/prefect/main.tf line 37, in resource "helm_release" "prefect":
[terraform]: │   37:         prefect__cloud__api = optional(string)
[terraform]: │ 
[terraform]: │ A reference to a resource type must be followed by at least one attribute
[terraform]: │ access, specifying the resource name.
[terraform]: ╵
[terraform]: ╷
[terraform]: │ Error: Invalid reference
[terraform]: │ 
[terraform]: │   on modules/kubernetes/services/prefect/main.tf line 38, in resource "helm_release" "prefect":
[terraform]: │   38:         image_pull_secrets = optional(string)
[terraform]: │ 
[terraform]: │ A reference to a resource type must be followed by at least one attribute
[terraform]: │ access, specifying the resource name.
[terraform]: ╵
[terraform]: ╷
[terraform]: │ Error: Invalid reference
[terraform]: │ 
[terraform]: │   on modules/kubernetes/services/prefect/main.tf line 39, in resource "helm_release" "prefect":
[terraform]: │   39:         prefect__cloud__agent__labels = optional(string)
[terraform]: │ 
[terraform]: │ A reference to a resource type must be followed by at least one attribute
[terraform]: │ access, specifying the resource name.
[terraform]: ╵

Problem encountered: Terraform error

The above error requesting "at least one attribute access" is similar to the error that occurs when the type is instead set to type map. Passing the object json-encoded into a string from kubernetes.tf to the helm chart seems to work fine but we can't figure how to reconstitute a map object within the chart (there is an error about a dynamic object whose values are not know until after the apply stage that I don't have convenient at the moment).

TLDR:
What would be the best way to make helm charts configurable without breaking the abstraction they represent. @viniciusdc suggested considering using .tpl files as an alternative to this strategy. Solving this problem with an elegant pattern seems worth it as it can likely be readily re-used for configuring the deployment of charts...

[viniciusdc]

Hi @leej3 @costrouc , I was looking to what was done for ClearMl here, I think we could then adapt the prefect agent created in #768 by following the same steps as is agentGroups for ClearML... we could use the experimental optional variables to retrieve the values from the QHub config file as you suggested and then include those values directly to the Values.yaml from prefectAgent, that way we will not need to parse any strings… as well as following the helm chart template structure (as our lists and dicts would be passed as springs within the prefect value)

Something under those lines...
on QHub config file:

prefect:
  enabled: true
  agent:
    prefect__cloud__agent__labels: " a string"
    job_mem_request: " a string"
    job_mem_limit: " a string"
    prefect__cloud__agent__env_vars : 
      agent_env_var_1: " a string"

on scheema.py

class PrefectAgent(Base):
    prefect__cloud__agent__labels: typing.Optional[str]
    job_mem_request: typing.Optional[str]
    job_mem_limit: typing.Optional[str]
    prefect__cloud__agent__env_vars: typing.Optional[dict]

class Prefect(Base):
    enabled: bool
    image: typing.Optional[str]
    agent: typing.Optional[PrefectAgent]

from cookiecutter perspective:

"prefect": {
        "enabled": null,
        "image": null,
        "agent": null
    },

now enters the intriguing terraform + helm approach I've suggested.... starting with kubernets.tf:

{% if cookiecutter.prefect.enabled -%}
module "prefect" {
  source = "./modules/kubernetes/services/prefect"

  depends_on = [
    module.qhub
  ]
...
{% if cookiecutter.prefect.agent is defined %}
  prefect_agent        = {
  {% for key, value in cookiecutter.prefect.agent.items() %}
    "{{ "%s" | format(key) }}" = "{{ value }}"
  {% endfor %}
    }
{% endif %}
...
{% endif -%}

the main purpose of this prefect_agent is mapping trhough cookiecutter the variable from the qhub config... (I don't know if there is a better way of doing this right now...), based on what set does when merging a helm resource, we could then define a dynamic block based on the prefect_agent var inside ../services/prefect/main.tf as something like this (this might not be the exact end result)

resource "helm_release" "prefect" {
  name      = "prefect"
  namespace = var.namespace
  chart     = "${path.module}/chart"
  
  dynamic "set" {
    for_each = var.prefect_agent
    iterator   = prefect_agent_var
    content {
      prefect_agent_var = var.prefect_agent["{{ prefect_agent_var }}"]
    }
  }

as the final touch, we should have inside the prefect.yaml the following structures defined

       env:
            - name: PREFECT__CLOUD__AGENT__LABELS
              value: {{- include ".Values.prefect_agent.__cloud__agent__labels " $ | nindent x }}
            - name: JOB_MEM_REQUEST
              value: {{ .Values.agent.job_mem_request }} 
            - name: JOB_MEM_LIMIT
              value: {{ .Values.prefect_agent.job_mem_limit }} 
            - name: PREFECT__CLOUD__AGENT__ENV_VARS
              value: "{{ .Values.prefect_agent.envVars }}"

As we are defining a set block under prefec/main.tfthose values for prefect_agent should be defined, what would be the following block in values.yaml under normal configurations:

prefect_agent:
  __cloud__agent__labels: "[]"
  job_mem_request : ""
  job_mem_limit: ""
  envVars: {}

[costrouc]

I see two actions:

• add an overrides section to prefect helm chart via variables
• determine which variables are variables that everyone will need to set and add expose those as

Opened an issue for this discussion #799

[viniciusdc]

This was fixed with #813

[leej3]

Agreed that the overrides challenges are now solved (very nicely).

One thing that seems like it would be a really nice addition for the Prefect integration would be to bind mount the conda-store nfs into the prefect runner. That way all environments available on QHub can be considered for flow execution. Has this been considered? Would it be feasible? I don’t have a sense of the gotchas that would arise in pursuing this.

My understanding of this now is that the poorer performance of the NFS mounts makes this less desirable. There are some other ideas for this though.

Regarding easier to manage… because we do not have access to docker on our client systems we have resorted to a rather convoluted approach on CI to registering flows.

This is still something I consider outstanding. It may just require an entry in the user docs. On the deployment I am working on I am still not confident the approach we are taking is one of least friction and so am reluctant to propose a PR with said docs. I think I'll open an issue though as the fuzzy stuff where I couldn't even clearly articulate the problem at the time have now been resolved

[Adam-D-Lewis]

I'm closing old discussions in a repo cleanup effort. Feel free to re-open if needed.