From 03424be0f1f07e003d30bb35c0b29debcddc74e4 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Mon, 15 Apr 2024 15:55:53 +0300 Subject: [PATCH] Add support for cache_path option The change is required to be able to configure persistent dir for cache files. The default path "/tmp/ovscache" is ephemeral in some distros, and this can cause CmdDel to exit without proper cleanup after the host reboot (because the cache is empty). Signed-off-by: Yury Kulazhenkov --- docs/cni-plugin.md | 1 + pkg/config/config.go | 8 +++--- pkg/mirror-consumer/consumer.go | 11 +++++--- pkg/mirror-producer/producer.go | 10 ++++--- pkg/plugin/plugin.go | 13 +++++++--- pkg/plugin/plugin_test.go | 46 ++++++++++++++++++++++++++++++++- pkg/types/types.go | 2 ++ pkg/utils/cache.go | 28 +++++++++++++------- 8 files changed, 93 insertions(+), 26 deletions(-) diff --git a/docs/cni-plugin.md b/docs/cni-plugin.md index 7536001d..18eae7fd 100644 --- a/docs/cni-plugin.md +++ b/docs/cni-plugin.md @@ -60,6 +60,7 @@ Another example with a port which has an interface of type system: * `interface_type` (string, optional): type of the interface belongs to ports. if value is "", ovs will use default interface of type 'internal' * `configuration_path` (optional): configuration file containing ovsdb socket file path, etc. +* `cache_path` (string, optional): path that ovs-cni uses to store temporary state, defaults to /tmp/ovscache ### Flatfile Configuation diff --git a/pkg/config/config.go b/pkg/config/config.go index 159447ba..73dfa4e1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -78,9 +78,9 @@ func LoadMirrorConf(data []byte) (*types.MirrorNetConf, error) { } // LoadPrevResultConfFromCache retrieve preResult config from cache -func LoadPrevResultConfFromCache(cRef string) (*types.CachedPrevResultNetConf, error) { +func LoadPrevResultConfFromCache(cacheDir, cRef string) (*types.CachedPrevResultNetConf, error) { netCache := &types.CachedPrevResultNetConf{} - netConfBytes, err := utils.ReadCache(cRef) + netConfBytes, err := utils.ReadCache(cacheDir, cRef) if err != nil { return nil, fmt.Errorf("error reading cached prevResult conf with name %s: %v", cRef, err) } @@ -93,9 +93,9 @@ func LoadPrevResultConfFromCache(cRef string) (*types.CachedPrevResultNetConf, e } // LoadConfFromCache retrieve net config from cache -func LoadConfFromCache(cRef string) (*types.CachedNetConf, error) { +func LoadConfFromCache(cacheDir, cRef string) (*types.CachedNetConf, error) { netCache := &types.CachedNetConf{} - netConfBytes, err := utils.ReadCache(cRef) + netConfBytes, err := utils.ReadCache(cacheDir, cRef) if err != nil { return nil, fmt.Errorf("error reading cached NetConf with name %s: %v", cRef, err) } diff --git a/pkg/mirror-consumer/consumer.go b/pkg/mirror-consumer/consumer.go index 48bdcc03..9614f89a 100644 --- a/pkg/mirror-consumer/consumer.go +++ b/pkg/mirror-consumer/consumer.go @@ -98,7 +98,7 @@ func CmdAdd(args *skel.CmdArgs) error { } // Cache PrevResult for CmdDel - if err = utils.SaveCache(config.GetCRef(args.ContainerID, args.IfName)+"_cons", + if err = utils.SaveCache(netconf.CachePath, config.GetCRef(args.ContainerID, args.IfName)+"_cons", &types.CachedPrevResultNetConf{PrevResult: netconf.PrevResult}); err != nil { return fmt.Errorf("error saving NetConf %q", err) } @@ -138,9 +138,12 @@ func CmdAdd(args *skel.CmdArgs) error { // CmdDel remove handler for deleting container from network func CmdDel(args *skel.CmdArgs) error { logCall("DEL", args) - + stdinConf, err := config.LoadConf(args.StdinData) + if err != nil { + return err + } cRef := config.GetCRef(args.ContainerID, args.IfName) - cache, err := config.LoadPrevResultConfFromCache(cRef + "_cons") + cache, err := config.LoadPrevResultConfFromCache(stdinConf.CachePath, cRef+"_cons") if err != nil { // If cmdDel() fails, cached prevResult is cleaned up by // the followed defer call. However, subsequence calls @@ -154,7 +157,7 @@ func CmdDel(args *skel.CmdArgs) error { defer func() { if err == nil { - if err := utils.CleanCache(cRef + "_cons"); err != nil { + if err := utils.CleanCache(stdinConf.CachePath, cRef+"_cons"); err != nil { log.Printf("Failed cleaning up cache: %v", err) } } diff --git a/pkg/mirror-producer/producer.go b/pkg/mirror-producer/producer.go index b848bfcd..8e3240b2 100644 --- a/pkg/mirror-producer/producer.go +++ b/pkg/mirror-producer/producer.go @@ -98,7 +98,7 @@ func CmdAdd(args *skel.CmdArgs) error { } // Cache PrevResult for CmdDel - if err = utils.SaveCache(config.GetCRef(args.ContainerID, args.IfName)+"_prod", + if err = utils.SaveCache(netconf.CachePath, config.GetCRef(args.ContainerID, args.IfName)+"_prod", &types.CachedPrevResultNetConf{PrevResult: netconf.PrevResult}); err != nil { return fmt.Errorf("error saving NetConf %q", err) } @@ -130,9 +130,13 @@ func CmdAdd(args *skel.CmdArgs) error { // CmdDel remove handler for deleting container from network func CmdDel(args *skel.CmdArgs) error { logCall("DEL", args) + stdinConf, err := config.LoadConf(args.StdinData) + if err != nil { + return err + } cRef := config.GetCRef(args.ContainerID, args.IfName) - cache, err := config.LoadPrevResultConfFromCache(cRef + "_prod") + cache, err := config.LoadPrevResultConfFromCache(stdinConf.CachePath, cRef+"_prod") if err != nil { // If cmdDel() fails, cached prevResult is cleaned up by // the followed defer call. However, subsequence calls @@ -146,7 +150,7 @@ func CmdDel(args *skel.CmdArgs) error { defer func() { if err == nil { - if err := utils.CleanCache(cRef + "_prod"); err != nil { + if err := utils.CleanCache(stdinConf.CachePath, cRef+"_prod"); err != nil { log.Printf("Failed cleaning up cache: %v", err) } } diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 7b29aced..d2ada030 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -287,7 +287,7 @@ func CmdAdd(args *skel.CmdArgs) error { } // Cache NetConf for CmdDel - if err = utils.SaveCache(config.GetCRef(args.ContainerID, args.IfName), + if err = utils.SaveCache(netconf.CachePath, config.GetCRef(args.ContainerID, args.IfName), &types.CachedNetConf{Netconf: netconf, OrigIfName: origIfName}); err != nil { return fmt.Errorf("error saving NetConf %q", err) } @@ -472,8 +472,13 @@ func removeOvsPort(ovsDriver *ovsdb.OvsBridgeDriver, portName string) error { func CmdDel(args *skel.CmdArgs) error { logCall("DEL", args) + stdinConf, err := config.LoadConf(args.StdinData) + if err != nil { + return err + } + cRef := config.GetCRef(args.ContainerID, args.IfName) - cache, err := config.LoadConfFromCache(cRef) + cache, err := config.LoadConfFromCache(stdinConf.CachePath, cRef) if err != nil { // If cmdDel() fails, cached netconf is cleaned up by // the followed defer call. However, subsequence calls @@ -487,7 +492,7 @@ func CmdDel(args *skel.CmdArgs) error { defer func() { if err == nil { - if err := utils.CleanCache(cRef); err != nil { + if err := utils.CleanCache(stdinConf.CachePath, cRef); err != nil { log.Printf("Failed cleaning up cache: %v", err) } } @@ -616,7 +621,7 @@ func CmdCheck(args *skel.CmdArgs) error { // check cache cRef := config.GetCRef(args.ContainerID, args.IfName) - cache, err := config.LoadConfFromCache(cRef) + cache, err := config.LoadConfFromCache(netconf.CachePath, cRef) if err != nil { return err } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 3cf85e85..d5585850 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -19,9 +19,9 @@ import ( "encoding/json" "errors" "fmt" - "github.com/k8snetworkplumbingwg/ovs-cni/pkg/config" "math/rand" "net" + "os" "os/exec" "strconv" "strings" @@ -41,6 +41,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/k8snetworkplumbingwg/ovs-cni/pkg/config" "github.com/k8snetworkplumbingwg/ovs-cni/pkg/types" ) @@ -73,6 +74,7 @@ type Net040 struct { VlanTag *uint `json:"vlan"` Trunk []*types.Trunk `json:"trunk,omitempty"` InterfaceType string `json:"interface_type"` + CachePath string `json:"cache_path"` RawPrevResult map[string]interface{} `json:"prevResult,omitempty"` PrevResult types040.Result `json:"-"` } @@ -86,6 +88,7 @@ type NetCurrent struct { VlanTag *uint `json:"vlan"` Trunk []*types.Trunk `json:"trunk,omitempty"` InterfaceType string `json:"interface_type"` + CachePath string `json:"cache_path"` RawPrevResult map[string]interface{} `json:"prevResult,omitempty"` PrevResult current.Result `json:"-"` } @@ -904,6 +907,47 @@ var testFunc = func(version string) { ContainSubstring(secondHostIface.Name), "OVS port with healthy interface should have been kept") }) }) + Context("cache path configuration", func() { + var ( + err error + tempDir string + ) + BeforeEach(func() { + tempDir, err = os.MkdirTemp("", "ovs-cni-test-*") + Expect(err).NotTo(HaveOccurred()) + }) + AfterEach(func() { + err = os.RemoveAll(tempDir) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should use provided path", func() { + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ovs", + "bridge": "%s", + "cache_path": "%s" + }`, version, bridgeName, tempDir) + targetNs := newNS() + defer func() { + closeNS(targetNs) + }() + hostIfName, result := testAdd(conf, false, false, "", targetNs) + + // cmdAdd should create file in the configured cache dir + cacheFiles, err := os.ReadDir(tempDir) + Expect(err).NotTo(HaveOccurred()) + Expect(cacheFiles).To(HaveLen(1)) + + testCheck(conf, result, targetNs) + testDel(conf, hostIfName, targetNs, true) + // cmdDel should remove file from the cache dir + cacheFiles, err = os.ReadDir(tempDir) + Expect(err).NotTo(HaveOccurred()) + Expect(cacheFiles).To(BeEmpty()) + }) + }) }) } diff --git a/pkg/types/types.go b/pkg/types/types.go index 6e168115..54a536ae 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -40,6 +40,7 @@ type NetConf struct { SocketFile string `json:"socket_file"` LinkStateCheckRetries int `json:"link_state_check_retries"` LinkStateCheckInterval int `json:"link_state_check_interval"` + CachePath string `json:"cache_path"` // path to the cache folder, defaults to /tmp/ovscache } // MirrorNetConf extends types.NetConf for ovs-mirrors @@ -55,6 +56,7 @@ type MirrorNetConf struct { ConfigurationPath string `json:"configuration_path"` SocketFile string `json:"socket_file"` Mirrors []*Mirror `json:"mirrors"` + CachePath string `json:"cache_path"` // path to the cache folder, defaults to /tmp/ovscache } // Mirror configuration diff --git a/pkg/utils/cache.go b/pkg/utils/cache.go index aa7ab85f..83678ef9 100644 --- a/pkg/utils/cache.go +++ b/pkg/utils/cache.go @@ -30,17 +30,18 @@ var ( ) // SaveCache takes in key as string and a json encoded struct Conf and save this Conf in cache dir -func SaveCache(key string, conf interface{}) error { +func SaveCache(cacheDir, key string, conf interface{}) error { + cacheDir = getCacheDirPath(cacheDir) confBytes, err := json.Marshal(conf) if err != nil { return fmt.Errorf("error serializing delegate conf: %v", err) } // save the rendered conf for cmdDel - if err = os.MkdirAll(DefaultCacheDir, 0700); err != nil { - return fmt.Errorf("failed to create the sriov data directory(%q): %v", DefaultCacheDir, err) + if err = os.MkdirAll(cacheDir, 0700); err != nil { + return fmt.Errorf("failed to create the sriov data directory(%q): %v", cacheDir, err) } - path := getKeyPath(key) + path := getKeyPath(cacheDir, key) err = ioutil.WriteFile(path, confBytes, 0600) if err != nil { return fmt.Errorf("failed to write container data in the path(%q): %v", path, err) @@ -49,8 +50,8 @@ func SaveCache(key string, conf interface{}) error { } // ReadCache read cached conf from disk for the given key and returns data in byte array -func ReadCache(key string) ([]byte, error) { - path := getKeyPath(key) +func ReadCache(cacheDir, key string) ([]byte, error) { + path := getKeyPath(getCacheDirPath(cacheDir), key) data, err := ioutil.ReadFile(path) if err != nil { return nil, fmt.Errorf("failed to read container data in the path(%q): %v", path, err) @@ -59,14 +60,21 @@ func ReadCache(key string) ([]byte, error) { } // CleanCache removes cached conf from disk for the given key -func CleanCache(key string) error { - path := getKeyPath(key) +func CleanCache(cacheDir, key string) error { + path := getKeyPath(getCacheDirPath(cacheDir), key) if err := os.Remove(path); err != nil { return fmt.Errorf("error removing Conf file %s: %q", path, err) } return nil } -func getKeyPath(key string) string { - return filepath.Join(DefaultCacheDir, key) +func getKeyPath(cacheDir, key string) string { + return filepath.Join(getCacheDirPath(cacheDir), key) +} + +func getCacheDirPath(cacheDir string) string { + if cacheDir == "" { + cacheDir = DefaultCacheDir + } + return cacheDir }