fix(k8s): ensure non-zero exit code if test/task with artifacts fails

garden-io · Mar 26, 2020 · e4f78c8 · e4f78c8
1 parent f60ba6c
commit e4f78c8
Show file tree

Hide file tree

Showing 10 changed files with 178 additions and 1 deletion.
diff --git a/garden-service/src/plugins/kubernetes/run.ts b/garden-service/src/plugins/kubernetes/run.ts
@@ -245,7 +245,7 @@ export async function runAndCopy({
         result = await runner.exec({
           // Pipe the output from the command to the /tmp/output pipe, including stderr. Some shell voodoo happening
           // here, but this was the only working approach I could find after a lot of trial and error.
-          command: ["sh", "-c", `echo $(${cmd}) >>/tmp/output 2>&1`],
+          command: ["sh", "-c", `exec >/tmp/output; ${cmd}`],
           container: mainContainerName,
           ignoreError: true,
           log,

diff --git a/garden-service/test/data/test-projects/container/simple/garden.yml b/garden-service/test/data/test-projects/container/simple/garden.yml
@@ -18,6 +18,12 @@ tasks:
       - source: /task.txt
       - source: /task.txt
         target: subdir
+  - name: artifacts-task-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-task
     command: [sh, -c, "touch /task.txt && mkdir -p /tasks && touch /tasks/output.txt && echo ok"]
     artifacts:
@@ -38,6 +44,12 @@ tests:
       - source: /test.txt
       - source: /test.txt
         target: subdir
+  - name: artifacts-test-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-test
     command: [sh, -c, "touch /test.txt && mkdir -p /tests && touch /tests/output.txt && echo ok"]
     artifacts:

diff --git a/garden-service/test/data/test-projects/helm/artifacts/garden.yml b/garden-service/test/data/test-projects/helm/artifacts/garden.yml
@@ -17,6 +17,12 @@ tasks:
       - source: /task.txt
       - source: /task.txt
         target: subdir
+  - name: artifacts-task-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-task
     command: [sh, -c, "touch /task.txt && mkdir -p /tasks && touch /tasks/output.txt && echo ok"]
     artifacts:
@@ -32,6 +38,12 @@ tests:
       - source: /test.txt
       - source: /test.txt
         target: subdir
+  - name: artifacts-test-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-test
     command: [sh, -c, "touch /test.txt && mkdir -p /tests && touch /tests/output.txt && echo ok"]
     artifacts:

diff --git a/garden-service/test/data/test-projects/kubernetes-module/artifacts/garden.yml b/garden-service/test/data/test-projects/kubernetes-module/artifacts/garden.yml
@@ -33,6 +33,12 @@ tasks:
       - source: /task.txt
       - source: /task.txt
         target: subdir
+  - name: artifacts-task-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-task
     command: [sh, -c, "touch /task.txt && mkdir -p /tasks && touch /tasks/output.txt && echo ok"]
     artifacts:
@@ -48,6 +54,12 @@ tests:
       - source: /test.txt
       - source: /test.txt
         target: subdir
+  - name: artifacts-test-fail
+    command: [sh, -c, "touch /test.txt && exit 1"]
+    artifacts:
+      - source: /test.txt
+      - source: /test.txt
+        target: subdir
   - name: globs-test
     command: [sh, -c, "touch /test.txt && mkdir -p /tests && touch /tests/output.txt && echo ok"]
     artifacts:

diff --git a/garden-service/test/integ/src/plugins/kubernetes/container/container.ts b/garden-service/test/integ/src/plugins/kubernetes/container/container.ts
@@ -623,6 +623,31 @@ describe("kubernetes container module handlers", () => {
         expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
       })
 
+      it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+        const module = await graph.getModule("simple")
+
+        const testTask = new TestTask({
+          garden,
+          graph,
+          module,
+          testConfig: findByName(module.testConfigs, "artifacts-test-fail")!,
+          log: garden.log,
+          force: true,
+          forceBuild: false,
+          version: module.version,
+          _guard: true,
+        })
+
+        await emptyDir(garden.artifactsPath)
+
+        const results = await garden.processTasks([testTask], { throwOnError: false })
+
+        expect(results[testTask.getKey()]!.error).to.exist
+
+        expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+        expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+      })
+
       it("should handle globs when copying artifacts out of the container", async () => {
         const module = await graph.getModule("simple")
 

diff --git a/garden-service/test/integ/src/plugins/kubernetes/container/run.ts b/garden-service/test/integ/src/plugins/kubernetes/container/run.ts
@@ -160,6 +160,28 @@ describe("runContainerTask", () => {
       expect(await pathExists(join(garden.artifactsPath, "subdir", "task.txt"))).to.be.true
     })
 
+    it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+      const task = await graph.getTask("artifacts-task-fail")
+
+      const testTask = new TaskTask({
+        garden,
+        graph,
+        task,
+        log: garden.log,
+        force: true,
+        forceBuild: false,
+        version: task.module.version,
+      })
+      await emptyDir(garden.artifactsPath)
+
+      const results = await garden.processTasks([testTask], { throwOnError: false })
+
+      expect(results[testTask.getKey()]!.error).to.exist
+
+      expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+      expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+    })
+
     it("should handle globs when copying artifacts out of the container", async () => {
       const task = await graph.getTask("globs-task")
 

diff --git a/garden-service/test/integ/src/plugins/kubernetes/helm/run.ts b/garden-service/test/integ/src/plugins/kubernetes/helm/run.ts
@@ -177,6 +177,28 @@ describe("runHelmTask", () => {
       expect(await pathExists(join(garden.artifactsPath, "subdir", "task.txt"))).to.be.true
     })
 
+    it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+      const task = await graph.getTask("artifacts-task-fail")
+
+      const testTask = new TaskTask({
+        garden,
+        graph,
+        task,
+        log: garden.log,
+        force: true,
+        forceBuild: false,
+        version: task.module.version,
+      })
+      await emptyDir(garden.artifactsPath)
+
+      const results = await garden.processTasks([testTask], { throwOnError: false })
+
+      expect(results[testTask.getKey()]!.error).to.exist
+
+      expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+      expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+    })
+
     it("should handle globs when copying artifacts out of the container", async () => {
       const task = await graph.getTask("globs-task")
 

diff --git a/garden-service/test/integ/src/plugins/kubernetes/helm/test.ts b/garden-service/test/integ/src/plugins/kubernetes/helm/test.ts
@@ -128,6 +128,31 @@ describe("testHelmModule", () => {
       expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
     })
 
+    it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+      const module = await graph.getModule("artifacts")
+
+      const testTask = new TestTask({
+        garden,
+        graph,
+        module,
+        testConfig: findByName(module.testConfigs, "artifacts-test-fail")!,
+        log: garden.log,
+        force: true,
+        forceBuild: false,
+        version: module.version,
+        _guard: true,
+      })
+
+      await emptyDir(garden.artifactsPath)
+
+      const results = await garden.processTasks([testTask], { throwOnError: false })
+
+      expect(results[testTask.getKey()]!.error).to.exist
+
+      expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+      expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+    })
+
     it("should handle globs when copying artifacts out of the container", async () => {
       const module = await graph.getModule("artifacts")
 

diff --git a/garden-service/test/integ/src/plugins/kubernetes/kubernetes-module/run.ts b/garden-service/test/integ/src/plugins/kubernetes/kubernetes-module/run.ts
@@ -176,6 +176,28 @@ describe("runKubernetesTask", () => {
       expect(await pathExists(join(garden.artifactsPath, "subdir", "task.txt"))).to.be.true
     })
 
+    it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+      const task = await graph.getTask("artifacts-task-fail")
+
+      const testTask = new TaskTask({
+        garden,
+        graph,
+        task,
+        log: garden.log,
+        force: true,
+        forceBuild: false,
+        version: task.module.version,
+      })
+      await emptyDir(garden.artifactsPath)
+
+      const results = await garden.processTasks([testTask], { throwOnError: false })
+
+      expect(results[testTask.getKey()]!.error).to.exist
+
+      expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+      expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+    })
+
     it("should handle globs when copying artifacts out of the container", async () => {
       const task = await graph.getTask("globs-task")
 

diff --git a/garden-service/test/integ/src/plugins/kubernetes/kubernetes-module/test.ts b/garden-service/test/integ/src/plugins/kubernetes/kubernetes-module/test.ts
@@ -128,6 +128,31 @@ describe("testKubernetesModule", () => {
       expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
     })
 
+    it("should fail if an error occurs, but copy the artifacts out of the container", async () => {
+      const module = await graph.getModule("artifacts")
+
+      const testTask = new TestTask({
+        garden,
+        graph,
+        module,
+        testConfig: findByName(module.testConfigs, "artifacts-test-fail")!,
+        log: garden.log,
+        force: true,
+        forceBuild: false,
+        version: module.version,
+        _guard: true,
+      })
+
+      await emptyDir(garden.artifactsPath)
+
+      const results = await garden.processTasks([testTask], { throwOnError: false })
+
+      expect(results[testTask.getKey()]!.error).to.exist
+
+      expect(await pathExists(join(garden.artifactsPath, "test.txt"))).to.be.true
+      expect(await pathExists(join(garden.artifactsPath, "subdir", "test.txt"))).to.be.true
+    })
+
     it("should handle globs when copying artifacts out of the container", async () => {
       const module = await graph.getModule("artifacts")