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

add prefectLabels and envVars to prefect config #768

Closed
wants to merge 2 commits into from
Closed

add prefectLabels and envVars to prefect config #768

wants to merge 2 commits into from

Conversation

jhamman
Copy link

@jhamman jhamman commented Aug 19, 2021

This is a quick attempt at adding labels and environment variables to the prefect agent chart. This is my first time touching qhub and terraform so I'm very open to suggestions on better ways to go about this.

If you are starting a review, I'd start at .../services/prefect/chart/templates/prefect.yaml and work backwards from there.

This initial implementation would expose two new parameters under prefect like this:

prefect:
  enabled: true
  agent:
    prefectLabels: ['foo', 'bar']
    envVars: {'TZ': 'UTC'}

closes #764

@@ -40,7 +40,7 @@ spec:
- name: IMAGE_PULL_SECRETS
value: ''
- name: PREFECT__CLOUD__AGENT__LABELS
value: '[]'
value: {{ .Values.agent.prefectLabels | toJson | quote }}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -57,6 +57,8 @@ spec:
value: cloud
- name: PREFECT__CLOUD__AGENT__AGENT_ADDRESS
value: http://:8080
- name: PREFECT__CLOUD__AGENT__ENV_VARS
value: {{ .Values.agent.envVars | toJson | quote }}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefect's server doesn't include this variable but the agent cli does:

prefect agent kubernetes install -e "spam=more"
...
        - name: PREFECT__CLOUD__AGENT__ENV_VARS
          value: '{"spam": "more"}'
...

@@ -235,6 +235,12 @@ module "prefect" {
{% if cookiecutter.prefect.image is defined -%}
image = "{{ cookiecutter.prefect.image }}"
{% endif -%}
{% if cookiecutter.prefect.agent.prefectLabels is defined -%}
image = "{{ cookiecutter.prefect.agent.prefectLabels }}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this will need to be changed to

agent = {
{% if cookiecutter.prefect.agent.prefectLabels is defined -%}
     prefectLabels = "{{ cookiecutter.prefect.agent.prefectLabels }}"
{% endif %}
...
}

@costrouc
Copy link
Member

Thanks for the PR @jhamman! I personally was not that responsible for the prefect agent deployment but I will make sure to check out this PR in more detail tomorrow.

@viniciusdc
Copy link
Contributor

Hi @jhamman thanks for opening this PR and raising this discussion, this modification is currently being handled as part of a feature request by one of our clients. We really appreciate your PR and will notify you when the other PR has been completed. The issue this is related to is #799.

@Adam-D-Lewis
Copy link
Member

@jhamman Thanks for this PR. We have a working version now merged in from #813 which allows you to define arbitrary environment variables on the prefect agent and add labels to the prefect agent.
Closing this PR since that is now resolved.

@jhamman
Copy link
Author

jhamman commented Sep 29, 2021

Fantastic! Thanks @Adam-D-Lewis!

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

Successfully merging this pull request may close these issues.

[enhancement] configure prefect agent labels and environment variables
4 participants