From e4f78c88b8a14fe7743b2a30d9b677292d39c89b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Thu, 26 Mar 2020 14:20:53 +0100 Subject: [PATCH] fix(k8s): ensure non-zero exit code if test/task with artifacts fails --- garden-service/src/plugins/kubernetes/run.ts | 2 +- .../test-projects/container/simple/garden.yml | 12 +++++++++ .../test-projects/helm/artifacts/garden.yml | 12 +++++++++ .../kubernetes-module/artifacts/garden.yml | 12 +++++++++ .../plugins/kubernetes/container/container.ts | 25 +++++++++++++++++++ .../src/plugins/kubernetes/container/run.ts | 22 ++++++++++++++++ .../integ/src/plugins/kubernetes/helm/run.ts | 22 ++++++++++++++++ .../integ/src/plugins/kubernetes/helm/test.ts | 25 +++++++++++++++++++ .../kubernetes/kubernetes-module/run.ts | 22 ++++++++++++++++ .../kubernetes/kubernetes-module/test.ts | 25 +++++++++++++++++++ 10 files changed, 178 insertions(+), 1 deletion(-) diff --git a/garden-service/src/plugins/kubernetes/run.ts b/garden-service/src/plugins/kubernetes/run.ts index 2071823590..ec02b72fd1 100644 --- 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 index c40822fba6..c841409761 100644 --- 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 index 9f16b63057..3573399b77 100644 --- 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 index 3e03451405..99a4571403 100644 --- 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 index 48c2be0743..1918bfa6a7 100644 --- 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 index 3ac9b28c8e..25cc6ce505 100644 --- 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 index c01998c4dd..f0ed458df4 100644 --- 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 index ef079eb032..a844f4a9f2 100644 --- 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 index 3e70232951..2daa122d26 100644 --- 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 index e60845de21..c820090909 100644 --- 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")