From f16cc4bda4a38d9591fed38436bafadad30d4678 Mon Sep 17 00:00:00 2001 From: chlins Date: Mon, 23 May 2022 09:08:27 +0800 Subject: [PATCH] feat(project): introduce cache layer for project_metadata (#16891) Implement cache layer for project_metadata and migrate metadata.Mgr to pkg.ProjectMetaMgr. Signed-off-by: chlins --- src/controller/project/controller.go | 2 +- src/controller/project/metadata/controller.go | 4 +- src/controller/scanner/base_controller.go | 3 +- src/pkg/cached/manager.go | 2 + src/pkg/cached/project/redis/manager_test.go | 4 +- .../cached/project_metadata/redis/manager.go | 182 ++++++++++++++++++ .../project_metadata/redis/manager_test.go | 123 ++++++++++++ src/pkg/factory.go | 14 ++ src/pkg/factory_test.go | 13 ++ src/pkg/project/metadata/manager.go | 5 - src/server/v2.0/handler/project.go | 3 +- src/server/v2.0/handler/retention.go | 3 +- 12 files changed, 346 insertions(+), 12 deletions(-) create mode 100644 src/pkg/cached/project_metadata/redis/manager.go create mode 100644 src/pkg/cached/project_metadata/redis/manager_test.go diff --git a/src/controller/project/controller.go b/src/controller/project/controller.go index fbd45d280..71286a692 100644 --- a/src/controller/project/controller.go +++ b/src/controller/project/controller.go @@ -70,7 +70,7 @@ type Controller interface { func NewController() Controller { return &controller{ projectMgr: pkg.ProjectMgr, - metaMgr: metadata.Mgr, + metaMgr: pkg.ProjectMetaMgr, allowlistMgr: allowlist.NewDefaultManager(), userMgr: user.Mgr, } diff --git a/src/controller/project/metadata/controller.go b/src/controller/project/metadata/controller.go index 6aa9ebc8b..1395a4c47 100644 --- a/src/controller/project/metadata/controller.go +++ b/src/controller/project/metadata/controller.go @@ -16,6 +16,8 @@ package metadata import ( "context" + + "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/project/metadata" ) @@ -40,7 +42,7 @@ type Controller interface { // NewController creates an instance of the default controller func NewController() Controller { return &controller{ - mgr: metadata.Mgr, + mgr: pkg.ProjectMetaMgr, } } diff --git a/src/controller/scanner/base_controller.go b/src/controller/scanner/base_controller.go index 1fa1c6bc5..2028da355 100644 --- a/src/controller/scanner/base_controller.go +++ b/src/controller/scanner/base_controller.go @@ -26,6 +26,7 @@ import ( "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/project/metadata" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" @@ -45,7 +46,7 @@ var DefaultController = New() func New() Controller { return &basicController{ manager: rscanner.New(), - proMetaMgr: metadata.Mgr, + proMetaMgr: pkg.ProjectMetaMgr, clientPool: v1.DefaultClientPool, } } diff --git a/src/pkg/cached/manager.go b/src/pkg/cached/manager.go index 943750411..824f10d8f 100644 --- a/src/pkg/cached/manager.go +++ b/src/pkg/cached/manager.go @@ -27,6 +27,8 @@ const ( ResourceTypeArtifact = "artifact" // ResourceTypeProject defines project type. ResourceTypeProject = "project" + // ResourceTypeProject defines project metadata type. + ResourceTypeProjectMeta = "project_metadata" // ResourceTypeRepository defines repository type. ResourceTypeRepository = "repository" ) diff --git a/src/pkg/cached/project/redis/manager_test.go b/src/pkg/cached/project/redis/manager_test.go index 65b8effb2..791f1840d 100644 --- a/src/pkg/cached/project/redis/manager_test.go +++ b/src/pkg/cached/project/redis/manager_test.go @@ -61,14 +61,14 @@ func (m *managerTestSuite) TestCount() { m.Equal(int64(100), c) } -func (m *managerTestSuite) List() { +func (m *managerTestSuite) TestList() { m.projectMgr.On("List", mock.Anything, mock.Anything).Return([]*models.Project{}, nil) ps, err := m.cachedManager.List(m.ctx, q.New(q.KeyWords{})) m.NoError(err) m.ElementsMatch([]*models.Project{}, ps) } -func (m *managerTestSuite) ListRoles() { +func (m *managerTestSuite) TestListRoles() { m.projectMgr.On("ListRoles", mock.Anything, mock.Anything, mock.Anything).Return([]int{1}, nil) rs, err := m.cachedManager.ListRoles(m.ctx, 1, 1) m.NoError(err) diff --git a/src/pkg/cached/project_metadata/redis/manager.go b/src/pkg/cached/project_metadata/redis/manager.go new file mode 100644 index 000000000..9ea05349c --- /dev/null +++ b/src/pkg/cached/project_metadata/redis/manager.go @@ -0,0 +1,182 @@ +// Copyright Project Harbor 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 redis + +import ( + "context" + "strings" + "time" + + libcache "github.com/goharbor/harbor/src/lib/cache" + "github.com/goharbor/harbor/src/lib/config" + "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/retry" + "github.com/goharbor/harbor/src/pkg/cached" + "github.com/goharbor/harbor/src/pkg/project/metadata" + "github.com/goharbor/harbor/src/pkg/project/metadata/models" +) + +var _ CachedManager = &manager{} + +// CachedManager is the interface combines raw resource manager and cached manager for better extension. +type CachedManager interface { + // Manager is the raw resource manager. + metadata.Manager + // Manager is the common interface for resource cache. + cached.Manager +} + +// manager is the cached manager implemented by redis. +type manager struct { + // delegator delegates the raw crud to DAO. + delegator metadata.Manager + // client returns the redis cache client. + client func() libcache.Cache + // keyBuilder builds cache object key. + keyBuilder *cached.ObjectKey + // lifetime is the cache life time. + lifetime time.Duration +} + +// NewManager returns the redis cache manager. +func NewManager(m metadata.Manager) *manager { + return &manager{ + delegator: m, + client: func() libcache.Cache { return libcache.Default() }, + keyBuilder: cached.NewObjectKey(cached.ResourceTypeProjectMeta), + lifetime: time.Duration(config.CacheExpireHours()) * time.Hour, + } +} + +func (m *manager) Add(ctx context.Context, projectID int64, meta map[string]string) error { + return m.delegator.Add(ctx, projectID, meta) +} + +func (m *manager) List(ctx context.Context, name string, value string) ([]*models.ProjectMetadata, error) { + return m.delegator.List(ctx, name, value) +} + +func (m *manager) Get(ctx context.Context, projectID int64, meta ...string) (map[string]string, error) { + key, err := m.keyBuilder.Format("projectID", projectID, "meta", strings.Join(meta, ",")) + if err != nil { + return nil, err + } + + result := make(map[string]string) + if err = m.client().Fetch(ctx, key, &result); err == nil { + return result, nil + } + + log.Debugf("get project %d metadata from cache error: %v, will query from database.", projectID, err) + + result, err = m.delegator.Get(ctx, projectID, meta...) + if err != nil { + return nil, err + } + + if err = m.client().Save(ctx, key, &result, m.lifetime); err != nil { + // log error if save to cache failed + log.Debugf("save project metadata %v to cache error: %v", result, err) + } + + return result, nil +} + +func (m *manager) Delete(ctx context.Context, projectID int64, meta ...string) error { + // pass on delete operation + if err := m.delegator.Delete(ctx, projectID, meta...); err != nil { + return err + } + // clean cache + m.cleanUp(ctx, projectID, meta...) + return nil +} + +func (m *manager) Update(ctx context.Context, projectID int64, meta map[string]string) error { + if err := m.delegator.Update(ctx, projectID, meta); err != nil { + return err + } + // clean cache + prefix, err := m.keyBuilder.Format("projectID", projectID) + if err != nil { + return err + } + // lookup all keys with projectID prefix + keys, err := m.client().Keys(ctx, prefix) + if err != nil { + return err + } + + for _, key := range keys { + if err = retry.Retry(func() error { return m.client().Delete(ctx, key) }); err != nil { + log.Errorf("delete project metadata cache key %s error: %v", key, err) + } + } + + return nil +} + +// cleanUp cleans up data in cache. +func (m *manager) cleanUp(ctx context.Context, projectID int64, meta ...string) { + key, err := m.keyBuilder.Format("projectID", projectID, "meta", strings.Join(meta, ",")) + if err != nil { + log.Errorf("format project metadata key error: %v", err) + } else { + // retry to avoid dirty data + if err = retry.Retry(func() error { return m.client().Delete(ctx, key) }); err != nil { + log.Errorf("delete project metadata cache key %s error: %v", key, err) + } + } +} + +func (m *manager) ResourceType(ctx context.Context) string { + return cached.ResourceTypeProjectMeta +} + +func (m *manager) CountCache(ctx context.Context) (int64, error) { + // prefix is resource type + keys, err := m.client().Keys(ctx, m.ResourceType(ctx)) + if err != nil { + return 0, err + } + + return int64(len(keys)), nil +} + +func (m *manager) DeleteCache(ctx context.Context, key string) error { + return m.client().Delete(ctx, key) +} + +func (m *manager) FlushAll(ctx context.Context) error { + // prefix is resource type + keys, err := m.client().Keys(ctx, m.ResourceType(ctx)) + if err != nil { + return err + } + + var errs errors.Errors + for _, key := range keys { + if err = m.client().Delete(ctx, key); err != nil { + errs = append(errs, err) + } + } + + if errs.Len() > 0 { + return errs + } + + return nil +} diff --git a/src/pkg/cached/project_metadata/redis/manager_test.go b/src/pkg/cached/project_metadata/redis/manager_test.go new file mode 100644 index 000000000..03f8b3811 --- /dev/null +++ b/src/pkg/cached/project_metadata/redis/manager_test.go @@ -0,0 +1,123 @@ +// Copyright Project Harbor 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 redis + +import ( + "context" + "errors" + "testing" + + "github.com/goharbor/harbor/src/lib/cache" + "github.com/goharbor/harbor/src/pkg/project/metadata/models" + testcache "github.com/goharbor/harbor/src/testing/lib/cache" + "github.com/goharbor/harbor/src/testing/mock" + testProjectMeta "github.com/goharbor/harbor/src/testing/pkg/project/metadata" + + "github.com/stretchr/testify/suite" +) + +type managerTestSuite struct { + suite.Suite + cachedManager CachedManager + projectMetaMgr *testProjectMeta.Manager + cache *testcache.Cache + ctx context.Context +} + +func (m *managerTestSuite) SetupTest() { + m.projectMetaMgr = &testProjectMeta.Manager{} + m.cache = &testcache.Cache{} + m.cachedManager = NewManager( + m.projectMetaMgr, + ) + m.cachedManager.(*manager).client = func() cache.Cache { return m.cache } + m.ctx = context.TODO() +} + +func (m *managerTestSuite) TestAdd() { + m.projectMetaMgr.On("Add", mock.Anything, mock.Anything, mock.Anything).Return(nil) + err := m.cachedManager.Add(m.ctx, 1, map[string]string{}) + m.NoError(err) +} + +func (m *managerTestSuite) TestList() { + m.projectMetaMgr.On("List", mock.Anything, mock.Anything, mock.Anything).Return([]*models.ProjectMetadata{}, nil) + ps, err := m.cachedManager.List(m.ctx, "", "") + m.NoError(err) + m.ElementsMatch([]*models.ProjectMetadata{}, ps) +} + +func (m *managerTestSuite) TestGet() { + // get from cache directly + m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + _, err := m.cachedManager.Get(m.ctx, 100) + m.NoError(err, "should get from cache") + m.projectMetaMgr.AssertNotCalled(m.T(), "Get", mock.Anything, mock.Anything) + + // not found in cache, read from dao + m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(cache.ErrNotFound).Once() + m.cache.On("Save", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + m.projectMetaMgr.On("Get", mock.Anything, mock.Anything).Return(map[string]string{}, nil).Once() + _, err = m.cachedManager.Get(m.ctx, 100) + m.NoError(err, "should get from projectMetaMgr") + m.projectMetaMgr.AssertCalled(m.T(), "Get", mock.Anything, mock.Anything) +} + +func (m *managerTestSuite) TestDelete() { + // delete from projectMgr error + errDelete := errors.New("delete failed") + m.projectMetaMgr.On("Delete", mock.Anything, mock.Anything).Return(errDelete).Once() + m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + err := m.cachedManager.Delete(m.ctx, 100) + m.ErrorIs(err, errDelete, "delete should error") + m.cache.AssertNotCalled(m.T(), "Delete", mock.Anything, mock.Anything) + + // delete from projectMgr success + m.projectMetaMgr.On("Delete", mock.Anything, mock.Anything).Return(nil).Once() + m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Twice() + err = m.cachedManager.Delete(m.ctx, 100) + m.NoError(err, "delete should success") + m.cache.AssertCalled(m.T(), "Delete", mock.Anything, mock.Anything) +} + +func (m *managerTestSuite) TestResourceType() { + t := m.cachedManager.ResourceType(m.ctx) + m.Equal("project_metadata", t) +} + +func (m *managerTestSuite) TestCountCache() { + m.cache.On("Keys", mock.Anything, mock.Anything).Return([]string{"1"}, nil).Once() + c, err := m.cachedManager.CountCache(m.ctx) + m.NoError(err) + m.Equal(int64(1), c) +} + +func (m *managerTestSuite) TestDeleteCache() { + m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Once() + err := m.cachedManager.DeleteCache(m.ctx, "key") + m.NoError(err) +} + +func (m *managerTestSuite) TestFlushAll() { + m.cache.On("Keys", mock.Anything, mock.Anything).Return([]string{"1"}, nil).Once() + m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Once() + err := m.cachedManager.FlushAll(m.ctx) + m.NoError(err) +} + +func TestManager(t *testing.T) { + suite.Run(t, &managerTestSuite{}) +} diff --git a/src/pkg/factory.go b/src/pkg/factory.go index be6bc0fba..4208fb8e3 100644 --- a/src/pkg/factory.go +++ b/src/pkg/factory.go @@ -19,8 +19,10 @@ import ( "github.com/goharbor/harbor/src/pkg/artifact" cachedArtifact "github.com/goharbor/harbor/src/pkg/cached/artifact/redis" cachedProject "github.com/goharbor/harbor/src/pkg/cached/project/redis" + cachedProjectMeta "github.com/goharbor/harbor/src/pkg/cached/project_metadata/redis" cachedRepo "github.com/goharbor/harbor/src/pkg/cached/repository/redis" "github.com/goharbor/harbor/src/pkg/project" + "github.com/goharbor/harbor/src/pkg/project/metadata" "github.com/goharbor/harbor/src/pkg/repository" ) @@ -30,6 +32,8 @@ var ( ArtifactMgr artifact.Manager // ProjectMgr is the manager for project. ProjectMgr project.Manager + // ProjectMetaMgr is the manager for project metadata. + ProjectMetaMgr metadata.Manager // RepositoryMgr is the manager for repository. RepositoryMgr repository.Manager ) @@ -39,6 +43,7 @@ func init() { cacheEnabled := config.CacheEnabled() initArtifactMgr(cacheEnabled) initProjectMgr(cacheEnabled) + initProjectMetaMgr(cacheEnabled) initRepositoryMgr(cacheEnabled) } @@ -62,6 +67,15 @@ func initProjectMgr(cacheEnabled bool) { } } +func initProjectMetaMgr(cacheEnabled bool) { + projectMetaMgr := metadata.New() + if cacheEnabled { + ProjectMetaMgr = cachedProjectMeta.NewManager(projectMetaMgr) + } else { + ProjectMetaMgr = projectMetaMgr + } +} + func initRepositoryMgr(cacheEnabled bool) { repoMgr := repository.New() if cacheEnabled { diff --git a/src/pkg/factory_test.go b/src/pkg/factory_test.go index fa981050a..820564b25 100644 --- a/src/pkg/factory_test.go +++ b/src/pkg/factory_test.go @@ -20,8 +20,10 @@ import ( "github.com/goharbor/harbor/src/pkg/artifact" cachedArtifact "github.com/goharbor/harbor/src/pkg/cached/artifact/redis" cachedProject "github.com/goharbor/harbor/src/pkg/cached/project/redis" + cachedProjectMeta "github.com/goharbor/harbor/src/pkg/cached/project_metadata/redis" cachedRepo "github.com/goharbor/harbor/src/pkg/cached/repository/redis" "github.com/goharbor/harbor/src/pkg/project" + "github.com/goharbor/harbor/src/pkg/project/metadata" "github.com/goharbor/harbor/src/pkg/repository" "github.com/stretchr/testify/assert" ) @@ -48,6 +50,17 @@ func TestInitProjectMgr(t *testing.T) { assert.IsType(t, cachedProject.NewManager(project.New()), ProjectMgr) } +func TestInitProjectMetaMgr(t *testing.T) { + // cache not enable + assert.NotNil(t, ProjectMetaMgr) + assert.IsType(t, metadata.New(), ProjectMetaMgr) + + // cache enable + initProjectMetaMgr(true) + assert.NotNil(t, ProjectMetaMgr) + assert.IsType(t, cachedProjectMeta.NewManager(metadata.New()), ProjectMetaMgr) +} + func TestInitRepositoryMgr(t *testing.T) { // cache not enable assert.NotNil(t, RepositoryMgr) diff --git a/src/pkg/project/metadata/manager.go b/src/pkg/project/metadata/manager.go index 7d1274eb6..eaab5d8e3 100644 --- a/src/pkg/project/metadata/manager.go +++ b/src/pkg/project/metadata/manager.go @@ -23,11 +23,6 @@ import ( "github.com/goharbor/harbor/src/pkg/project/metadata/models" ) -var ( - // Mgr is the global project metadata manager - Mgr = New() -) - // Manager defines the operations that a project metadata manager should implement type Manager interface { // Add metadatas for project specified by projectID diff --git a/src/server/v2.0/handler/project.go b/src/server/v2.0/handler/project.go index a0e33285c..10757675f 100644 --- a/src/server/v2.0/handler/project.go +++ b/src/server/v2.0/handler/project.go @@ -43,6 +43,7 @@ import ( "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/audit" "github.com/goharbor/harbor/src/pkg/member" "github.com/goharbor/harbor/src/pkg/project/metadata" @@ -62,7 +63,7 @@ const defaultDaysToRetentionForProxyCacheProject = 7 func newProjectAPI() *projectAPI { return &projectAPI{ auditMgr: audit.Mgr, - metadataMgr: metadata.Mgr, + metadataMgr: pkg.ProjectMetaMgr, userCtl: user.Ctl, repositoryCtl: repository.Ctl, projectCtl: project.Ctl, diff --git a/src/server/v2.0/handler/retention.go b/src/server/v2.0/handler/retention.go index d01dda500..d6ae7490e 100644 --- a/src/server/v2.0/handler/retention.go +++ b/src/server/v2.0/handler/retention.go @@ -12,6 +12,7 @@ import ( projectCtl "github.com/goharbor/harbor/src/controller/project" retentionCtl "github.com/goharbor/harbor/src/controller/retention" "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/project/metadata" "github.com/goharbor/harbor/src/pkg/retention/policy" "github.com/goharbor/harbor/src/pkg/task" @@ -24,7 +25,7 @@ func newRetentionAPI() *retentionAPI { return &retentionAPI{ projectCtl: projectCtl.Ctl, retentionCtl: retentionCtl.Ctl, - proMetaMgr: metadata.Mgr, + proMetaMgr: pkg.ProjectMetaMgr, } }