-
Notifications
You must be signed in to change notification settings - Fork 39
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
Cron job maintenance #448
Merged
Merged
Cron job maintenance #448
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
4138130
Remove spaces that makes helm lint unhappy
ksuderman e0b960d
First pass at defining cron jobs in the values.yaml file.
ksuderman 9c84d17
Fix command formatting and quoting
ksuderman d2941d7
Remove nodeSelector
ksuderman 95474e2
Tmp cleanup and maintenance scripts are treated as special cases.
ksuderman 93930d7
Update the name of the container used to run the maintenance script.
ksuderman 287cf13
Define all cron jobs in the values.yaml file again, but allows time d…
ksuderman 8225be5
Use walltime limit for cleanup and other minor tweaks
nuwang 8cc5d94
Change find units from seconds to days
nuwang 64320c8
Parameterize the nodeSelector
ksuderman 2bbaa3a
Remove the chown cron job
ksuderman df1fb39
Reverting my revert
ksuderman 247cbf5
Add extraFileMappings for cron jobs
ksuderman cf77963
Start documenting cron jobs
ksuderman 13e4f0b
Make the galaxy.yml file available in a configmap for the maintenance…
ksuderman 27fd304
Update galaxy/values.yaml
ksuderman 42488ff
Add helper to calculate the postgres connection string
ksuderman 0fafea8
Add cron job documentation and remove Galaxy versions section
ksuderman 497b572
Add env definitions
ksuderman da6a97a
Maintenace job should include default env vars
ksuderman cdc2a98
Update galaxy/values.yaml
ksuderman 63bc997
Update galaxy/values.yaml
ksuderman d96979a
Parameterize Docker image for cron jobs
ksuderman 3af24cd
Allow mode (permissions) to be defined on extraFileMappings
ksuderman 3fe5723
Add example cron job
ksuderman 07fdbd4
Additional documentation for cron jobs
ksuderman b4cbdf6
Comment out the example cron job. Examples should not add arbitrary c…
ksuderman db82ef0
Run extraEnv values through the template engine
ksuderman a7c1aed
Merge branch 'master' into 408-maintenance
ksuderman 14f190d
Add newline to the end of the file.
ksuderman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| apiVersion: v1 | ||
| metadata: | ||
| name: {{ .Release.Name }}-galaxy-config | ||
| labels: | ||
| {{- include "galaxy.labels" $ | nindent 4 }} | ||
| kind: ConfigMap | ||
| data: | ||
| galaxy.yml: | | ||
| {{- .Values.galaxy | toYaml | nindent 4 }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,47 +1,112 @@ | ||
| {{ range $key, $cronjob := .Values.cronJobs }} | ||
| --- | ||
| apiVersion: batch/v1 | ||
| kind: CronJob | ||
| metadata: | ||
| name: {{ include "galaxy.fullname" . }}-maintenance | ||
| name: {{ include "galaxy.fullname" $ }}-cron-{{ $key }} | ||
| labels: | ||
| {{- include "galaxy.labels" . | nindent 4 }} | ||
| {{- include "galaxy.labels" $ | nindent 4 }} | ||
| spec: | ||
| schedule: "0 2 * * *" | ||
| schedule: {{ $cronjob.schedule | quote }} | ||
| jobTemplate: | ||
| spec: | ||
| template: | ||
| spec: | ||
| {{- if $cronjob.securityContext }} | ||
| securityContext: | ||
| {{- toYaml .Values.securityContext | nindent 12 }} | ||
| {{- with .Values.nodeSelector }} | ||
| {{- toYaml $cronjob.securityContext | nindent 12 }} | ||
| {{- end}} | ||
| {{- if $cronjob.nodeSelector }} | ||
| nodeSelector: | ||
| {{- toYaml . | nindent 16 }} | ||
| {{- toYaml $cronjob.nodeSelector | nindent 12 }} | ||
| {{- else if $.Values.nodeSelector }} | ||
| nodeSelector: | ||
| {{- toYaml $.Values.nodeSelector | nindent 12 }} | ||
| {{- end }} | ||
| containers: | ||
| - name: galaxy-maintenance | ||
| image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" | ||
| imagePullPolicy: {{ .Values.image.pullPolicy }} | ||
| # delete all tmp files older than walltime limit | ||
| - name: galaxy-cron-{{ $key }} | ||
| {{- if $cronjob.image }} | ||
| image: {{ $cronjob.image.repository }}:{{ $cronjob.image.tag }} | ||
| {{- else }} | ||
| image: "{{ $.Values.image.repository }}:{{ $.Values.image.tag }}" | ||
| {{- end }} | ||
| imagePullPolicy: {{ $.Values.image.pullPolicy }} | ||
| {{- if or $cronjob.defaultEnv $cronjob.extraEnv }} | ||
| env: | ||
| {{- if $cronjob.defaultEnv }} | ||
| {{- include "galaxy.podEnvVars" $}} | ||
| {{- end }} | ||
| {{- if $cronjob.extraEnv }} | ||
| {{- range $env := $cronjob.extraEnv }} | ||
| - name: {{ $env.name }} | ||
| value: {{ tpl $env.value $ | quote }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| command: | ||
| - find | ||
| - {{ .Values.persistence.mountPath }}/tmp | ||
| - '!' | ||
| - -newermt | ||
| - -{{ (index .Values "configs" "job_conf.yml" "runners" "k8s" "k8s_walltime_limit" | default 604800) }} seconds | ||
| - -type | ||
| - f | ||
| - -exec | ||
| - rm | ||
| - '{}' | ||
| - ; | ||
| {{- range $cmd := $cronjob.command }} | ||
| - {{ tpl $cmd $ | quote }} | ||
| {{- end}} | ||
| {{- if $cronjob.args }} | ||
| args: | ||
| {{- range $arg := $cronjob.args }} | ||
| - {{ tpl $arg $ | quote }} | ||
| {{- end }} | ||
| {{- end }} | ||
| volumeMounts: | ||
| - name: galaxy-data | ||
| mountPath: {{ .Values.persistence.mountPath }} | ||
| mountPath: {{ $.Values.persistence.mountPath }} | ||
| {{- range $key, $entry := $cronjob.extraFileMappings }} | ||
| - name: {{ include "galaxy.getExtraFilesUniqueName" $key }} | ||
| mountPath: {{ $key }} | ||
| subPath: {{ include "galaxy.getFilenameFromPath" $key }} | ||
| {{- end }} | ||
| volumes: | ||
| - name: galaxy-data | ||
| {{- if .Values.persistence.enabled }} | ||
| {{- if $.Values.persistence.enabled }} | ||
| persistentVolumeClaim: | ||
| claimName: {{ template "galaxy.pvcname" . }} | ||
| claimName: {{ template "galaxy.pvcname" $ }} | ||
| {{- else }} | ||
| emptyDir: {} | ||
| {{- end }} | ||
| {{- range $key, $entry := $cronjob.extraFileMappings }} | ||
| - name: {{ include "galaxy.getExtraFilesUniqueName" $key }} | ||
| {{- if $entry.useSecret }} | ||
| secret: | ||
| secretName: {{ printf "%s-%s" (include "galaxy.fullname" $) (include "galaxy.getExtraFilesUniqueName" $key) }} | ||
| {{- else }} | ||
| configMap: | ||
| name: {{ printf "%s-%s" (include "galaxy.fullname" $) (include "galaxy.getExtraFilesUniqueName" $key) }} | ||
| {{- end }} | ||
| {{- if $entry.mode }} | ||
| defaultMode: {{ $entry.mode }} | ||
| {{- end }} | ||
| {{- end }} | ||
| restartPolicy: OnFailure | ||
| {{- if $cronjob.extraFileMappings }} | ||
| {{- range $name, $entry := $cronjob.extraFileMappings }} | ||
| --- | ||
| apiVersion: v1 | ||
| metadata: | ||
| # Extract the filename portion only | ||
ksuderman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| name: {{ printf "%s-%s" (include "galaxy.fullname" $) (include "galaxy.getExtraFilesUniqueName" $name) }} | ||
| labels: | ||
| {{- include "galaxy.labels" $ | nindent 4 }} | ||
| {{- if $entry.useSecret }} | ||
| kind: Secret | ||
| type: Opaque | ||
| stringData: | ||
| {{- else }} | ||
| kind: ConfigMap | ||
| data: | ||
| {{- end }} | ||
| {{- include "galaxy.getFilenameFromPath" $name | nindent 2 }}: | | ||
| {{- if $entry.tpl }} | ||
| {{- tpl (tpl $entry.content $) $ | nindent 4 }} | ||
| {{- else }} | ||
| {{- $entry.content | nindent 4 }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| {{- end }} | ||
ksuderman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to add an enabled flag as we have for everything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered that, but in other cases when
enabled: falseis set we don't render the template. However, in this case it means theCronJobis not defined and not available to be run manually. By setting the schedule to a time that never occurs theCronJobis defined and is available to be run manually if desired. At least that was my thought process.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading more about cron jobs, maybe I should be using
.spec.suspendhere. I'll play around with that and see if I get the desired behavior.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to investigate suspend. Maybe we should add both. Some instructions on how to manually invoke the cronjob would also be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
.spec.suspendlooks to be less than ideal, particularly if one wants to run the job once but keep it suspended. In that case the user must:.spec.suspend=false.spec.suspend=trueGiven users can always disable a cron job with the
scheduleI am not sure.spec.suspendis worth it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that adding an enabled flag would still be useful. Users who want to run the cronjob manually can still use the trick you suggested. Just that it feels weird to have to think of an invalid date for the simplest case of not wanting the cron job.