Include override configuration for Prefect Agent

[viniciusdc]

No description provided.

[costrouc]

Additionally there need to be docs on how to specify the overrides within the qhub-config.yaml in the configuration docs.

[viniciusdc]

I just need to add a quick doc about this and we are ready to go 👍 . I've made a simple test with local deployment, this is the structure present on qhub-config:

prefect:
  enabled: true
  overrides:
      prefect_agent:
        prefectLabels: [dev, prod]
        job:
          resources:
            requests:
              memory: 20Gi

and this one is the output result on k9s:

Environment:                                                                                                                                                                                                                                                                                                                                                                                       
│       PREFECT__CLOUD__AGENT__LABELS:         ["dev","prod"]                                                                                                                                                                       │
│       JOB_MEM_REQUEST:                       20Gi                                                                                                                                                                                 

currently the only overrideble variables are:

- IMAGE_PULL_SECRETS
-  PREFECT__CLOUD__AGENT__LABELS
- JOB_MEM_REQUEST
- JOB_MEM_LIMIT
- JOB_CPU_REQUEST
- JOB_CPU_LIMIT
- IMAGE_PULL_POLICY

I can include the secrets as well, this way the default values will be present in values.yaml and the end user will still be able to override it

[Adam-D-Lewis]

@viniciusdc I added these lines. They allow you to add arbitrary environment variables to the prefect agent. This was one thing that was requested by jhamman, and may simplify this PR. This also allows the users to specify any environment variable including others that could affect prefect without requiring us to make a PR to qhub in the future.

E.g. A values file like the one below . . .

# values.yaml
prefectImage: <placeholder>
namespace: <placeholder>
serviceAccount: prefect
prefectToken: "<placeholder>"
cloudApi: "<placeholder>"
env: 
  MY_ENV_VAR: my_env_var_value
  MY_ENV_VAR2: my_env_var_value2
  MY_ENV_VAR3: my_env_var_value3

. . . would produce the following kubernetes manifest.

# Source: prefect/templates/prefect.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: prefect-agent
  name: prefect-agent
spec:
  replicas: 1
  selector:
    matchLabels:
      app: prefect-agent
  template:
    metadata:
      labels:
        app: prefect-agent
    spec:
      serviceAccountName: prefect
      containers:
        - args:
            - prefect agent start kubernetes -e JUPYTERHUB_API_TOKEN=$JUPYTERHUB_API_TOKEN
          command:
            - /bin/bash
            - -c
          env:
            - name: MY_ENV_VAR
              value: my_env_var_value
            - name: MY_ENV_VAR2
              value: my_env_var_value2
            - name: MY_ENV_VAR3
              value: my_env_var_value3
# . . . rest of file would be below

[viniciusdc]

Oh that's nice! thanks for the pointer Adam

[viniciusdc]

My only concern with that is related with the variables that need use toJson when included in the manifest, e.g. PREFECT__CLOUD__AGENT__LABELS which is a list and IMAGE_PULL_SECRETS which is a dictionary containing the secrets. How would I handle that using the proposed env structure above?

[Adam-D-Lewis]

I imagine you could get the desired format by using back slashes. If not, I think it's reasonable to still keep the environment variables that need special formatting in there.

[Adam-D-Lewis]

typo: you should be your

[viniciusdc]

So based on @Adam-D-Lewis comment and some talks I had with him and @costrouc about the agent helm chart as a whole. I've decided to maintain the overrides values from the values.yaml files, using the whole structure block for the variable. But I also used the loop block Adam suggested to permor inclusions of new variables into the agent, this way, for example, in the qhub-config we have now under prefect field:

overrides:
      extraVars:
        PREFECT__CONTEXT__SECRETS__AWS_CREDENTIALS: eyJBQ0NFU1NfS0VZIjogImFiY2RlZiIsICJTRUNSRVRfQUNDRVNTX0tFWSI6ICJnaGlqa2xtbiJ9Cg==
      prefect_agent:
        prefectLabels: [dev, prod]
        job:
          resources:
            requests:
              memory: 20Gi

where in this case we will override the PREFECT__CLOUD__AGENT__LABELS and JOB_MEM_REQUEST fields in the chart as well as adding a new variable as an extra, called PREFECT__CONTEXT__SECRETS__GCP_CREDENTIALS. We ended up encoding base64 to maintain the string characteristics until reaching the helm chart.

The result can be seen in the cluster deployment for the agent:

PREFECT__CLOUD__AGENT__LABELS:               ["dev","prod"] 
JOB_MEM_REQUEST:                             20Gi
PREFECT__CONTEXT__SECRETS__AWS_CREDENTIALS:  {"ACCESS_KEY": "abcdef", "SECRET_ACCESS_KEY": "ghijklmn"}

[Adam-D-Lewis]

For extraVars, we'd need to add some documentation explaining it. Also, maybe call it envVars or extraEnvVars to make it clear that these are environment variables?

[Adam-D-Lewis]

I've made some adjustments that don't require base64 encoding of environment variables. Check it out, @viniciusdc and let me know if this meets all your needs. I made a repo where I was testing out a local deployment of this branch if that's helpful for you. You can access it at https://github.com/Quansight/grafana-prometheus-qhub/tree/prefect_overrides

[viniciusdc]

I've made some adjustments that don't require base64 encoding of environment variables. Check it out, @viniciusdc and let me know if this meets all your needs. I made a repo where I was testing out a local deployment of this branch if that's helpful for you. You can access it at https://github.com/Quansight/grafana-prometheus-qhub/tree/prefect_overrides

Hi @Adam-D-Lewis thank you so much for that! I noticed that if we pass a dic as a key in the overrides envVars, e.g { }, we end up getting a map[] object in the final manifest, but this is easily resolved if we pass the original dict as a string. (I will add this to the docs later).

Also, I've included an extra secretsEnvVars field in the overrides. This way the user can set a secret that will be added to the manifest chart as a kubernetes secret. Let me know your thoughts about that as well. I've teste locally and it worked pretty well.

[costrouc]

Looks great! Thanks @viniciusdc and @Adam-D-Lewis for working hard on this PR.