diff --git a/deployments/multus-daemonset-thick.yml b/deployments/multus-daemonset-thick.yml index d2ef13c0c..41d98070c 100644 --- a/deployments/multus-daemonset-thick.yml +++ b/deployments/multus-daemonset-thick.yml @@ -166,6 +166,11 @@ spec: volumeMounts: - name: cni mountPath: /host/etc/cni/net.d + # multus-daemon expects that cni-bin path must be identical between pod and container host. + # e.g. if the cni bin is in '/opt/cni/bin' on the container host side, then it should be mount to '/opt/cni/bin' in multus-daemon, + # not to any other directory, like '/opt/bin' or '/usr/bin'. + - name: cni-bin + mountPath: /opt/cni/bin - name: host-run mountPath: /host/run - name: host-var-lib-cni-multus diff --git a/pkg/k8sclient/k8sclient.go b/pkg/k8sclient/k8sclient.go index c1d1857a9..77ccc9fab 100644 --- 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 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 index 3f84c5ed8..61e2bc541 100644 --- 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,24 @@ 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) + // execute delegate CNI with host filesystem context. + c.SysProcAttr = &syscall.SysProcAttr{ + Chroot: e.chrootDir, + } c.Env = environ c.Stdin = bytes.NewBuffer(stdinData) c.Stdout = stdout @@ -169,14 +100,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/exec_chroot_test.go b/pkg/server/exec_chroot_test.go index 80b3b2c50..3290355b1 100644 --- a/pkg/server/exec_chroot_test.go +++ b/pkg/server/exec_chroot_test.go @@ -43,15 +43,4 @@ var _ = Describe("exec_chroot", func() { _, err := chrootExec.ExecPlugin(context.Background(), "/bin/true", nil, nil) Expect(err).To(HaveOccurred()) }) - - It("Call ChrootExec.FindInPath with dummy", func() { - chrootExec := &ChrootExec{ - Stderr: os.Stderr, - chrootDir: "/usr/bin", - } - - _, err := chrootExec.FindInPath("true", []string{"/"}) - Expect(err).NotTo(HaveOccurred()) - }) - }) diff --git a/pkg/server/server.go b/pkg/server/server.go index 4bea6ffc5..a215758c7 100644 --- 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 index 8295bd9c6..4402b4ec7 100644 --- 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)