Use /var/lib/cni/ovs-cni/cache dir to store config cache (#304)

Old path ("/tmp/ovscache") is cleaned up after reboot
on some operating systems. Lack of the cache entries
may prevent ovs-cni to do a proper cleanup on CmdDel.

Use /var/lib/cni/ovs-cni/cache as a persistent cache dir.

Signed-off-by: Yury Kulazhenkov <[email protected]>

Author: ykulazhenkov

pkg/utils/cache.go

@@ -19,14 +19,17 @@ package utils
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 )
 
 var (
 	// DefaultCacheDir used for caching
-	DefaultCacheDir = "/tmp/ovscache"
+	DefaultCacheDir = "/var/lib/cni/ovs-cni/cache"
+	// OldDefaultCacheDir path to the old caching dir
+	OldDefaultCacheDir = "/tmp/ovscache"
+	// used for tests
+	rootDir = ""
 )
 
 // SaveCache takes in key as string and a json encoded struct Conf and save this Conf in cache dir
@@ -35,13 +38,13 @@ func SaveCache(key string, conf interface{}) error {
 	if err != nil {
 		return fmt.Errorf("error serializing delegate conf: %v", err)
 	}
-
+	path := getKeyPath(key)
+	cacheDir := filepath.Dir(path)
 	// 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)
-	err = ioutil.WriteFile(path, confBytes, 0600)
+	err = os.WriteFile(path, confBytes, 0600)
 	if err != nil {
 		return fmt.Errorf("failed to write container data in the path(%q): %v", path, err)
 	}
@@ -51,22 +54,56 @@ 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)
-	data, err := ioutil.ReadFile(path)
+	oldPath := getOldKeyPath(key)
+	data, err := readCacheFile(path)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read container data in the path(%q): %v", path, err)
+		return nil, err
+	}
+	if data == nil {
+		data, err = readCacheFile(oldPath)
+		if err != nil {
+			return nil, err
+		}
 	}
-	return data, err
+	if data == nil {
+		return nil, fmt.Errorf("failed to read container data from old(%q) and current(%q) path: not found", oldPath, path)
+	}
+	return data, nil
 }
 
 // CleanCache removes cached conf from disk for the given key
 func CleanCache(key string) error {
-	path := getKeyPath(key)
-	if err := os.Remove(path); err != nil {
-		return fmt.Errorf("error removing Conf file %s: %q", path, err)
+	if err := removeCacheFile(getKeyPath(key)); err != nil {
+		return nil
+	}
+	return removeCacheFile(getOldKeyPath(key))
+}
+
+// read content from the file in the provided path, returns nil, nil
+// if file not found
+func readCacheFile(path string) ([]byte, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, nil
+		}
+		return nil, fmt.Errorf("failed to read container data in the path(%q): %v", path, err)
+	}
+	return data, nil
+}
+
+// remove file in the provided path, returns nil if file not found
+func removeCacheFile(path string) error {
+	if err := os.RemoveAll(path); err != nil {
+		return fmt.Errorf("failed to remove container data from the path(%q): %v", path, err)
 	}
 	return nil
 }
 
 func getKeyPath(key string) string {
-	return filepath.Join(DefaultCacheDir, key)
+	return filepath.Join(rootDir, DefaultCacheDir, key)
+}
+
+func getOldKeyPath(key string) string {
+	return filepath.Join(rootDir, OldDefaultCacheDir, key)
 }

pkg/utils/cache_test.go

@@ -0,0 +1,87 @@
+// Copyright (c) 2024 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+type testConf struct {
+	Data string `json:"data"`
+}
+
+func writeToCacheDir(tmpDir, cacheDir, key string, data []byte) {
+	d := filepath.Join(tmpDir, cacheDir)
+	ExpectWithOffset(1, os.MkdirAll(d, 0700)).NotTo(HaveOccurred())
+	ExpectWithOffset(1, os.WriteFile(filepath.Join(d, key), data, 0700)).NotTo(HaveOccurred())
+}
+
+var _ = Describe("Utils", func() {
+	Context("Cache", func() {
+		var (
+			tmpDir string
+			err    error
+		)
+		BeforeEach(func() {
+			tmpDir, err = os.MkdirTemp("", "ovs-cni-cache-test*")
+			rootDir = tmpDir
+			Expect(err).NotTo(HaveOccurred())
+		})
+		AfterEach(func() {
+			rootDir = ""
+			Expect(os.RemoveAll(tmpDir)).NotTo(HaveOccurred())
+		})
+		It("should save data to the new cache path", func() {
+			Expect(SaveCache("key1", testConf{Data: "test"})).NotTo(HaveOccurred())
+			data, err := os.ReadFile(filepath.Join(tmpDir, "/var/lib/cni/ovs-cni/cache/key1"))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(string(data)).To(Equal(`{"data":"test"}`))
+		})
+		It("should return data from the new cache dir", func() {
+			origData := []byte(`{"data":"test"}`)
+			writeToCacheDir(tmpDir, "/var/lib/cni/ovs-cni/cache", "key1", origData)
+			data, err := ReadCache("key1")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(data).To(Equal([]byte(`{"data":"test"}`)))
+		})
+		It("should return data from the old cache dir", func() {
+			origData := []byte(`{"data":"test"}`)
+			writeToCacheDir(tmpDir, "/tmp/ovscache", "key1", origData)
+			data, err := ReadCache("key1")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(data).To(Equal([]byte(`{"data":"test"}`)))
+		})
+		It("should return error if can't read data from new and old path", func() {
+			data, err := ReadCache("key1")
+			Expect(err).To(MatchError(ContainSubstring("not found")))
+			Expect(data).To(BeNil())
+		})
+		It("should remove data from old and new path", func() {
+			origData := []byte(`{"data":"test"}`)
+			writeToCacheDir(tmpDir, "/var/lib/cni/ovs-cni/cache", "key1", origData)
+			writeToCacheDir(tmpDir, "/tmp/ovscache", "key1", origData)
+			Expect(CleanCache("key1")).NotTo(HaveOccurred())
+			_, err := ReadCache("key1")
+			Expect(err).To(MatchError(ContainSubstring("not found")))
+		})
+		It("should not return error when clean called for unknown key", func() {
+			Expect(CleanCache("key1")).NotTo(HaveOccurred())
+		})
+	})
+})

pkg/utils/utils_suite_test.go

@@ -0,0 +1,27 @@
+// Copyright (c) 2024 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestUtils(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Utils Suite")
+}