From 949573c5a879e2486fb14984b77660a347b78403 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 21 Jun 2021 14:01:56 +0200 Subject: [PATCH] Do not use inotify for OCICNI Podman does not need to watch the cni config directory. If a network is not found in the cache, OCICNI will reload the networks anyway and thus even podman system service should work as expected. Also include a change to not mount a "new" /var by default in the rootless cni ns, instead try to use /var/lib/cni first and then the parent dir. This allows users to store cni configs under /var/... which is the case for the CI compose test. [NO TESTS NEEDED] Fixes #10686 Signed-off-by: Paul Holzinger --- go.mod | 2 +- go.sum | 4 +- libpod/networking_linux.go | 34 +++++++-- libpod/runtime.go | 2 +- .../cri-o/ocicni/pkg/ocicni/ocicni.go | 71 ++++++++++--------- vendor/modules.txt | 2 +- 6 files changed, 71 insertions(+), 44 deletions(-) diff --git a/go.mod b/go.mod index a6fc311644..7962fb0dcb 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/containers/storage v1.31.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 diff --git a/go.sum b/go.sum index 899093c6cf..9f26ec90d4 100644 --- a/go.sum +++ b/go.sum @@ -265,8 +265,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.20210301205850-541cf7c703cf/go.mod h1:vingr1ztOAzP2WyTgGbpMov9dFhbjNxdLtDv0+PhAvY= +github.com/cri-o/ocicni v0.2.1-0.20210621164014-d0acc7862283 h1:7FyIYKksGvRF8XjMkG5T6uIxg8PcgZoPyO+f6kHT5+s= +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= diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index fc1d7df502..b37b150985 100644 --- a/libpod/networking_linux.go +++ b/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 - runDir := filepath.Join(r.dir, "run") - varDir := filepath.Join(r.dir, "var") + // cni plugins need access to /var/lib/cni and /run + varDir := "" + varTarget := persistentCNIDir + // we can only mount to a target dir which exists, check /var/lib/cni recursively + // while we could always use /var there are cases where a user might store the cni + // configs under /var/custom and this would break + 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, "/var", "none", unix.MS_BIND, "") + err = unix.Mount(varDir, varTarget, "none", unix.MS_BIND, "") if err != nil { - return errors.Wrap(err, "failed to mount /var for rootless cni") + 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 { @@ -386,7 +406,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") } @@ -1025,7 +1045,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 { diff --git a/libpod/runtime.go b/libpod/runtime.go index 2cf2b3f79b..f0c85df3b0 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -458,7 +458,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") } diff --git a/vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go b/vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go index b383401267..90d5b6c508 100644 --- a/vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go +++ b/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,22 +250,26 @@ func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir strin plugin.syncNetworkConfig() - plugin.watcher, err = newWatcher(plugin.confDir) - if err != nil { - return nil, err - } + if useInotify { + plugin.watcher, err = newWatcher(plugin.confDir) + if err != nil { + return nil, err + } - startWg := sync.WaitGroup{} - startWg.Add(1) - go plugin.monitorConfDir(&startWg) - startWg.Wait() + startWg := sync.WaitGroup{} + startWg.Add(1) + go plugin.monitorConfDir(&startWg) + startWg.Wait() + } return plugin, nil } func (plugin *cniNetworkPlugin) Shutdown() error { close(plugin.shutdownChan) - plugin.watcher.Close() + 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 err + return fmt.Errorf("error adding pod %s to CNI network %q: %v", fullPodName, network.name, 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 err + return fmt.Errorf("error checking pod %s for CNI network %q: %v", fullPodName, network.name, 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) - res, err := cni.AddNetworkList(ctx, network.config, rt) - if err != nil { - logrus.Errorf("Error adding network: %v", err) - return nil, err - } - - return res, nil + return cni.AddNetworkList(ctx, network.config, rt) } 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) - if err := cni.DelNetworkList(ctx, network.config, rt); err != nil { - return err - } - return nil + return cni.DelNetworkList(ctx, network.config, rt) } 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 != "" { diff --git a/vendor/modules.txt b/vendor/modules.txt index 6be35cb065..1b76b4da80 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -250,7 +250,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