runsc/do: do a proper cleanup if a command failed due to internal errors

Fatalf calls os.Exit and a process exits without calling defer callbacks.

Should we do this for other runsc commands?

PiperOrigin-RevId: 249776310
Change-Id: If9d8b54d0ae37db443895906eb33bd9e9b600cc9

2 files changed, 17 insertions, 10 deletions

diff --git a/runsc/cmd/cmd.go b/runsc/cmd/cmd.go
index aa7b1a636..a2fc377d1 100644
--- a/runsc/cmd/cmd.go
+++ b/runsc/cmd/cmd.go
@@ -22,19 +22,26 @@ import (
 	"strconv"
 	"syscall"
 
+	"github.com/google/subcommands"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"gvisor.googlesource.com/gvisor/pkg/log"
 	"gvisor.googlesource.com/gvisor/runsc/specutils"
 )
 
-// Fatalf logs to stderr and exits with a failure status code.
-func Fatalf(s string, args ...interface{}) {
+// Errorf logs to stderr and returns subcommands.ExitFailure.
+func Errorf(s string, args ...interface{}) subcommands.ExitStatus {
 	// If runsc is being invoked by docker or cri-o, then we might not have
 	// access to stderr, so we log a serious-looking warning in addition to
 	// writing to stderr.
 	log.Warningf("FATAL ERROR: "+s, args...)
 	fmt.Fprintf(os.Stderr, s+"\n", args...)
 	// Return an error that is unlikely to be used by the application.
+	return subcommands.ExitFailure
+}
+
+// Fatalf logs to stderr and exits with a failure status code.
+func Fatalf(s string, args ...interface{}) {
+	Errorf(s, args...)
 	os.Exit(128)
 }
 
diff --git a/runsc/cmd/do.go b/runsc/cmd/do.go
index c5e72f32b..425db8efe 100644
--- a/runsc/cmd/do.go
+++ b/runsc/cmd/do.go
@@ -89,16 +89,16 @@ func (c *Do) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) su
 
 	hostname, err := os.Hostname()
 	if err != nil {
-		Fatalf("Error to retrieve hostname: %v", err)
+		return Errorf("Error to retrieve hostname: %v", err)
 	}
 
 	absRoot, err := resolvePath(c.root)
 	if err != nil {
-		Fatalf("Error resolving root: %v", err)
+		return Errorf("Error resolving root: %v", err)
 	}
 	absCwd, err := resolvePath(c.cwd)
 	if err != nil {
-		Fatalf("Error resolving current directory: %v", err)
+		return Errorf("Error resolving current directory: %v", err)
 	}
 
 	spec := &specs.Spec{
@@ -121,18 +121,18 @@ func (c *Do) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) su
 	if conf.Network != boot.NetworkNone {
 		clean, err := c.setupNet(cid, spec)
 		if err != nil {
-			Fatalf("Error setting up network: %v", err)
+			return Errorf("Error setting up network: %v", err)
 		}
 		defer clean()
 	}
 
 	out, err := json.Marshal(spec)
 	if err != nil {
-		Fatalf("Error to marshal spec: %v", err)
+		return Errorf("Error to marshal spec: %v", err)
 	}
 	tmpDir, err := ioutil.TempDir("", "runsc-do")
 	if err != nil {
-		Fatalf("Error to create tmp dir: %v", err)
+		return Errorf("Error to create tmp dir: %v", err)
 	}
 	defer os.RemoveAll(tmpDir)
 
@@ -141,12 +141,12 @@ func (c *Do) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) su
 
 	cfgPath := filepath.Join(tmpDir, "config.json")
 	if err := ioutil.WriteFile(cfgPath, out, 0755); err != nil {
-		Fatalf("Error write spec: %v", err)
+		return Errorf("Error write spec: %v", err)
 	}
 
 	ws, err := container.Run(cid, spec, conf, tmpDir, "", "", "", false)
 	if err != nil {
-		Fatalf("running container: %v", err)
+		return Errorf("running container: %v", err)
 	}
 
 	*waitStatus = ws