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

[Feature] Allows custom job templates on helm chart #322

Merged
merged 6 commits into from
Nov 30, 2021

Conversation

gabriel-milan
Copy link
Contributor

@gabriel-milan gabriel-milan commented Nov 29, 2021

Summary

This allows setting a custom job template directly on the values.yaml file.

Importance

In our use case, the only thing we needed to do was to set environment variables for all our jobs using envFrom. With this change (tested and working for us) it allows us to change the job template directly on the values.yaml file, making it much easier to manage.

Checklist

This PR:

  • adds new tests (not applicable, I guess. please correct me if I'm wrong)
  • adds a change file in the changes/ directory (wip)

@zanieb
Copy link
Contributor

zanieb commented Nov 29, 2021

Can you share an example of using this? It seems like the user experience would not really be great from a helm-chart perspective as you'd have to ensure the file is available.

@gabriel-milan
Copy link
Contributor Author

Sure! In our deployment, we're setting agent.jobTemplate to a YAML hosted on GCS. The YAML looks like this:

apiVersion: batch/v1
kind: Job
spec:
  template:
    spec:
      containers:
        - name: flow
          envFrom:
            - secretRef:
                name: gcp-credentials

This allows us to set many environment variables from existing secrets in our different namespaces, which eases management of dev/staging/prod environments.

@zanieb
Copy link
Contributor

zanieb commented Nov 29, 2021

Perhaps we should name it "jobTemplateFilePath" or something instead in values.yaml. I expect to be able to inline some yaml as currently named.

@gabriel-milan
Copy link
Contributor Author

That makes sense

@zanieb
Copy link
Contributor

zanieb commented Nov 30, 2021

#323 will resolve the CI failures

@zanieb
Copy link
Contributor

zanieb commented Nov 30, 2021

@gabriel-milan I don't have permission to update your branch

@gabriel-milan
Copy link
Contributor Author

That's weird. I've enabled the Allow edits by maintainers. Is there anything else I can do?

@zanieb
Copy link
Contributor

zanieb commented Nov 30, 2021

@gabriel-milan that works! If this passes CI I'll get it into #324

@zanieb zanieb merged commit 5dcb159 into PrefectHQ:master Nov 30, 2021
zanieb added a commit that referenced this pull request Nov 30, 2021
@zanieb zanieb mentioned this pull request Nov 30, 2021
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.

2 participants