Open christian-kreuzberger-dtx opened 2 years ago
The alternative definition looks better and leaves open the possibility to define multiple volumes with different implementations in the future.
For the moment the init container could use the first volume of the definition to copy keptn files over when initializing the job.
In the future if we support multiple volumes to be used by other (non-init) containers we may introduce a name
property and a convention to know which ones need to be populated by the init container, for example:
apiVersion: v2
actions:
- name: "Print files"
events:
- name: "sh.keptn.event.sample.triggered"
files:
- my-32-megabyte-file.txt
volumes: # <---- new field
- name: keptn-files # this could be the convention for the init container volume name if we have multiple
emptyDir:
sizeLimit: 64Mi
- name: persisted-project-files # some funky name chosen by the user for their own uses
gcePersistentDisk: # some other (persistent) disk to be mounted and used by the job itself
pdName: my-data-disk
fsType: ext4
tasks:
- name: "Show files"
image: "alpine"
workingDir: "/keptn"
cmd:
- ls -lh
volumeMounts:
- mountPath: /test-pd
name: persisted-project-files
After some more consultation, we figured that it might not be beneficial to provide the option to mount volumes from Kubernetes, as
Primary focus should be the emptyDir size, which can then be configured using option 1
A PR is available in https://github.com/keptn-contrib/job-executor-service/pull/311, though there were some comments on what needs to be improved, which I don't have time to work on, unfortunately. Unassigning myself from this issue.
As a user, I want to specify the size limit of the emptyDir for a Kubernetes Job, in order to be able to download files that exceed this limit on demand, e.g., test-data.
Technical Details
Limiting the size of emptyDir is a good practice especially considering that multiple jobs can run in parallel, and to avoid filling up a single hosts memory/disk (temporarily).
However, there’s a feature gate in beta already in K8s 1.22 which would allow to use up to half the memory of the Linux host when not specifying a sizeLimit. Therefore we could even consider not having a sizeLimit by default.
See https://kubernetes.io/docs/concepts/storage/volumes/#emptydir
Proposed config.yaml change
Alternative definition (which provides more flexibility, especially towards future use-cases where we might want to share volumes between jobs)
To be decided: How should config.yaml look like
Code
It should be enough to make this part of the code configurable:
https://github.com/keptn-contrib/job-executor-service/blob/4557ca2dc0dc5f9f4aeb69b986cc4487b324c8c2/pkg/k8sutils/job.go#L54-L55
Definition of Done