Skip to content

Commit

Permalink
bugfix: proper handling of values set to 0 (#97)
Browse files Browse the repository at this point in the history
`if .Values.X` evaluates to false if X is defined to be 0.
This resulted in retryLimit and terminationGracePeriodSeconds
not being properly propagated through the the generated yaml
if the user sets their value to 0 (as opposed to nil).
  • Loading branch information
dgrove-oss authored Nov 11, 2024
1 parent aa6a58a commit 7c289d7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
2 changes: 1 addition & 1 deletion tools/pytorchjob-generator/chart/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ annotations:


{{- define "mlbatch.schedulingSpec" }}
{{- if .Values.terminationGracePeriodSeconds }}
{{- if ne .Values.terminationGracePeriodSeconds nil }}
terminationGracePeriodSeconds: {{ .Values.terminationGracePeriodSeconds }}
{{- end }}
{{- if .Values.bypassCoscheduler }}
Expand Down
2 changes: 1 addition & 1 deletion tools/pytorchjob-generator/chart/templates/appwrapper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ metadata:
{{- if .Values.retryPausePeriodDuration }}
workload.codeflare.dev.appwrapper/retryPausePeriodDuration: "{{ .Values.retryPausePeriodDuration }}"
{{- end }}
{{- if .Values.retryLimit }}
{{- if ne .Values.retryLimit nil }}
workload.codeflare.dev.appwrapper/retryLimit: "{{ .Values.retryLimit }}"
{{- end }}
{{- if .Values.forcefulDeletionGracePeriodDuration }}
Expand Down
16 changes: 16 additions & 0 deletions tools/pytorchjob-generator/chart/tests/helloworld_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,22 @@ tests:
workload.codeflare.dev.appwrapper/forcefulDeletionGracePeriodDuration: "19s"
workload.codeflare.dev.appwrapper/deletionOnFailureGracePeriodDuration: "2s"

- it: Setting integer fault tolerance annotation to 0
set:
retryLimit: 0
terminationGracePeriodSeconds: 0
asserts:
- isSubset:
path: metadata.annotations
content:
workload.codeflare.dev.appwrapper/retryLimit: "0"
- equal:
path: spec.components[0].template.spec.pytorchReplicaSpecs.Master.template.spec.terminationGracePeriodSeconds
value: 0
- equal:
path: spec.components[0].template.spec.pytorchReplicaSpecs.Worker.template.spec.terminationGracePeriodSeconds
value: 0

- it: Setting just one tolerance annotation
set:
deletionOnFailureGracePeriodDuration: "6h"
Expand Down

0 comments on commit 7c289d7

Please sign in to comment.