From 9d2b10726561af7616b26ee6568b42333290da4b Mon Sep 17 00:00:00 2001 From: Christopher Obbard Date: Fri, 7 Jan 2022 11:06:17 +0000 Subject: [PATCH] Refactor the error handling Currently the debos error handling sets an exitcode during running, then exits with that code. Since we only ever exit with a 1, unless an error occurs while running fakemachine, let's switch around the error handling to use the context.State and check the state after running. [TODO: THIS SHOULD BE A SEPERATE COMMIT] While we are here, also rework the fakemachine error handling to show that an error occured during run. Signed-off-by: Christopher Obbard --- cmd/debos/debos.go | 64 ++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/cmd/debos/debos.go b/cmd/debos/debos.go index 0f9d6a16..c3263e0f 100644 --- a/cmd/debos/debos.go +++ b/cmd/debos/debos.go @@ -15,18 +15,18 @@ import ( "github.com/jessevdk/go-flags" ) -func checkError(context *debos.DebosContext, err error, a debos.Action, stage string) int { +func checkError(context *debos.DebosContext, err error, a debos.Action, stage string) bool { if err == nil { - return 0 + return true } context.State = debos.Failed log.Printf("Action `%s` failed at stage %s, error: %s", a, stage, err) debos.DebugShell(*context) - return 1 + return false } -func do_run(r actions.Recipe, context *debos.DebosContext) int { +func do_run(r actions.Recipe, context *debos.DebosContext) bool { for _, a := range r.Actions { err := a.Run(context) @@ -35,12 +35,12 @@ func do_run(r actions.Recipe, context *debos.DebosContext) int { defer a.Cleanup(context) // Check the state of Run method - if exitcode := checkError(context, err, a, "Run"); exitcode != 0 { - return exitcode + if !checkError(context, err, a, "Run") { + return false } } - return 0 + return true } func warnLocalhost(variable string, value string) { @@ -88,11 +88,12 @@ func main() { "no_proxy", } - var exitcode int = 0 // Allow to run all deferred calls prior to os.Exit() - defer func() { - os.Exit(exitcode) - }() + defer func(context debos.DebosContext) { + if context.State == debos.Failed { + os.Exit(1) + } + }(context) parser := flags.NewParser(&options, flags.Default) fakemachineBackends := parser.FindOptionByLongName("fakemachine-backend") @@ -104,20 +105,20 @@ func main() { if ok && flagsErr.Type == flags.ErrHelp { return } else { - exitcode = 1 + context.State = debos.Failed return } } if len(args) != 1 { log.Println("No recipe given!") - exitcode = 1 + context.State = debos.Failed return } if options.DisableFakeMachine && options.Backend != "auto" { log.Println("--disable-fakemachine and --fakemachine-backend are mutually exclusive") - exitcode = 1 + context.State = debos.Failed return } @@ -140,12 +141,12 @@ func main() { r := actions.Recipe{} if _, err := os.Stat(file); os.IsNotExist(err) { log.Println(err) - exitcode = 1 + context.State = debos.Failed return } if err := r.Parse(file, options.PrintRecipe, options.Verbose, options.TemplateVars); err != nil { log.Println(err) - exitcode = 1 + context.State = debos.Failed return } @@ -162,14 +163,14 @@ func main() { // attempt to create a fakemachine m, err = fakemachine.NewMachineWithBackend(options.Backend) if err != nil { - log.Printf("error creating fakemachine: %v", err) + log.Printf("Couldn't create fakemachine: %v", err) /* fallback to running on the host unless the user has chosen * a specific backend */ if options.Backend == "auto" { runInFakeMachine = false } else { - exitcode = 1 + context.State = debos.Failed return } } @@ -233,7 +234,7 @@ func main() { for _, a := range r.Actions { err = a.Verify(&context) - if exitcode = checkError(&context, err, a, "Verify"); exitcode != 0 { + if !checkError(&context, err, a, "Verify") { return } } @@ -253,7 +254,7 @@ func main() { memsize, err := units.RAMInBytes(options.Memory) if err != nil { fmt.Printf("Couldn't parse memory size: %v\n", err) - exitcode = 1 + context.State = debos.Failed return } m.SetMemory(int(memsize / 1024 / 1024)) @@ -268,7 +269,7 @@ func main() { size, err := units.FromHumanSize(options.ScratchSize) if err != nil { fmt.Printf("Couldn't parse scratch size: %v\n", err) - exitcode = 1 + context.State = debos.Failed return } m.SetScratch(size, "") @@ -310,25 +311,26 @@ func main() { defer a.PostMachineCleanup(&context) err = a.PreMachine(&context, m, &args) - if exitcode = checkError(&context, err, a, "PreMachine"); exitcode != 0 { + if !checkError(&context, err, a, "PreMachine") { return } } - exitcode, err = m.RunInMachineWithArgs(args) + exitcode, err := m.RunInMachineWithArgs(args) if err != nil { - fmt.Println(err) + fmt.Printf("Couldn't start fakemachine: %v\n", err) + context.State = debos.Failed return } - if exitcode != 0 { + fmt.Printf("fakemachine failed with non-zero exitcode: %d\n", exitcode) context.State = debos.Failed return } for _, a := range r.Actions { err = a.PostMachine(&context) - if exitcode = checkError(&context, err, a, "Postmachine"); exitcode != 0 { + if !checkError(&context, err, a, "Postmachine") { return } } @@ -343,7 +345,7 @@ func main() { defer a.PostMachineCleanup(&context) err = a.PreNoMachine(&context) - if exitcode = checkError(&context, err, a, "PreNoMachine"); exitcode != 0 { + if !checkError(&context, err, a, "PreNoMachine") { return } } @@ -353,20 +355,20 @@ 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 + fmt.Printf("Could not create rootdir: %v\n", err) + context.State = debos.Failed return } } - exitcode = do_run(r, &context) - if exitcode != 0 { + if !do_run(r, &context) { return } if !fakemachine.InMachine() { for _, a := range r.Actions { err = a.PostMachine(&context) - if exitcode = checkError(&context, err, a, "PostMachine"); exitcode != 0 { + if !checkError(&context, err, a, "PostMachine") { return } }