Merge pull request #62 from Duke-GCB/30-tmpdir-as-emptydir

Use emptyDir for '/tmp' inside containers

calrissian/job.py

@@ -90,6 +90,7 @@ class KubernetesVolumeBuilder(object):
 
     def __init__(self):
         self.persistent_volume_entries = {}
+        self.emptydir_volume_names = []
         self.volume_mounts = []
         self.volumes = []
 
@@ -116,6 +117,14 @@ def add_persistent_volume_entry(self, prefix, sub_path, claim_name):
         self.persistent_volume_entries[prefix] = entry
         self.volumes.append(entry['volume'])
 
+    def add_emptydir_volume(self, name):
+        volume = {
+            'name': name,
+            'emptyDir': {},
+        }
+        self.emptydir_volume_names.append(name)
+        self.volumes.append(volume)
+
     def find_persistent_volume(self, source):
         """
         For a given source path, return the volume entry that contains it
@@ -148,6 +157,16 @@ def add_volume_binding(self, source, target, writable):
         }
         self.volume_mounts.append(volume_mount)
 
+    def add_emptydir_volume_binding(self, name, target):
+        if not name in self.emptydir_volume_names:
+            # fail if the name is not registered
+            raise VolumeBuilderException('Could not find an emptyDir volume named {}'.format(name))
+        volume_mount = {
+            'name': name,
+            'mountPath': target
+        }
+        self.volume_mounts.append(volume_mount)
+
 
 class KubernetesPodBuilder(object):
 
@@ -282,6 +301,8 @@ def build(self):
 # create_file_and_add_volume and add_volumes
 class CalrissianCommandLineJob(ContainerCommandLineJob):
 
+    container_tmpdir = '/tmp'
+
     def __init__(self, *args, **kwargs):
         super(CalrissianCommandLineJob, self).__init__(*args, **kwargs)
         self.client = KubernetesClient()
@@ -301,7 +322,7 @@ def populate_env_vars(self):
         self.environment["HOME"] = self.builder.outdir
         # cwltool DockerCommandLineJob always sets TMPDIR to /tmp
         # https://github.com/common-workflow-language/cwltool/blob/1.0.20181201184214/cwltool/docker.py#L333
-        self.environment["TMPDIR"] = '/tmp'
+        self.environment["TMPDIR"] = self.container_tmpdir
 
     def wait_for_kubernetes_pod(self):
         return self.client.wait_for_completion()
@@ -388,8 +409,11 @@ def create_kubernetes_runtime(self, runtimeContext):
 
         # Append volume for outdir
         self._add_volume_binding(os.path.realpath(self.outdir), self.builder.outdir, writable=True)
-        # Append volume for tmp
-        self._add_volume_binding(os.path.realpath(self.tmpdir), '/tmp', writable=True)
+        # Use a kubernetes emptyDir: {} volume for /tmp
+        # Note that below add_volumes() may result in other temporary files being mounted
+        # from the calrissian host's tmpdir prefix into an absolute container path, but this will
+        # not conflict with '/tmp' as an emptyDir
+        self._add_emptydir_volume_and_binding('tmpdir', self.container_tmpdir)
 
         # Call the ContainerCommandLineJob add_volumes method
         self.add_volumes(self.pathmapper,
@@ -435,6 +459,10 @@ def create_kubernetes_runtime(self, runtimeContext):
     def execute_kubernetes_pod(self, pod):
         self.client.submit_pod(pod)
 
+    def _add_emptydir_volume_and_binding(self, name, target):
+        self.volume_builder.add_emptydir_volume(name)
+        self.volume_builder.add_emptydir_volume_binding(name, target)
+
     def _add_volume_binding(self, source, target, writable=False):
         self.volume_builder.add_volume_binding(source, target, writable)
 

openshift/CalrissianJob-fail-wf.yaml

@@ -14,8 +14,6 @@ spec:
           - "1G"
           - "--max-cores"
           - "2"
-          - "--tmpdir-prefix"
-          - "/calrissian/tmptmp/"
           - "--tmp-outdir-prefix"
           - "/calrissian/tmpout/"
           - "--outdir"
@@ -27,8 +25,6 @@ spec:
           name: calrissian-input-data
         - mountPath: /calrissian/tmpout
           name: calrissian-tmpout
-        - mountPath: /calrissian/tmptmp
-          name: calrissian-tmp
         - mountPath: /calrissian/output-data
           name: calrissian-output-data
         env:
@@ -44,9 +40,6 @@ spec:
       - name: calrissian-tmpout
         persistentVolumeClaim:
           claimName: calrissian-tmpout
-      - name: calrissian-tmp
-        persistentVolumeClaim:
-          claimName: calrissian-tmp
       - name: calrissian-output-data
         persistentVolumeClaim:
           claimName: calrissian-output-data

openshift/CalrissianJob-revsort-single-default-container.yaml

@@ -17,8 +17,6 @@ spec:
           - "16G"
           - "--max-cores"
           - "8"
-          - "--tmpdir-prefix"
-          - "/calrissian/tmptmp/"
           - "--tmp-outdir-prefix"
           - "/calrissian/tmpout/"
           - "--outdir"
@@ -30,8 +28,6 @@ spec:
           name: calrissian-input-data
         - mountPath: /calrissian/tmpout
           name: calrissian-tmpout
-        - mountPath: /calrissian/tmptmp
-          name: calrissian-tmp
         - mountPath: /calrissian/output-data
           name: calrissian-output-data
         env:
@@ -47,9 +43,6 @@ spec:
       - name: calrissian-tmpout
         persistentVolumeClaim:
           claimName: calrissian-tmpout
-      - name: calrissian-tmp
-        persistentVolumeClaim:
-          claimName: calrissian-tmp
       - name: calrissian-output-data
         persistentVolumeClaim:
           claimName: calrissian-output-data

openshift/CalrissianJob-revsort-single.yaml

@@ -15,8 +15,6 @@ spec:
           - "16G"
           - "--max-cores"
           - "8"
-          - "--tmpdir-prefix"
-          - "/calrissian/tmptmp/"
           - "--tmp-outdir-prefix"
           - "/calrissian/tmpout/"
           - "--outdir"
@@ -28,8 +26,6 @@ spec:
           name: calrissian-input-data
         - mountPath: /calrissian/tmpout
           name: calrissian-tmpout
-        - mountPath: /calrissian/tmptmp
-          name: calrissian-tmp
         - mountPath: /calrissian/output-data
           name: calrissian-output-data
         env:
@@ -45,9 +41,6 @@ spec:
       - name: calrissian-tmpout
         persistentVolumeClaim:
           claimName: calrissian-tmpout
-      - name: calrissian-tmp
-        persistentVolumeClaim:
-          claimName: calrissian-tmp
       - name: calrissian-output-data
         persistentVolumeClaim:
           claimName: calrissian-output-data

openshift/CalrissianJob-revsort.yaml

@@ -19,8 +19,6 @@ spec:
           - "16G"
           - "--max-cores"
           - "8"
-          - "--tmpdir-prefix"
-          - "/calrissian/tmptmp/"
           - "--tmp-outdir-prefix"
           - "/calrissian/tmpout/"
           - "--outdir"
@@ -34,8 +32,6 @@ spec:
           name: calrissian-input-data
         - mountPath: /calrissian/tmpout
           name: calrissian-tmpout
-        - mountPath: /calrissian/tmptmp
-          name: calrissian-tmp
         - mountPath: /calrissian/output-data
           name: calrissian-output-data
         env:
@@ -51,9 +47,6 @@ spec:
       - name: calrissian-tmpout
         persistentVolumeClaim:
           claimName: calrissian-tmpout
-      - name: calrissian-tmp
-        persistentVolumeClaim:
-          claimName: calrissian-tmp
       - name: calrissian-output-data
         persistentVolumeClaim:
           claimName: calrissian-output-data

openshift/VolumeClaims.yaml

@@ -22,17 +22,6 @@ spec:
 ---
 apiVersion: v1
 kind: PersistentVolumeClaim
-metadata:
-  name: calrissian-tmp
-spec:
-  accessModes:
-    - ReadWriteMany
-  resources:
-    requests:
-      storage: 1Gi
----
-apiVersion: v1
-kind: PersistentVolumeClaim
 metadata:
   name: calrissian-output-data
 spec:

tests/test_job.py

@@ -8,6 +8,7 @@
 import threading
 
 
+
 class SafeNameTestCase(TestCase):
 
     def setUp(self):
@@ -18,6 +19,7 @@ def test_makes_name_safe(self):
         made_safe = k8s_safe_name(self.unsafe_name)
         self.assertEqual(self.safe_name, made_safe)
 
+
 class RandomTagTestCase(TestCase):
 
     def test_makes_random_tags(self):
@@ -148,6 +150,22 @@ def test_volume_binding_exception_if_not_found(self):
             self.volume_builder.add_volume_binding('/prefix/2/input2', '/input2-target', False)
         self.assertIn('Could not find a persistent volume', str(context.exception))
 
+    def test_add_emptydir_volume(self):
+        self.assertEqual(0, len(self.volume_builder.emptydir_volume_names))
+        self.volume_builder.add_emptydir_volume('empty-volume')
+        self.assertIn('empty-volume', self.volume_builder.emptydir_volume_names)
+
+    def test_add_emptydir_volume_binding(self):
+        self.volume_builder.add_emptydir_volume('empty-volume')
+        self.volume_builder.add_emptydir_volume_binding('empty-volume', '/path/to/empty')
+        expected = {'name': 'empty-volume', 'mountPath': '/path/to/empty'}
+        self.assertIn(expected, self.volume_builder.volume_mounts)
+
+    def test_add_emptydir_volume_binding_exception_if_not_found(self):
+        self.assertEqual(0, len(self.volume_builder.emptydir_volume_names))
+        with self.assertRaises(VolumeBuilderException) as context:
+            self.volume_builder.add_emptydir_volume_binding('empty-volume', '/path/to/empty')
+
     @patch('calrissian.job.KubernetesPodVolumeInspector')
     def test_add_persistent_volume_entries_from_pod(self, mock_kubernetes_pod_inspector):
         mock_kubernetes_pod_inspector.return_value.get_mounted_persistent_volumes.return_value = [
@@ -464,7 +482,11 @@ def realpath(path):
             return '/real' + path
         mock_os.path.realpath = realpath
         mock_add_volume_binding = Mock()
+        mock_add_emptydir_volume = Mock()
+        mock_add_emptydir_volume_binding = Mock()
         mock_volume_builder.return_value.add_volume_binding = mock_add_volume_binding
+        mock_volume_builder.return_value.add_emptydir_volume = mock_add_emptydir_volume
+        mock_volume_builder.return_value.add_emptydir_volume_binding = mock_add_emptydir_volume_binding
 
         mock_pod_builder.return_value.build.return_value = '<built pod>'
         job = self.make_job()
@@ -473,10 +495,10 @@ def realpath(path):
         mock_runtime_context = Mock(tmpdir_prefix='TP')
         built = job.create_kubernetes_runtime(mock_runtime_context)
         # Adds volume binding for outdir
-        # Adds volume binding for /tmp
-        self.assertEqual(mock_add_volume_binding.call_args_list,
-                         [call('/real/outdir', '/out', True),
-                          call('/real/tmpdir', '/tmp', True)])
+        self.assertEqual(mock_add_volume_binding.call_args, call('/real/outdir', '/out', True))
+        # Adds emptydir binding for tmpdir
+        self.assertEqual(mock_add_emptydir_volume.call_args, call('tmpdir'))
+        self.assertEqual(mock_add_emptydir_volume_binding.call_args, call('tmpdir', '/tmp'))
         # looks at generatemapper
         # creates a KubernetesPodBuilder
         self.assertEqual(mock_pod_builder.call_args, call(