Merge pull request kata-containers#605 from jodh-intel/improve-ut-cov…

…erage

Improve unit test coverage

agent.go

@@ -42,14 +42,18 @@ import (
 
 const (
 	procCgroups = "/proc/cgroups"
-	meminfo     = "/proc/meminfo"
 
 	bashPath         = "/bin/bash"
 	shPath           = "/bin/sh"
 	debugConsolePath = "/dev/console"
 )
 
 var (
+	// List of shells that are tried (in order) to setup a debug console
+	supportedShells = []string{bashPath, shPath}
+
+	meminfo = "/proc/meminfo"
+
 	// cgroup fs is mounted at /sys/fs when systemd is the init process
 	sysfsDir                     = "/sys"
 	cgroupPath                   = sysfsDir + "/fs/cgroup"
@@ -390,11 +394,11 @@ func (s *sandbox) removeSandboxStorage(path string) error {
 
 	err := removeMounts([]string{path})
 	if err != nil {
-		return grpcStatus.Errorf(codes.Unknown, "Unable to unmount sandbox storage path %s", path)
+		return grpcStatus.Errorf(codes.Unknown, "Unable to unmount sandbox storage path %s: %v", path, err)
 	}
 	err = os.RemoveAll(path)
 	if err != nil {
-		return grpcStatus.Errorf(codes.Unknown, "Unable to delete sandbox storage path %s", path)
+		return grpcStatus.Errorf(codes.Unknown, "Unable to delete sandbox storage path %s: %v", path, err)
 	}
 	return nil
 }
@@ -410,21 +414,18 @@ func (s *sandbox) unsetAndRemoveSandboxStorage(path string) error {
 	span.SetTag("path", path)
 	defer span.Finish()
 
-	if _, ok := s.storages[path]; ok {
-		removeSbs, err := s.unSetSandboxStorage(path)
-		if err != nil {
-			return err
-		}
+	removeSbs, err := s.unSetSandboxStorage(path)
+	if err != nil {
+		return err
+	}
 
-		if removeSbs {
-			if err := s.removeSandboxStorage(path); err != nil {
-				return err
-			}
+	if removeSbs {
+		if err := s.removeSandboxStorage(path); err != nil {
+			return err
 		}
-
-		return nil
 	}
-	return grpcStatus.Errorf(codes.NotFound, "Sandbox storage with path %s not found", path)
+
+	return nil
 }
 
 func (s *sandbox) getContainer(id string) (*container, error) {
@@ -843,6 +844,9 @@ func getMemory() (string, error) {
 		}
 
 		memTotal := strings.TrimSpace(fields[1])
+		if memTotal == "" {
+			return "", fmt.Errorf("cannot determine total memory from line %q", line)
+		}
 
 		return memTotal, nil
 	}
@@ -1173,9 +1177,9 @@ func mountToRootfs(m initMount) error {
 		return err
 	}
 
-	if flags, options, err := parseMountFlagsAndOptions(m.options); err != nil {
-		return grpcStatus.Errorf(codes.Internal, "Could not parseMountFlagsAndOptions(%v)", m.options)
-	} else if err := syscall.Mount(m.src, m.dest, m.fstype, uintptr(flags), options); err != nil {
+	flags, options := parseMountFlagsAndOptions(m.options)
+
+	if err := syscall.Mount(m.src, m.dest, m.fstype, uintptr(flags), options); err != nil {
 		return grpcStatus.Errorf(codes.Internal, "Could not mount %v to %v: %v", m.src, m.dest, err)
 	}
 	return nil
@@ -1210,13 +1214,13 @@ func cgroupsMount() error {
 	return ioutil.WriteFile(cgroupMemoryUseHierarchyPath, []byte{'1'}, cgroupMemoryUseHierarchyMode)
 }
 
-func setupDebugConsole() error {
+func setupDebugConsole(ctx context.Context, debugConsolePath string) error {
 	if !debugConsole {
 		return nil
 	}
 
 	var shellPath string
-	for _, s := range []string{bashPath, shPath} {
+	for _, s := range supportedShells {
 		var err error
 		if _, err = os.Stat(s); err == nil {
 			shellPath = s
@@ -1226,7 +1230,7 @@ func setupDebugConsole() error {
 	}
 
 	if shellPath == "" {
-		return errors.New("Shell not found")
+		return fmt.Errorf("No available shells (checked %v)", supportedShells)
 	}
 
 	cmd := exec.Command(shellPath)
@@ -1250,9 +1254,15 @@ func setupDebugConsole() error {
 
 	go func() {
 		for {
-			dcmd := *cmd
-			if err := dcmd.Run(); err != nil {
-				agentLog.WithError(err).Warn("failed to start debug console")
+			select {
+			case <-ctx.Done():
+				// stop the thread
+				return
+			default:
+				dcmd := *cmd
+				if err := dcmd.Run(); err != nil {
+					agentLog.WithError(err).Warn("failed to start debug console")
+				}
 			}
 		}
 	}()
@@ -1335,15 +1345,15 @@ func realMain() error {
 		return fmt.Errorf("failed to setup logger: %v", err)
 	}
 
-	if err := setupDebugConsole(); err != nil {
-		agentLog.WithError(err).Error("failed to setup debug console")
-	}
-
 	rootSpan, rootContext, err = setupTracing(agentName)
 	if err != nil {
 		return fmt.Errorf("failed to setup tracing: %v", err)
 	}
 
+	if err := setupDebugConsole(rootContext, debugConsolePath); err != nil {
+		agentLog.WithError(err).Error("failed to setup debug console")
+	}
+
 	// Set the sandbox context now that the context contains the tracing
 	// information.
 	s.ctx = rootContext