From 2ddc5198fbeaf15931ed9eb072bc5ecdda900442 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Mon, 2 Sep 2024 17:42:08 +0200 Subject: [PATCH] Add `Remove` into `filesystemCache` Enables us to deletes cache directory for a given catalog from the filesystem. Signed-off-by: Mikalai Radchuk --- cmd/manager/main.go | 5 +- internal/catalogmetadata/cache/cache.go | 27 ++++++++- internal/catalogmetadata/cache/cache_test.go | 62 +++++++++++++++++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index db25c3ad0..03de6c1c5 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -219,9 +219,10 @@ func main() { setupLog.Error(err, "unable to create catalogs cache directory") os.Exit(1) } - catalogClient := catalogclient.New(cache.NewFilesystemCache(catalogsCachePath, func() (*http.Client, error) { + cacheFetcher := cache.NewFilesystemCache(catalogsCachePath, func() (*http.Client, error) { return httputil.BuildHTTPClient(certPoolWatcher) - })) + }) + catalogClient := catalogclient.New(cacheFetcher) resolver := &resolve.CatalogResolver{ WalkCatalogsFunc: resolve.CatalogWalker( diff --git a/internal/catalogmetadata/cache/cache.go b/internal/catalogmetadata/cache/cache.go index f5c8a52eb..e986c9714 100644 --- a/internal/catalogmetadata/cache/cache.go +++ b/internal/catalogmetadata/cache/cache.go @@ -25,7 +25,7 @@ var _ client.Fetcher = &filesystemCache{} // - IF cached it will verify the cache is up to date. If it is up to date it will return // the cached contents, if not it will fetch the new contents from the catalogd HTTP // server and update the cached contents. -func NewFilesystemCache(cachePath string, clientFunc func() (*http.Client, error)) client.Fetcher { +func NewFilesystemCache(cachePath string, clientFunc func() (*http.Client, error)) *filesystemCache { return &filesystemCache{ cachePath: cachePath, mutex: sync.RWMutex{}, @@ -80,7 +80,7 @@ func (fsc *filesystemCache) FetchCatalogContents(ctx context.Context, catalog *c return nil, fmt.Errorf("error: catalog %q has a nil status.resolvedSource.image value", catalog.Name) } - cacheDir := filepath.Join(fsc.cachePath, catalog.Name) + cacheDir := fsc.cacheDir(catalog.Name) fsc.mutex.RLock() if data, ok := fsc.cacheDataByCatalogName[catalog.Name]; ok { if catalog.Status.ResolvedSource.Image.ResolvedRef == data.ResolvedRef { @@ -166,3 +166,26 @@ func (fsc *filesystemCache) FetchCatalogContents(ctx context.Context, catalog *c return os.DirFS(cacheDir), nil } + +// Remove deletes cache directory for a given catalog from the filesystem +func (fsc *filesystemCache) Remove(catalogName string) error { + cacheDir := fsc.cacheDir(catalogName) + + fsc.mutex.Lock() + defer fsc.mutex.Unlock() + + if _, exists := fsc.cacheDataByCatalogName[catalogName]; !exists { + return nil + } + + if err := os.RemoveAll(cacheDir); err != nil { + return fmt.Errorf("error removing cache directory: %v", err) + } + + delete(fsc.cacheDataByCatalogName, catalogName) + return nil +} + +func (fsc *filesystemCache) cacheDir(catalogName string) string { + return filepath.Join(fsc.cachePath, catalogName) +} diff --git a/internal/catalogmetadata/cache/cache_test.go b/internal/catalogmetadata/cache/cache_test.go index 51b554721..74f7d79c0 100644 --- a/internal/catalogmetadata/cache/cache_test.go +++ b/internal/catalogmetadata/cache/cache_test.go @@ -10,12 +10,14 @@ import ( "io/fs" "maps" "net/http" + "os" "path/filepath" "testing" "testing/fstest" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -62,7 +64,7 @@ var defaultFS = fstest.MapFS{ "fake1/olm.channel/stable.json": &fstest.MapFile{Data: []byte(stableChannel)}, } -func TestFilesystemCache(t *testing.T) { +func TestFilesystemCacheFetchCatalogContents(t *testing.T) { type test struct { name string catalog *catalogd.ClusterCatalog @@ -245,6 +247,64 @@ func TestFilesystemCache(t *testing.T) { } } +func TestFilesystemCacheRemove(t *testing.T) { + testCatalog := &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-catalog", + }, + Status: catalogd.ClusterCatalogStatus{ + ResolvedSource: &catalogd.ResolvedCatalogSource{ + Type: catalogd.SourceTypeImage, + Image: &catalogd.ResolvedImageSource{ + ResolvedRef: "fake/catalog@sha256:fakesha", + }, + }, + }, + } + + ctx := context.Background() + cacheDir := t.TempDir() + + tripper := &mockTripper{} + tripper.content = make(fstest.MapFS) + maps.Copy(tripper.content, defaultFS) + httpClient := &http.Client{ + Transport: tripper, + } + c := cache.NewFilesystemCache(cacheDir, func() (*http.Client, error) { + return httpClient, nil + }) + + catalogCachePath := filepath.Join(cacheDir, testCatalog.Name) + + t.Log("Remove cache before it exists") + require.NoDirExists(t, catalogCachePath) + err := c.Remove(testCatalog.Name) + require.NoError(t, err) + assert.NoDirExists(t, catalogCachePath) + + t.Log("Fetch contents to populate cache") + _, err = c.FetchCatalogContents(ctx, testCatalog) + require.NoError(t, err) + require.DirExists(t, catalogCachePath) + + t.Log("Temporary change permissions to the cache dir to cause error") + require.NoError(t, os.Chmod(catalogCachePath, 0000)) + + t.Log("Remove cache causes an error") + err = c.Remove(testCatalog.Name) + require.ErrorContains(t, err, "error removing cache directory") + require.DirExists(t, catalogCachePath) + + t.Log("Restore directory permissions for successful removal") + require.NoError(t, os.Chmod(catalogCachePath, 0777)) + + t.Log("Remove cache") + err = c.Remove(testCatalog.Name) + require.NoError(t, err) + assert.NoDirExists(t, catalogCachePath) +} + var _ http.RoundTripper = &mockTripper{} type mockTripper struct {