runtime: optimize code for managing temp users for rootless mode

This commit does two chagnes:

- move code for managing temp users to rootless.go.
- use common function in qemu.go when shutdown the VM.

Fixes: #2759

Signed-off-by: bin <bin@hyper.sh>

src/runtime/pkg/containerd-shim-v2/create.go

@@ -10,9 +10,6 @@ package containerdshim
 import (
 	"context"
 	"fmt"
-	"github.com/kata-containers/kata-containers/src/runtime/pkg/utils"
-	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless"
-	"math/rand"
 	"os"
 	"os/user"
 	"path"
@@ -24,6 +21,8 @@ import (
 	"github.com/containerd/containerd/mount"
 	taskAPI "github.com/containerd/containerd/runtime/v2/task"
 	"github.com/containerd/typeurl"
+	"github.com/kata-containers/kata-containers/src/runtime/pkg/utils"
+	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 
@@ -274,13 +273,15 @@ func doMount(mounts []*containerd_types.Mount, rootfs string) error {
 }
 
 func configureNonRootHypervisor(runtimeConfig *oci.RuntimeConfig) error {
-	userName, err := createVmmUser()
+	userName, err := utils.CreateVmmUser()
 	if err != nil {
 		return err
 	}
 	defer func() {
 		if err != nil {
-			removeVmmUser(userName)
+			if err2 := utils.RemoveVmmUser(userName); err2 != nil {
+				shimLog.WithField("userName", userName).WithError(err).Warn("failed to remove user")
+			}
 		}
 	}()
 
@@ -336,48 +337,3 @@ func configureNonRootHypervisor(runtimeConfig *oci.RuntimeConfig) error {
 	}
 	return fmt.Errorf("failed to get the gid of /dev/kvm")
 }
-
-func createVmmUser() (string, error) {
-	var (
-		err      error
-		userName string
-	)
-
-	useraddPath, err := utils.FirstValidExecutable([]string{"/usr/sbin/useradd", "/sbin/useradd", "/bin/useradd"})
-	if err != nil {
-		return "", err
-	}
-	nologinPath, err := utils.FirstValidExecutable([]string{"/usr/sbin/nologin", "/sbin/nologin", "/bin/nologin"})
-	if err != nil {
-		return "", err
-	}
-
-	// Add retries to mitigate temporary errors and race conditions. For example, the user already exists
-	// or another instance of the runtime is also creating a user.
-	maxAttempt := 5
-	for i := 0; i < maxAttempt; i++ {
-		userName = fmt.Sprintf("kata-%v", rand.Intn(100000))
-		_, err = utils.RunCommand([]string{useraddPath, "-M", "-s", nologinPath, userName, "-c", "\"Kata Containers temporary hypervisor user\""})
-		if err == nil {
-			return userName, nil
-		}
-		shimLog.WithField("attempt", i+1).WithField("username", userName).
-			WithError(err).Warn("failed to add user, will try again")
-	}
-	return "", fmt.Errorf("could not create VMM user: %v", err)
-}
-
-func removeVmmUser(user string) {
-	userdelPath, err := utils.FirstValidExecutable([]string{"/usr/sbin/userdel", "/sbin/userdel", "/bin/userdel"})
-	if err != nil {
-		shimLog.WithField("username", user).WithError(err).Warn("failed to remove user")
-	}
-	// Add retries to mitigate temporary errors and race conditions.
-	for i := 0; i < 5; i++ {
-		_, err := utils.RunCommand([]string{userdelPath, "-f", user})
-		if err == nil {
-			return
-		}
-		shimLog.WithField("username", user).WithField("attempt", i+1).WithError(err).Warn("failed to remove user")
-	}
-}

src/runtime/pkg/utils/utils.go

@@ -7,17 +7,22 @@ package utils
 
 import (
 	"fmt"
+	"math/rand"
 	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+
+	"github.com/sirupsen/logrus"
 )
 
 const (
 	acceptEncodingHeader = "Accept-Encoding"
 )
 
+var utilsLog = logrus.WithFields(logrus.Fields{"source": "pkg/utils"})
+
 // GzipAccepted returns whether the client will accept gzip-encoded content.
 func GzipAccepted(header http.Header) bool {
 	a := header.Get(acceptEncodingHeader)
@@ -99,3 +104,52 @@ func FirstValidExecutable(paths []string) (string, error) {
 	}
 	return "", fmt.Errorf("all the executables are invalid")
 }
+
+// CreateVmmUser create a temp user for running Kata Containers under rootless mode.
+func CreateVmmUser() (string, error) {
+	var (
+		err      error
+		userName string
+	)
+
+	useraddPath, err := FirstValidExecutable([]string{"/usr/sbin/useradd", "/sbin/useradd", "/bin/useradd"})
+	if err != nil {
+		return "", err
+	}
+	nologinPath, err := FirstValidExecutable([]string{"/usr/sbin/nologin", "/sbin/nologin", "/bin/nologin"})
+	if err != nil {
+		return "", err
+	}
+
+	// Add retries to mitigate temporary errors and race conditions. For example, the user already exists
+	// or another instance of the runtime is also creating a user.
+	maxAttempt := 5
+	for i := 0; i < maxAttempt; i++ {
+		userName = fmt.Sprintf("kata-%v", rand.Intn(100000))
+		_, err = RunCommand([]string{useraddPath, "-M", "-s", nologinPath, userName, "-c", "\"Kata Containers temporary hypervisor user\""})
+		if err == nil {
+			return userName, nil
+		}
+		utilsLog.WithField("attempt", i+1).WithField("username", userName).
+			WithError(err).Warn("failed to add user, will try again")
+	}
+	return "", fmt.Errorf("could not create VMM user: %v", err)
+}
+
+// RemoveVmmUser delete user created by CreateVmmUser.
+func RemoveVmmUser(user string) error {
+	userdelPath, err := FirstValidExecutable([]string{"/usr/sbin/userdel", "/sbin/userdel", "/bin/userdel"})
+	if err != nil {
+		utilsLog.WithField("username", user).WithError(err).Warn("failed to remove user")
+		return err
+	}
+
+	// Add retries to mitigate temporary errors and race conditions.
+	for i := 0; i < 5; i++ {
+		if _, err = RunCommand([]string{userdelPath, "-f", user}); err == nil {
+			return nil
+		}
+		utilsLog.WithField("username", user).WithField("attempt", i+1).WithError(err).Warn("failed to remove user")
+	}
+	return err
+}

src/runtime/virtcontainers/pkg/rootless/rootless.go

@@ -23,13 +23,14 @@ import (
 	"context"
 	"crypto/rand"
 	"fmt"
-	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
 
+	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils"
+
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/sirupsen/logrus"

src/runtime/virtcontainers/qemu.go

@@ -11,7 +11,6 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless"
 	"io/ioutil"
 	"math"
 	"os"
@@ -24,6 +23,8 @@ import (
 	"time"
 	"unsafe"
 
+	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless"
+
 	govmmQemu "github.com/kata-containers/govmm/qemu"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
@@ -1026,13 +1027,8 @@ func (q *qemu) cleanupVM() error {
 			q.Logger().WithError(err).WithField("uid", q.config.Uid).Warn("failed to find the user")
 			return nil
 		}
-		userdelPath, err := pkgUtils.FirstValidExecutable([]string{"/usr/sbin/userdel", "/sbin/userdel", "/bin/userdel"})
+
-		if err != nil {
+		if err := pkgUtils.RemoveVmmUser(u.Username); err != nil {
-			q.Logger().WithError(err).WithField("user", u.Username).Warn("failed to delete the user")
-			return nil
-		}
-		_, err = pkgUtils.RunCommand([]string{userdelPath, "-f", u.Username})
-		if err != nil {
 			q.Logger().WithError(err).WithField("user", u.Username).Warn("failed to delete the user")
 		}
 	}