Merge pull request #10741 from Luap99/test-ocicni

Do not use inotify for OCICNI

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/containers/storage v1.32.3
 	github.com/coreos/go-systemd/v22 v22.3.2
 	github.com/coreos/stream-metadata-go v0.0.0-20210225230131-70edb9eb47b3
-	github.com/cri-o/ocicni v0.2.1-0.20210301205850-541cf7c703cf
+	github.com/cri-o/ocicni v0.2.1-0.20210621164014-d0acc7862283
 	github.com/cyphar/filepath-securejoin v0.2.2
 	github.com/davecgh/go-spew v1.1.1
 	github.com/digitalocean/go-qemu v0.0.0-20210209191958-152a1535e49f

go.sum

@@ -271,8 +271,8 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/cri-o/ocicni v0.2.1-0.20210301205850-541cf7c703cf h1:k2wrxBiBseRfOD7h+9fABEuesABBQuUuW5fWwpARbeI=
+github.com/cri-o/ocicni v0.2.1-0.20210621164014-d0acc7862283 h1:7FyIYKksGvRF8XjMkG5T6uIxg8PcgZoPyO+f6kHT5+s=
-github.com/cri-o/ocicni v0.2.1-0.20210301205850-541cf7c703cf/go.mod h1:vingr1ztOAzP2WyTgGbpMov9dFhbjNxdLtDv0+PhAvY=
+github.com/cri-o/ocicni v0.2.1-0.20210621164014-d0acc7862283/go.mod h1:vingr1ztOAzP2WyTgGbpMov9dFhbjNxdLtDv0+PhAvY=
 github.com/cyphar/filepath-securejoin v0.2.2 h1:jCwT2GTP+PY5nBz3c/YL5PAIbusElVrPujOBSCj8xRg=
 github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4=
 github.com/d2g/dhcp4 v0.0.0-20170904100407-a1d1b6c41b1c/go.mod h1:Ct2BUK8SB0YC1SMSibvLzxjeJLnrYEVLULFNiHY9YfQ=

libpod/networking_linux.go

@@ -46,6 +46,9 @@ const (
 
 	// rootlessCNINSName is the file name for the rootless network namespace bind mount
 	rootlessCNINSName = "rootless-cni-ns"
+
+	// persistentCNIDir is the directory where the CNI files are stored
+	persistentCNIDir = "/var/lib/cni"
 )
 
 // Get an OCICNI network config
@@ -150,14 +153,31 @@ func (r *RootlessCNI) Do(toRun func() error) error {
 			}
 		}
 
