Move chroot from multus main process to its child processes

We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issue, when facing heavy CNI_ADD/CNI_DEL requests. With this patch, we do chroot in the child processes instead. So file operations in the main process will not affect by chroot. Signed-off-by: Peng Liu <[email protected]>
k8snetworkplumbingwg · Sep 21, 2023 · 5ca3a2e · 5ca3a2e
1 parent 857d070
commit 5ca3a2e
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 107 deletions.
diff --git a/pkg/k8sclient/k8sclient.go b/pkg/k8sclient/k8sclient.go
@@ -286,11 +286,6 @@ func getKubernetesDelegate(client *ClientInfo, net *types.NetworkSelectionElemen
 		}
 	}
 
-	// acquire lock to access file
-	if types.ChrootMutex != nil {
-		types.ChrootMutex.Lock()
-		defer types.ChrootMutex.Unlock()
-	}
 	configBytes, err := netutils.GetCNIConfig(customResource, confdir)
 	if err != nil {
 		return nil, resourceMap, err
@@ -466,12 +461,6 @@ func getNetDelegate(client *ClientInfo, pod *v1.Pod, netname, confdir, namespace
 
 		// option2) search CNI json config file, which has <netname> as CNI name, from confDir
 
-		// acquire lock to access file
-		if types.ChrootMutex != nil {
-			types.ChrootMutex.Lock()
-			defer types.ChrootMutex.Unlock()
-		}
-
 		configBytes, err = netutils.GetCNIConfigFromFile(netname, confdir)
 		if err == nil {
 			delegate, err := types.LoadDelegateNetConf(configBytes, nil, "", "")
@@ -481,12 +470,6 @@ func getNetDelegate(client *ClientInfo, pod *v1.Pod, netname, confdir, namespace
 			return delegate, resourceMap, nil
 		}
 	} else {
-		// acquire lock to access file
-		if types.ChrootMutex != nil {
-			types.ChrootMutex.Lock()
-			defer types.ChrootMutex.Unlock()
-		}
-
 		fInfo, err := os.Stat(netname)
 		if err != nil {
 			return nil, resourceMap, err

diff --git a/pkg/server/exec_chroot.go b/pkg/server/exec_chroot.go
@@ -20,10 +20,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"os"
 	"os/exec"
 	"strings"
-	"sync"
 	"syscall"
 	"time"
 
@@ -34,91 +32,23 @@ import (
 
 // ChrootExec implements invoke.Exec to execute CNI with chroot
 type ChrootExec struct {
-	Stderr     io.Writer
-	chrootDir  string
-	workingDir string   // working directory in the outer root
-	outerRoot  *os.File // outer root directory
+	Stderr    io.Writer
+	chrootDir string
 	version.PluginDecoder
-	mu sync.Mutex
 }
 
 var _ invoke.Exec = &ChrootExec{}
 
-func (e *ChrootExec) chroot() error {
-	var err error
-	e.workingDir, err = os.Getwd()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "getwd before chroot failed: %v\n", err)
-		return fmt.Errorf("getwd before chroot failed: %v", err)
-	}
-
-	e.outerRoot, err = os.Open("/")
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "getwd before chroot failed: %v\n", err)
-		return fmt.Errorf("getwd before chroot failed: %v", err)
-	}
-
-	if err := syscall.Chroot(e.chrootDir); err != nil {
-		fmt.Fprintf(os.Stderr, "chroot to %s failed: %v\n", e.chrootDir, err)
-		return fmt.Errorf("chroot to %s failed: %v", e.chrootDir, err)
-	}
-
-	if err := os.Chdir("/"); err != nil {
-		fmt.Fprintf(os.Stderr, "chdir to \"/\" failed: %v\n", err)
-		return fmt.Errorf("chdir to \"/\" failed: %v", err)
-	}
-
-	return nil
-}
-
-func (e *ChrootExec) escape() error {
-	if e.outerRoot == nil || e.workingDir == "" {
-		return nil
-	}
-
-	// change directory to outer root and close it
-	if err := syscall.Fchdir(int(e.outerRoot.Fd())); err != nil {
-		fmt.Fprintf(os.Stderr, "changing directory to outer root failed: %v\n", err)
-		return fmt.Errorf("changing directory to outer root failed: %v", err)
-	}
-
-	if err := e.outerRoot.Close(); err != nil {
-		fmt.Fprintf(os.Stderr, "closing outer root failed: %v\n", err)
-		return fmt.Errorf("closing outer root failed: %v", err)
-	}
-
-	// chroot to current directory aka "." being the outer root
-	if err := syscall.Chroot("."); err != nil {
-		fmt.Fprintf(os.Stderr, "chroot to current directory failed: %v\n", err)
-		return fmt.Errorf("chroot to current directory failed: %v", err)
-	}
-
-	if err := os.Chdir(e.workingDir); err != nil {
-		fmt.Fprintf(os.Stderr, "chdir to working directory failed: %v\n", err)
-		return fmt.Errorf("chdir to working directory failed: %v", err)
-	}
-	e.outerRoot = nil
-	e.workingDir = ""
-
-	return nil
-}
-
 // ExecPlugin executes CNI plugin with given environment/stdin data.
 func (e *ChrootExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData []byte, environ []string) ([]byte, error) {
-	// lock and do chroot to execute plugin with host root
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	err := e.chroot()
-	defer e.escape()
-
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "ExecPlugin failed at chroot: %v\n", err)
-		return nil, fmt.Errorf("ExecPlugin failed at chroot: %v", err)
-	}
+	var err error
 
 	stdout := &bytes.Buffer{}
 	stderr := &bytes.Buffer{}
 	c := exec.CommandContext(ctx, pluginPath)
+	c.SysProcAttr = &syscall.SysProcAttr{
+		Chroot: e.chrootDir,
+	}
 	c.Env = environ
 	c.Stdin = bytes.NewBuffer(stdinData)
 	c.Stdout = stdout
@@ -169,14 +99,5 @@ func (e *ChrootExec) pluginErr(err error, stdout, stderr []byte) error {
 
 // FindInPath try to find CNI plugin based on given path
 func (e *ChrootExec) FindInPath(plugin string, paths []string) (string, error) {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	err := e.chroot()
-	defer e.escape()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "FindInPath failed at chroot: %v\n", err)
-		return "", fmt.Errorf("FindInPath failed at chroot: %v", err)
-	}
-
 	return invoke.FindInPath(plugin, paths)
 }
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -210,7 +210,6 @@ func NewCNIServer(daemonConfig *ControllerNetConf, serverConfig []byte, ignoreRe
 			Stderr:    os.Stderr,
 			chrootDir: daemonConfig.ChrootDir,
 		}
-		types.ChrootMutex = &chrootExec.mu
 		exec = chrootExec
 		logging.Verbosef("server configured with chroot: %s", daemonConfig.ChrootDir)
 	}

diff --git a/pkg/types/conf.go b/pkg/types/conf.go
@@ -21,7 +21,6 @@ import (
 	"net"
 	"os"
 	"strings"
-	"sync"
 	"time"
 
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
@@ -43,9 +42,6 @@ const (
 	defaultNonIsolatedNamespace   = "default"
 )
 
-// ChrootMutex provides lock to access host filesystem
-var ChrootMutex *sync.Mutex
-
 // LoadDelegateNetConfList reads DelegateNetConf from bytes
 func LoadDelegateNetConfList(bytes []byte, delegateConf *DelegateNetConf) error {
 	logging.Debugf("LoadDelegateNetConfList: %s, %v", string(bytes), delegateConf)