Skip to content

Commit

Permalink
Reverse the exitcode initialization value
Browse files Browse the repository at this point in the history
 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).

 Signed-off-by: Gaël PORTAY <[email protected]>
  • Loading branch information
santoshmahto7 committed Jul 15, 2021
1 parent 2e004a2 commit d0f1d97
Showing 1 changed file with 15 additions and 19 deletions.
34 changes: 15 additions & 19 deletions cmd/debos/debos.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,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)
Expand All @@ -102,22 +102,20 @@ func main() {
if err != nil {
flagsErr, ok := err.(*flags.Error)
if ok && flagsErr.Type == flags.ErrHelp {
exitcode = 0
return
} else {
exitcode = 1

This comment has been minimized.

Copy link
@evelikov

evelikov Jul 15, 2021

Nit: personally I'm a fan of using a "happy" path and avoiding the else-after-return statements - see example below. Note that gofmt is the king dictating the formatting, take its suggestion with higher priority than mine.

Example:

ret := foo()
if ret != nil { // aka we hit an error path
  do_something()
  return;
}
continue_with_normal_codepath() // normal codeflow stays on the same indentation level, reducing the cognitive burden
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
}

Expand All @@ -140,12 +138,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
}

Expand All @@ -169,7 +165,6 @@ func main() {
if options.Backend == "auto" {
runInFakeMachine = false
} else {
exitcode = 1
return
}
}
Expand Down Expand Up @@ -233,13 +228,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
}

Expand All @@ -253,7 +249,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))
Expand All @@ -268,7 +263,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, "")
Expand Down Expand Up @@ -310,30 +304,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
}

Expand All @@ -343,7 +339,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
}
}
Expand All @@ -353,23 +349,23 @@ 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
}
}

exitcode = 0
log.Printf("==== Recipe done ====")
}
}

1 comment on commit d0f1d97

@evelikov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to amend the patch and reinstate Gaël as author, unless you've made substantial changes. If so, add a small note/summary in the commit message.

With the authorship fixed, regardless of the style suggestion, this patch is

Reviewed-by: Emil Velikov <[email protected]>

Please sign in to comment.