From 225b718ec37099bdec06322e4a07e1471eed13b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20PORTAY?= Date: Tue, 3 Dec 2019 15:27:53 -0500 Subject: [PATCH 1/3] Reverse the exitcode initialization value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fakemachine is subject to panic and causes Debos to exit success due to the current logic of the exitcode. For example, the fakemachine function CopyFileTo() panics if the file is missing. In the case of a panic, the function never returns. Thus, the exitcode cannot be set to 1 and Debos exits with 0. This commit reverses the logic of the exitcode: it is initialized to 1 (i.e. failure), and it is set to 0 (i.e. success) only if Debos has reached the end of the things it has to do (or for the help message). Co-authored-by: Gaël PORTAY Co-authored-by: Santosh Mahto Signed-off-by: Gaël PORTAY --- cmd/debos/debos.go | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/cmd/debos/debos.go b/cmd/debos/debos.go index d7e1b5aa..a09baa3e 100644 --- a/cmd/debos/debos.go +++ b/cmd/debos/debos.go @@ -89,7 +89,7 @@ func main() { "no_proxy", } - var exitcode int = 0 + var exitcode int = 1 // Allow to run all deferred calls prior to os.Exit() defer func() { os.Exit(exitcode) @@ -101,24 +101,19 @@ func main() { args, err := parser.Parse() if err != nil { - flagsErr, ok := err.(*flags.Error) - if ok && flagsErr.Type == flags.ErrHelp { - return - } else { - exitcode = 1 - return + if flagsErr, ok := err.(*flags.Error); ok && flagsErr.Type == flags.ErrHelp { + exitcode = 0 } + return } if len(args) != 1 { log.Println("No recipe given!") - exitcode = 1 return } if options.DisableFakeMachine && options.Backend != "auto" { log.Println("--disable-fakemachine and --fakemachine-backend are mutually exclusive") - exitcode = 1 return } @@ -141,12 +136,10 @@ func main() { r := actions.Recipe{} if _, err := os.Stat(file); os.IsNotExist(err) { log.Println(err) - exitcode = 1 return } if err := r.Parse(file, options.PrintRecipe, options.Verbose, options.TemplateVars); err != nil { log.Println(err) - exitcode = 1 return } @@ -170,7 +163,6 @@ func main() { if options.Backend == "auto" { runInFakeMachine = false } else { - exitcode = 1 return } } @@ -234,13 +226,14 @@ func main() { for _, a := range r.Actions { err = a.Verify(&context) - if exitcode = checkError(&context, err, a, "Verify"); exitcode != 0 { + if ret := checkError(&context, err, a, "Verify"); ret != 0 { return } } if options.DryRun { log.Printf("==== Recipe done (Dry run) ====") + exitcode = 0 return } @@ -254,7 +247,6 @@ func main() { memsize, err := units.RAMInBytes(options.Memory) if err != nil { fmt.Printf("Couldn't parse memory size: %v\n", err) - exitcode = 1 return } m.SetMemory(int(memsize / 1024 / 1024)) @@ -269,7 +261,6 @@ func main() { size, err := units.FromHumanSize(options.ScratchSize) if err != nil { fmt.Printf("Couldn't parse scratch size: %v\n", err) - exitcode = 1 return } m.SetScratch(size, "") @@ -311,30 +302,32 @@ func main() { defer a.PostMachineCleanup(&context) err = a.PreMachine(&context, m, &args) - if exitcode = checkError(&context, err, a, "PreMachine"); exitcode != 0 { + if ret := checkError(&context, err, a, "PreMachine"); ret != 0 { return } } - exitcode, err = m.RunInMachineWithArgs(args) + var status int + status, err = m.RunInMachineWithArgs(args) if err != nil { fmt.Println(err) return } - if exitcode != 0 { + if status != 0 { context.State = debos.Failed return } for _, a := range r.Actions { err = a.PostMachine(&context) - if exitcode = checkError(&context, err, a, "Postmachine"); exitcode != 0 { + if ret := checkError(&context, err, a, "Postmachine"); ret != 0 { return } } log.Printf("==== Recipe done ====") + exitcode = 0 return } @@ -344,7 +337,7 @@ func main() { defer a.PostMachineCleanup(&context) err = a.PreNoMachine(&context) - if exitcode = checkError(&context, err, a, "PreNoMachine"); exitcode != 0 { + if err := checkError(&context, err, a, "PreNoMachine"); err != 0 { return } } @@ -354,23 +347,28 @@ func main() { if _, err = os.Stat(context.Rootdir); os.IsNotExist(err) { err = os.Mkdir(context.Rootdir, 0755) if err != nil && os.IsNotExist(err) { - exitcode = 1 return } } - exitcode = do_run(r, &context) - if exitcode != 0 { + if ret := do_run(r, &context); ret != 0 { return } if !fakemachine.InMachine() { for _, a := range r.Actions { err = a.PostMachine(&context) - if exitcode = checkError(&context, err, a, "PostMachine"); exitcode != 0 { + if err := checkError(&context, err, a, "PostMachine"); err != 0 { return } } log.Printf("==== Recipe done ====") } + + // We reach this point when we have had no errors, and are not + // responsible for spawning a fakemachine to run the + // recipe. This is true both when this instance of debos is + // itself running in a fakemachine, and when no fakemachine is + // being used at all. + exitcode = 0 } From 512757d45f816cbce5a6107619455b44967e1d3b Mon Sep 17 00:00:00 2001 From: Ed Smith Date: Sat, 14 Aug 2021 13:19:57 +0100 Subject: [PATCH 2/3] Add exit code test to validate logic is as expected --- tests/exit_test/bad.yaml | 5 ++ tests/exit_test/exit_test.sh | 88 +++++++++++++++++++++++ tests/exit_test/garbled.yaml | 4 ++ tests/exit_test/good.yaml | 5 ++ tests/exit_test/post-machine-failure.yaml | 6 ++ tests/exit_test/pre-machine-failure.yaml | 8 +++ tests/exit_test/verify-fail.yaml | 6 ++ 7 files changed, 122 insertions(+) create mode 100644 tests/exit_test/bad.yaml create mode 100755 tests/exit_test/exit_test.sh create mode 100644 tests/exit_test/garbled.yaml create mode 100644 tests/exit_test/good.yaml create mode 100644 tests/exit_test/post-machine-failure.yaml create mode 100644 tests/exit_test/pre-machine-failure.yaml create mode 100644 tests/exit_test/verify-fail.yaml diff --git a/tests/exit_test/bad.yaml b/tests/exit_test/bad.yaml new file mode 100644 index 00000000..7e7da779 --- /dev/null +++ b/tests/exit_test/bad.yaml @@ -0,0 +1,5 @@ +architecture: amd64 + +actions: + - action: run + command: exit 1 diff --git a/tests/exit_test/exit_test.sh b/tests/exit_test/exit_test.sh new file mode 100755 index 00000000..dc6c74a4 --- /dev/null +++ b/tests/exit_test/exit_test.sh @@ -0,0 +1,88 @@ +#!/bin/bash + +TEST=0 +FAILURES=0 +VERBOSE=1 + +function test_failed { + local MSG="$1" + TEST=$(($TEST + 1)) + FAILURES=$(($FAILURES + 1)) + echo "Test ${TEST}: ${MSG}" +} + +function test_passed { + TEST=$(($TEST + 1)) + echo "Test ${TEST}: PASS" +} + +function run_cmd { + local CMD="$@" + echo + echo "Running ${CMD}" + if [[ $VERBOSE == 0 ]]; then + $CMD &>/dev/null + else + $CMD + fi + return $? +} + +function expect_success { + local CMD="$@" + run_cmd $CMD && test_passed || test_failed "${CMD} failed with exitcode $?, expected success" +} + +function expect_failure { + local CMD="$@" + run_cmd $CMD && test_failed "${CMD} succeeded, failure expected." || test_passed +} + +function rename_command { + newname="$1" + shift + (exec -a "$newname" "$@") + return $? +} + +if [ -v sudo ]; then + SUDO=sudo +else + SUDO= +fi + +expect_success debos --help +expect_failure debos --not-a-valid-option +expect_failure debos +expect_failure debos good.yaml good.yaml +expect_failure debos --disable-fakemachine --fakemachine-backend=uml good.yaml +expect_failure debos non-existent-file.yaml +expect_failure debos garbled.yaml +expect_failure debos --fakemachine-backend=kvm good.yaml +expect_failure debos verify-fail.yaml +expect_success debos --dry-run good.yaml +expect_failure debos --memory=NotANumber good.yaml +expect_failure debos --scratchsize=NotANumber good.yaml +expect_success debos good.yaml +expect_failure debos bad.yaml +expect_failure debos pre-machine-failure.yaml +expect_failure debos post-machine-failure.yaml +expect_failure rename_command NOT_DEBOS debos good.yaml + +expect_failure $SUDO debos non-existent-file.yaml --disable-fakemachine +expect_failure $SUDO debos garbled.yaml --disable-fakemachine +expect_failure $SUDO debos verify-fail.yaml --disable-fakemachine +expect_success $SUDO debos --dry-run good.yaml --disable-fakemachine +expect_success $SUDO debos good.yaml --disable-fakemachine +expect_failure $SUDO debos bad.yaml --disable-fakemachine +expect_failure $SUDO debos pre-machine-failure.yaml --disable-fakemachine +expect_failure $SUDO debos post-machine-failure.yaml --disable-fakemachine + +echo +if [[ $FAILURES -ne 0 ]]; then + SUCCESSES=$(( $TEST - $FAILURES )) + echo "Error: Only $SUCCESSES/$TEST tests passed" + exit 1 +fi + +echo "All tests passed" diff --git a/tests/exit_test/garbled.yaml b/tests/exit_test/garbled.yaml new file mode 100644 index 00000000..a42aa053 --- /dev/null +++ b/tests/exit_test/garbled.yaml @@ -0,0 +1,4 @@ +This isn't + ::: + + very good YAML diff --git a/tests/exit_test/good.yaml b/tests/exit_test/good.yaml new file mode 100644 index 00000000..49005565 --- /dev/null +++ b/tests/exit_test/good.yaml @@ -0,0 +1,5 @@ +architecture: amd64 + +actions: + - action: run + command: exit 0 diff --git a/tests/exit_test/post-machine-failure.yaml b/tests/exit_test/post-machine-failure.yaml new file mode 100644 index 00000000..1a73a591 --- /dev/null +++ b/tests/exit_test/post-machine-failure.yaml @@ -0,0 +1,6 @@ +architecture: amd64 + +actions: + - action: run + command: exit 1 + postprocess: true diff --git a/tests/exit_test/pre-machine-failure.yaml b/tests/exit_test/pre-machine-failure.yaml new file mode 100644 index 00000000..96743a5d --- /dev/null +++ b/tests/exit_test/pre-machine-failure.yaml @@ -0,0 +1,8 @@ +architecture: amd64 + +actions: + - action: image-partition + imagename: test + imagesize: 500000000000000TB + partitiontype: gpt + diff --git a/tests/exit_test/verify-fail.yaml b/tests/exit_test/verify-fail.yaml new file mode 100644 index 00000000..4f90077d --- /dev/null +++ b/tests/exit_test/verify-fail.yaml @@ -0,0 +1,6 @@ +architecture: amd64 + +actions: + - action: unpack + compression: gz + file: ~ From d1b566034b06f0a1baa8f07ceefb384de5f070e1 Mon Sep 17 00:00:00 2001 From: Ed Smith Date: Mon, 16 Aug 2021 15:24:14 +0100 Subject: [PATCH 3/3] Capture the exitcode test as a docker compose recipe --- docker/exitcode-test.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 docker/exitcode-test.yaml diff --git a/docker/exitcode-test.yaml b/docker/exitcode-test.yaml new file mode 100644 index 00000000..3578c3be --- /dev/null +++ b/docker/exitcode-test.yaml @@ -0,0 +1,21 @@ +version: '3.6' + +services: + sut: + build: + context: .. + dockerfile: docker/Dockerfile + volumes: + - type: bind + source: ../tests/exit_test + target: /recipes + tmpfs: + - /scratch:exec + environment: + - TMP=/scratch + cap_add: + - SYS_PTRACE + security_opt: + - label:disable + working_dir: /recipes/ + entrypoint: ./exit_test.sh