From 6e11ecc6fc217c55d751f4b2c8c34f991a0398b0 Mon Sep 17 00:00:00 2001 From: wang yan <wangyan@vmware.com> Date: Mon, 12 Aug 2019 16:46:01 +0800 Subject: [PATCH] Update codes per review comments Signed-off-by: wang yan <wangyan@vmware.com> --- docs/swagger.yaml | 14 ++++++ .../postgresql/0010_1.9.0_schema.up.sql | 3 +- src/core/api/harborapi_test.go | 6 +++ src/core/api/internal.go | 26 ++++++++-- src/core/api/internal_test.go | 33 +++++++++++++ src/core/api/quota/chart/chart.go | 11 +++-- src/core/api/quota/migrator.go | 13 ++--- src/core/api/quota/registry/registry.go | 8 +++ src/core/main.go | 49 ++++++++++++++++--- 9 files changed, 138 insertions(+), 25 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index e62ecaecc..f11279553 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2409,6 +2409,20 @@ paths: $ref: '#/responses/UnsupportedMediaType' '500': description: Unexpected internal errors. + /internal/syncquota: + post: + summary: Sync quota from registry/chart to DB. + description: | + This endpoint is for syncing quota usage of registry/chart with database. + tags: + - Products + responses: + '200': + description: Sync repositories successfully. + '401': + description: User need to log in first. + '403': + description: User does not have permission of system admin role. /systeminfo: get: summary: Get general system info diff --git a/make/migrations/postgresql/0010_1.9.0_schema.up.sql b/make/migrations/postgresql/0010_1.9.0_schema.up.sql index 7fd3a4241..7000c69e9 100644 --- a/make/migrations/postgresql/0010_1.9.0_schema.up.sql +++ b/make/migrations/postgresql/0010_1.9.0_schema.up.sql @@ -86,6 +86,7 @@ CREATE TABLE quota_usage UNIQUE (reference, reference_id) ); +/* only set quota and usage for 'library', and let the sync quota handling others. */ INSERT INTO quota (reference, reference_id, hard, creation_time, update_time) SELECT 'project', CAST(project_id AS VARCHAR), @@ -93,7 +94,7 @@ SELECT 'project', NOW(), NOW() FROM project -WHERE deleted = 'f'; +WHERE name = 'library' and deleted = 'f'; INSERT INTO quota_usage (id, reference, reference_id, used, creation_time, update_time) SELECT id, diff --git a/src/core/api/harborapi_test.go b/src/core/api/harborapi_test.go index f76530f88..b6ed840b2 100644 --- a/src/core/api/harborapi_test.go +++ b/src/core/api/harborapi_test.go @@ -35,6 +35,7 @@ import ( testutils "github.com/goharbor/harbor/src/common/utils/test" api_models "github.com/goharbor/harbor/src/core/api/models" apimodels "github.com/goharbor/harbor/src/core/api/models" + quota "github.com/goharbor/harbor/src/core/api/quota" _ "github.com/goharbor/harbor/src/core/auth/db" _ "github.com/goharbor/harbor/src/core/auth/ldap" "github.com/goharbor/harbor/src/core/config" @@ -203,12 +204,17 @@ func init() { beego.Router("/api/quotas/:id([0-9]+)", quotaAPIType, "get:Get;put:Put") beego.Router("/api/internal/switchquota", &InternalAPI{}, "put:SwitchQuota") + beego.Router("/api/internal/syncquota", &InternalAPI{}, "post:SyncQuota") // syncRegistry if err := SyncRegistry(config.GlobalProjectMgr); err != nil { log.Fatalf("failed to sync repositories from registry: %v", err) } + if err := quota.Sync(config.GlobalProjectMgr, false); err != nil { + log.Fatalf("failed to sync quota from backend: %v", err) + } + // Init user Info admin = &usrInfo{adminName, adminPwd} unknownUsr = &usrInfo{"unknown", "unknown"} diff --git a/src/core/api/internal.go b/src/core/api/internal.go index 5e4b621dc..06e6c45a2 100644 --- a/src/core/api/internal.go +++ b/src/core/api/internal.go @@ -28,6 +28,8 @@ import ( "strconv" quota "github.com/goharbor/harbor/src/core/api/quota" + + comcfg "github.com/goharbor/harbor/src/common/config" ) // InternalAPI handles request of harbor admin... @@ -154,9 +156,25 @@ func (ia *InternalAPI) ensureQuota() error { // SyncQuota ... func (ia *InternalAPI) SyncQuota() { - err := quota.Sync(ia.ProjectMgr, false) - if err != nil { - ia.SendInternalServerError(err) - return + cur := config.ReadOnly() + cfgMgr := comcfg.NewDBCfgManager() + if cur != true { + cfgMgr.Set(common.ReadOnly, true) } + // For api call, to avoid the timeout, it should be asynchronous + go func() { + defer func() { + if cur != true { + cfgMgr.Set(common.ReadOnly, false) + } + }() + log.Info("start to sync quota(API), the system will be set to ReadOnly and back it normal once it done.") + err := quota.Sync(ia.ProjectMgr, false) + if err != nil { + log.Errorf("fail to sync quota(API), but with error: %v, please try to do it again.", err) + return + } + log.Info("success to sync quota(API).") + }() + return } diff --git a/src/core/api/internal_test.go b/src/core/api/internal_test.go index 87f4eb8b9..02903a98b 100644 --- a/src/core/api/internal_test.go +++ b/src/core/api/internal_test.go @@ -54,3 +54,36 @@ func TestSwitchQuota(t *testing.T) { } runCodeCheckingCases(t, cases...) } + +// cannot verify the real scenario here +func TestSyncQuota(t *testing.T) { + cases := []*codeCheckingCase{ + // 401 + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/internal/syncquota", + }, + code: http.StatusUnauthorized, + }, + // 200 + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/internal/syncquota", + credential: sysAdmin, + }, + code: http.StatusOK, + }, + // 403 + { + request: &testingRequest{ + url: "/api/internal/syncquota", + method: http.MethodPost, + credential: nonSysAdmin, + }, + code: http.StatusForbidden, + }, + } + runCodeCheckingCases(t, cases...) +} diff --git a/src/core/api/quota/chart/chart.go b/src/core/api/quota/chart/chart.go index 438fc6beb..31dbb42ec 100644 --- a/src/core/api/quota/chart/chart.go +++ b/src/core/api/quota/chart/chart.go @@ -103,19 +103,22 @@ func (rm *Migrator) Dump() ([]quota.ProjectInfo, error) { ctr, err := chartController() if err != nil { - log.Error(err) + errChan <- err + return } chartInfo, err := ctr.ListCharts(project.Name) if err != nil { - log.Error(err) + errChan <- err + return } // repo for _, chart := range chartInfo { chartVersions, err := ctr.GetChart(project.Name, chart.Name) if err != nil { - log.Error(err) + errChan <- err + continue } for _, chart := range chartVersions { af := &models.Artifact{ @@ -143,7 +146,7 @@ func (rm *Migrator) Dump() ([]quota.ProjectInfo, error) { }(project) } - wg.Done() + wg.Wait() close(infoChan) <-done diff --git a/src/core/api/quota/migrator.go b/src/core/api/quota/migrator.go index dfab9ce41..b7f36b61a 100644 --- a/src/core/api/quota/migrator.go +++ b/src/core/api/quota/migrator.go @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package models +package api import ( - "fmt" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/quota" @@ -75,23 +74,21 @@ func Register(name string, adapter Instance) { // Sync ... func Sync(pm promgr.ProjectManager, populate bool) error { - for name := range adapters { + for name, instanceFunc := range adapters { if !config.WithChartMuseum() { if name == "chart" { continue } } - instanceFunc, ok := adapters[name] - if !ok { - err := fmt.Errorf("quota migtator: unknown adapter name %q", name) - return err - } adapter := instanceFunc(pm) data, err := adapter.Dump() if err != nil { return err } usage, err := adapter.Usage(data) + if err != nil { + return err + } if err := ensureQuota(usage); err != nil { return err } diff --git a/src/core/api/quota/registry/registry.go b/src/core/api/quota/registry/registry.go index 19ad7aafc..5f69fb710 100644 --- a/src/core/api/quota/registry/registry.go +++ b/src/core/api/quota/registry/registry.go @@ -383,6 +383,14 @@ func infoOfRepo(pid int64, repo string) (quota.RepoData, error) { DigestBlob: desc.Digest.String(), } afnbs = append(afnbs, afnb) + // add manifest as a blob. + blob := &models.Blob{ + Digest: desc.Digest.String(), + ContentType: desc.MediaType, + Size: desc.Size, + CreationTime: time.Now(), + } + blobs = append(blobs, blob) for _, layer := range manifest.References() { afnb := &models.ArtifactAndBlob{ DigestAF: desc.Digest.String(), diff --git a/src/core/main.go b/src/core/main.go index 9883f9cc8..412ab25d5 100755 --- a/src/core/main.go +++ b/src/core/main.go @@ -17,16 +17,12 @@ package main import ( "encoding/gob" "fmt" - "os" - "os/signal" - "strconv" - "syscall" - "github.com/astaxie/beego" _ "github.com/astaxie/beego/session/redis" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/job" "github.com/goharbor/harbor/src/common/models" + common_quota "github.com/goharbor/harbor/src/common/quota" "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/api" @@ -34,6 +30,10 @@ import ( _ "github.com/goharbor/harbor/src/core/auth/db" _ "github.com/goharbor/harbor/src/core/auth/ldap" _ "github.com/goharbor/harbor/src/core/auth/uaa" + "os" + "os/signal" + "strconv" + "syscall" quota "github.com/goharbor/harbor/src/core/api/quota" _ "github.com/goharbor/harbor/src/core/api/quota/chart" @@ -46,6 +46,7 @@ import ( "github.com/goharbor/harbor/src/core/service/token" "github.com/goharbor/harbor/src/pkg/notification" "github.com/goharbor/harbor/src/pkg/scheduler" + "github.com/goharbor/harbor/src/pkg/types" "github.com/goharbor/harbor/src/replication" ) @@ -92,16 +93,48 @@ func quotaSync() error { return err } - // upgrade from old version - if len(projects) > 1 && len(usages) == 1 { + // The condition handles these two cases: + // 1, len(project) > 1 && len(usages) == 1. existing projects without usage, as we do always has 'library' usage in DB. + // 2, migration fails at the phase of inserting usage into DB, and parts of them are inserted successfully. + if len(projects) != len(usages) { log.Info("Start to sync quota data .....") if err := quota.Sync(config.GlobalProjectMgr, true); err != nil { - log.Errorf("Error happened when syncing quota usage data, %v", err) + log.Errorf("Fail to sync quota data, %v", err) return err } log.Info("Success to sync quota data .....") + return nil } + // Only has one project without usage + zero := common_quota.ResourceList{ + common_quota.ResourceCount: 0, + common_quota.ResourceStorage: 0, + } + if len(projects) == 1 && len(usages) == 1 { + totalRepo, err := dao.GetTotalOfRepositories() + if totalRepo == 0 { + return nil + } + refID, err := strconv.ParseInt(usages[0].ReferenceID, 10, 64) + if err != nil { + log.Error(err) + return err + } + usedRes, err := types.NewResourceList(usages[0].Used) + if err != nil { + log.Error(err) + return err + } + if types.Equals(usedRes, zero) && refID == projects[0].ProjectID { + log.Info("Start to sync quota data .....") + if err := quota.Sync(config.GlobalProjectMgr, true); err != nil { + log.Errorf("Fail to sync quota data, %v", err) + return err + } + log.Info("Success to sync quota data .....") + } + } return nil }