-		// cni plugins need access to /var and /run
+		// cni plugins need access to /var/lib/cni and /run
-		runDir := filepath.Join(r.dir, "run")
+		varDir := ""
-		varDir := filepath.Join(r.dir, "var")
+		varTarget := persistentCNIDir
-		// make sure to mount var first
+		// we can only mount to a target dir which exists, check /var/lib/cni recursively
-		err = unix.Mount(varDir, "/var", "none", unix.MS_BIND, "")
+		// while we could always use /var there are cases where a user might store the cni
-		if err != nil {
+		// configs under /var/custom and this would break
-			return errors.Wrap(err, "failed to mount /var for rootless cni")
+		for {
+			if _, err := os.Stat(varTarget); err == nil {
+				varDir = filepath.Join(r.dir, strings.TrimPrefix(varTarget, "/"))
+				break
 			}
+			varTarget = filepath.Base(varTarget)
+			if varTarget == "/" {
+				break
+			}
+		}
+		if varDir == "" {
+			return errors.New("failed to stat /var directory")
+		}
+		// make sure to mount var first
+		err = unix.Mount(varDir, varTarget, "none", unix.MS_BIND, "")
+		if err != nil {
+			return errors.Wrapf(err, "failed to mount %s for rootless cni", varTarget)
+		}
+		runDir := filepath.Join(r.dir, "run")
 		// recursive mount to keep the netns mount
 		err = unix.Mount(runDir, "/run", "none", unix.MS_BIND|unix.MS_REC, "")
 		if err != nil {
@@ -385,7 +405,7 @@ func (r *Runtime) GetRootlessCNINetNs(new bool) (*RootlessCNI, error) {
 
 				// create cni directories to store files
 				// they will be bind mounted to the correct location in a extra mount ns
-				err = os.MkdirAll(filepath.Join(cniDir, "var"), 0700)
+				err = os.MkdirAll(filepath.Join(cniDir, strings.TrimPrefix(persistentCNIDir, "/")), 0700)
 				if err != nil {
 					return nil, errors.Wrap(err, "could not create rootless-cni var directory")
 				}
@@ -1043,7 +1063,7 @@ func resultToBasicNetworkConfig(result *cnitypes.Result) (define.InspectBasicNet
 // after itself on an unclean reboot. Return what we're pretty sure is the path
 // to CNI's internal files (it's not really exposed to us).
 func getCNINetworksDir() (string, error) {
-	return "/var/lib/cni/networks", nil
+	return filepath.Join(persistentCNIDir, "networks"), nil
 }
 
 type logrusDebugWriter struct {

libpod/runtime.go

@@ -468,7 +468,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
 	}
 
 	// Set up the CNI net plugin
-	netPlugin, err := ocicni.InitCNI(runtime.config.Network.DefaultNetwork, runtime.config.Network.NetworkConfigDir, runtime.config.Network.CNIPluginDirs...)
+	netPlugin, err := ocicni.InitCNINoInotify(runtime.config.Network.DefaultNetwork, runtime.config.Network.NetworkConfigDir, "", runtime.config.Network.CNIPluginDirs...)
 	if err != nil {
 		return errors.Wrapf(err, "error configuring CNI network plugin")
 	}

vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go

@@ -195,16 +195,21 @@ func (plugin *cniNetworkPlugin) monitorConfDir(start *sync.WaitGroup) {
 // If defaultNetName is empty, CNI config files should be reloaded real-time and
 // defaultNetName should be changeable and determined by file sorting.
 func InitCNI(defaultNetName string, confDir string, binDirs ...string) (CNIPlugin, error) {
-	return initCNI(nil, "", defaultNetName, confDir, binDirs...)
+	return initCNI(nil, "", defaultNetName, confDir, true, binDirs...)
 }
 
 // InitCNIWithCache works like InitCNI except that it takes the cni cache directory as third param.
 func InitCNIWithCache(defaultNetName, confDir, cacheDir string, binDirs ...string) (CNIPlugin, error) {
-	return initCNI(nil, cacheDir, defaultNetName, confDir, binDirs...)
+	return initCNI(nil, cacheDir, defaultNetName, confDir, true, binDirs...)
+}
+
+// InitCNINoInotify works like InitCNI except that it does not use inotify to watch for changes in the CNI config dir.
+func InitCNINoInotify(defaultNetName, confDir, cacheDir string, binDirs ...string) (CNIPlugin, error) {
+	return initCNI(nil, cacheDir, defaultNetName, confDir, false, binDirs...)
 }
 
 // Internal function to allow faking out exec functions for testing
-func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir string, binDirs ...string) (CNIPlugin, error) {
+func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir string, useInotify bool, binDirs ...string) (CNIPlugin, error) {
 	if confDir == "" {
 		confDir = DefaultConfDir
 	}
@@ -245,6 +250,7 @@ func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir strin
 
 	plugin.syncNetworkConfig()
 
+	if useInotify {
 		plugin.watcher, err = newWatcher(plugin.confDir)
 		if err != nil {
 			return nil, err
@@ -254,13 +260,16 @@ func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir strin
 		startWg.Add(1)
 		go plugin.monitorConfDir(&startWg)
 		startWg.Wait()
+	}
 
 	return plugin, nil
 }
 
 func (plugin *cniNetworkPlugin) Shutdown() error {
 	close(plugin.shutdownChan)
+	if plugin.watcher != nil {
 		plugin.watcher.Close()
+	}
 	plugin.done.Wait()
 	return nil
 }
@@ -539,10 +548,11 @@ func (plugin *cniNetworkPlugin) SetUpPodWithContext(ctx context.Context, podNetw
 
 	results := make([]NetResult, 0)
 	if err := plugin.forEachNetwork(&podNetwork, false, func(network *cniNetwork, podNetwork *PodNetwork, rt *libcni.RuntimeConf) error {
+		fullPodName := buildFullPodName(*podNetwork)
+		logrus.Infof("Adding pod %s to CNI network %q (type=%v)", fullPodName, network.name, network.config.Plugins[0].Network.Type)
 		result, err := network.addToNetwork(ctx, rt, plugin.cniConfig)
 		if err != nil {
-			logrus.Errorf("Error while adding pod to CNI network %q: %s", network.name, err)
+			return fmt.Errorf("error adding pod %s to CNI network %q: %v", fullPodName, network.name, err)
-			return err
 		}
 		results = append(results, NetResult{
 			Result: result,
@@ -654,8 +664,10 @@ func (plugin *cniNetworkPlugin) TearDownPodWithContext(ctx context.Context, podN
 	}
 
 	return plugin.forEachNetwork(&podNetwork, true, func(network *cniNetwork, podNetwork *PodNetwork, rt *libcni.RuntimeConf) error {
+		fullPodName := buildFullPodName(*podNetwork)
+		logrus.Infof("Deleting pod %s from CNI network %q (type=%v)", fullPodName, network.name, network.config.Plugins[0].Network.Type)
 		if err := network.deleteFromNetwork(ctx, rt, plugin.cniConfig); err != nil {
-			return fmt.Errorf("Error while removing pod from CNI network %q: %s", network.name, err)
+			return fmt.Errorf("error removing pod %s from CNI network %q: %v", fullPodName, network.name, err)
 		}
 		return nil
 	})
@@ -680,10 +692,11 @@ func (plugin *cniNetworkPlugin) GetPodNetworkStatusWithContext(ctx context.Conte
 
 	results := make([]NetResult, 0)
 	if err := plugin.forEachNetwork(&podNetwork, true, func(network *cniNetwork, podNetwork *PodNetwork, rt *libcni.RuntimeConf) error {
+		fullPodName := buildFullPodName(*podNetwork)
+		logrus.Infof("Checking pod %s for CNI network %s (type=%v)", fullPodName, network.name, network.config.Plugins[0].Network.Type)
 		result, err := network.checkNetwork(ctx, rt, plugin.cniConfig, plugin.nsManager, podNetwork.NetNS)
 		if err != nil {
-			logrus.Errorf("Error while checking pod to CNI network %q: %s", network.name, err)
+			return fmt.Errorf("error checking pod %s for CNI network %q: %v", fullPodName, network.name, err)
-			return err
 		}
 		if result != nil {
 			results = append(results, NetResult{
@@ -703,19 +716,10 @@ func (plugin *cniNetworkPlugin) GetPodNetworkStatusWithContext(ctx context.Conte
 }
 
 func (network *cniNetwork) addToNetwork(ctx context.Context, rt *libcni.RuntimeConf, cni *libcni.CNIConfig) (cnitypes.Result, error) {
-	logrus.Infof("About to add CNI network %s (type=%v)", network.name, network.config.Plugins[0].Network.Type)
+	return cni.AddNetworkList(ctx, network.config, rt)
-	res, err := cni.AddNetworkList(ctx, network.config, rt)
-	if err != nil {
-		logrus.Errorf("Error adding network: %v", err)
-		return nil, err
-	}
-
-	return res, nil
 }
 
 func (network *cniNetwork) checkNetwork(ctx context.Context, rt *libcni.RuntimeConf, cni *libcni.CNIConfig, nsManager *nsManager, netns string) (cnitypes.Result, error) {
-	logrus.Infof("About to check CNI network %s (type=%v)", network.name, network.config.Plugins[0].Network.Type)
-
 	gtet, err := cniversion.GreaterThanOrEqualTo(network.config.CNIVersion, "0.4.0")
 	if err != nil {
 		return nil, err
@@ -786,11 +790,7 @@ func (network *cniNetwork) checkNetwork(ctx context.Context, rt *libcni.RuntimeC
 }
 
 func (network *cniNetwork) deleteFromNetwork(ctx context.Context, rt *libcni.RuntimeConf, cni *libcni.CNIConfig) error {
-	logrus.Infof("About to del CNI network %s (type=%v)", network.name, network.config.Plugins[0].Network.Type)
+	return cni.DelNetworkList(ctx, network.config, rt)
-	if err := cni.DelNetworkList(ctx, network.config, rt); err != nil {
-		return err
-	}
-	return nil
 }
 
 func buildCNIRuntimeConf(podNetwork *PodNetwork, ifName string, runtimeConfig RuntimeConfig) (*libcni.RuntimeConf, error) {
@@ -809,6 +809,13 @@ func buildCNIRuntimeConf(podNetwork *PodNetwork, ifName string, runtimeConfig Ru
 		CapabilityArgs: map[string]interface{}{},
 	}
 
+	// Propagate existing CNI_ARGS to non-k8s consumers
+	for _, kvpairs := range strings.Split(os.Getenv("CNI_ARGS"), ";") {
+		if keyval := strings.SplitN(kvpairs, "=", 2); len(keyval) == 2 {
+			rt.Args = append(rt.Args, [2]string{keyval[0], keyval[1]})
+		}
+	}
+
 	// Add requested static IP to CNI_ARGS
 	ip := runtimeConfig.IP
 	if ip != "" {

vendor/modules.txt

@@ -254,7 +254,7 @@ github.com/coreos/stream-metadata-go/fedoracoreos
 github.com/coreos/stream-metadata-go/fedoracoreos/internals
 github.com/coreos/stream-metadata-go/stream
 github.com/coreos/stream-metadata-go/stream/rhcos
-# github.com/cri-o/ocicni v0.2.1-0.20210301205850-541cf7c703cf
+# github.com/cri-o/ocicni v0.2.1-0.20210621164014-d0acc7862283
 github.com/cri-o/ocicni/pkg/ocicni
 # github.com/cyphar/filepath-securejoin v0.2.2
 github.com/cyphar/filepath-securejoin