Merge pull request #8343 from ywk253100/190717_scheduler

Move the pkg manager/controller declaration to the api package

src/core/api/base.go

@@ -15,9 +15,9 @@
 package api
 
 import (
+	"errors"
 	"net/http"
 
-	"errors"
 	"github.com/ghodss/yaml"
 	"github.com/goharbor/harbor/src/common/api"
 	"github.com/goharbor/harbor/src/common/security"
@@ -25,10 +25,21 @@ import (
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/core/filter"
 	"github.com/goharbor/harbor/src/core/promgr"
+	"github.com/goharbor/harbor/src/pkg/project"
+	"github.com/goharbor/harbor/src/pkg/repository"
 )
 
 const (
 	yamlFileContentType = "application/x-yaml"
+	// ReplicationJobType ...
+	ReplicationJobType = "replication"
+	// ScanJobType ...
+	ScanJobType = "scan"
+)
+
+var (
+	projectMgr    project.Manager
+	repositoryMgr repository.Manager
 )
 
 // BaseController ...
@@ -41,13 +52,6 @@ type BaseController struct {
 	ProjectMgr promgr.ProjectManager
 }
 
-const (
-	// ReplicationJobType ...
-	ReplicationJobType = "replication"
-	// ScanJobType ...
-	ScanJobType = "scan"
-)
-
 // Prepare inits security context and project manager from request
 // context
 func (b *BaseController) Prepare() {
@@ -91,6 +95,22 @@ func (b *BaseController) WriteYamlData(object interface{}) {
 // Init related objects/configurations for the API controllers
 func Init() error {
 	registerHealthCheckers()
+
+	// init chart controller
+	if err := initChartController(); err != nil {
+		return err
+	}
+
+	// init project manager
+	initProjectManager()
+
+	// init repository manager
+	initRepositoryManager()
+
+	return nil
+}
+
+func initChartController() error {
 	// If chart repository is not enabled then directly return
 	if !config.WithChartMuseum() {
 		return nil
@@ -102,6 +122,13 @@ func Init() error {
 	}
 
 	chartController = chartCtl
-
 	return nil
 }
+
+func initProjectManager() {
+	projectMgr = project.New()
+}
+
+func initRepositoryManager() {
+	repositoryMgr = repository.New(projectMgr, chartController)
+}

src/core/api/repository_label_test.go

@@ -28,7 +28,7 @@ import (
 
 var (
 	resourceLabelAPIBasePath = "/api/repositories"
-	repository               = "library/hello-world"
+	repo                     = "library/hello-world"
 	tag                      = "latest"
 	proLibraryLabelID        int64
 )
@@ -63,7 +63,7 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method: http.MethodPost,
 			},
 			code: http.StatusUnauthorized,
@@ -72,13 +72,13 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method:     http.MethodPost,
 				credential: projGuest,
 			},
 			code: http.StatusForbidden,
 		},
-		// 404 repository doesn't exist
+		// 404 repo doesn't exist
 		{
 			request: &testingRequest{
 				url:        fmt.Sprintf("%s/library/non-exist-repo/tags/%s/labels", resourceLabelAPIBasePath, tag),
@@ -90,7 +90,7 @@ func TestAddToImage(t *testing.T) {
 		// 404 image doesn't exist
 		{
 			request: &testingRequest{
-				url:        fmt.Sprintf("%s/%s/tags/non-exist-tag/labels", resourceLabelAPIBasePath, repository),
+				url:        fmt.Sprintf("%s/%s/tags/non-exist-tag/labels", resourceLabelAPIBasePath, repo),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 			},
@@ -99,7 +99,7 @@ func TestAddToImage(t *testing.T) {
 		// 400
 		{
 			request: &testingRequest{
-				url:        fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath, repository, tag),
+				url:        fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath, repo, tag),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 			},
@@ -109,7 +109,7 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 				bodyJSON: struct {
@@ -124,7 +124,7 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 				bodyJSON: struct {
@@ -139,7 +139,7 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 				bodyJSON: struct {
@@ -154,7 +154,7 @@ func TestAddToImage(t *testing.T) {
 		{
 			request: &testingRequest{
 				url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-					repository, tag),
+					repo, tag),
 				method:     http.MethodPost,
 				credential: projDeveloper,
 				bodyJSON: struct {
@@ -172,7 +172,7 @@ func TestAddToImage(t *testing.T) {
 func TestGetOfImage(t *testing.T) {
 	labels := []*models.Label{}
 	err := handleAndParse(&testingRequest{
-		url:        fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath, repository, tag),
+		url:        fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath, repo, tag),
 		method:     http.MethodGet,
 		credential: projDeveloper,
 	}, &labels)
@@ -185,7 +185,7 @@ func TestRemoveFromImage(t *testing.T) {
 	runCodeCheckingCases(t, &codeCheckingCase{
 		request: &testingRequest{
 			url: fmt.Sprintf("%s/%s/tags/%s/labels/%d", resourceLabelAPIBasePath,
-				repository, tag, proLibraryLabelID),
+				repo, tag, proLibraryLabelID),
 			method:     http.MethodDelete,
 			credential: projDeveloper,
 		},
@@ -195,7 +195,7 @@ func TestRemoveFromImage(t *testing.T) {
 	labels := []*models.Label{}
 	err := handleAndParse(&testingRequest{
 		url: fmt.Sprintf("%s/%s/tags/%s/labels", resourceLabelAPIBasePath,
-			repository, tag),
+			repo, tag),
 		method:     http.MethodGet,
 		credential: projDeveloper,
 	}, &labels)
@@ -206,7 +206,7 @@ func TestRemoveFromImage(t *testing.T) {
 func TestAddToRepository(t *testing.T) {
 	runCodeCheckingCases(t, &codeCheckingCase{
 		request: &testingRequest{
-			url:    fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repository),
+			url:    fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repo),
 			method: http.MethodPost,
 			bodyJSON: struct {
 				ID int64
@@ -222,7 +222,7 @@ func TestAddToRepository(t *testing.T) {
 func TestGetOfRepository(t *testing.T) {
 	labels := []*models.Label{}
 	err := handleAndParse(&testingRequest{
-		url:        fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repository),
+		url:        fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repo),
 		method:     http.MethodGet,
 		credential: projDeveloper,
 	}, &labels)
@@ -235,7 +235,7 @@ func TestRemoveFromRepository(t *testing.T) {
 	runCodeCheckingCases(t, &codeCheckingCase{
 		request: &testingRequest{
 			url: fmt.Sprintf("%s/%s/labels/%d", resourceLabelAPIBasePath,
-				repository, proLibraryLabelID),
+				repo, proLibraryLabelID),
 			method:     http.MethodDelete,
 			credential: projDeveloper,
 		},
@@ -244,7 +244,7 @@ func TestRemoveFromRepository(t *testing.T) {
 
 	labels := []*models.Label{}
 	err := handleAndParse(&testingRequest{
-		url:        fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repository),
+		url:        fmt.Sprintf("%s/%s/labels", resourceLabelAPIBasePath, repo),
 		method:     http.MethodGet,
 		credential: projDeveloper,
 	}, &labels)

src/pkg/project/manager.go

@@ -21,11 +21,6 @@ import (
 	"github.com/goharbor/harbor/src/common/models"
 )
 
-var (
-	// Mgr is an instance of the default project Manager
-	Mgr = New()
-)
-
 // Manager is used for project management
 // currently, the interface only defines the methods needed for tag retention
 // will expand it when doing refactor

src/pkg/repository/manager.go

@@ -18,15 +18,9 @@ import (
 	"github.com/goharbor/harbor/src/chartserver"
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
-	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/pkg/project"
 )
 
-var (
-	// Mgr is an instance of the default repository Manager
-	Mgr = New()
-)
-
 // Manager is used for repository management
 // currently, the interface only defines the methods needed for tag retention
 // will expand it when doing refactor
@@ -38,11 +32,17 @@ type Manager interface {
 }
 
 // New returns a default implementation of Manager
-func New() Manager {
+func New(projectMgr project.Manager, chartCtl *chartserver.Controller) Manager {
-	return &manager{}
+	return &manager{
+		projectMgr: projectMgr,
+		chartCtl:   chartCtl,
+	}
 }
 
-type manager struct{}
+type manager struct {
+	projectMgr project.Manager
+	chartCtl   *chartserver.Controller
+}
 
 // List image repositories under the project specified by the ID
 func (m *manager) ListImageRepositories(projectID int64) ([]*models.RepoRecord, error) {
@@ -53,9 +53,9 @@ func (m *manager) ListImageRepositories(projectID int64) ([]*models.RepoRecord,
 
 // List chart repositories under the project specified by the ID
 func (m *manager) ListChartRepositories(projectID int64) ([]*chartserver.ChartInfo, error) {
-	project, err := project.Mgr.Get(projectID)
+	project, err := m.projectMgr.Get(projectID)
 	if err != nil {
 		return nil, err
 	}
-	return api.GetChartController().ListCharts(project.Name)
+	return m.chartCtl.ListCharts(project.Name)
 }

src/pkg/retention/launcher.go

@@ -27,12 +27,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-// TODO init the client
-var (
-	client Client
-	mgr    Manager
-)
-
 // Launcher provides function to launch the async jobs to run retentions based on the provided policy.
 type Launcher interface {
 	// Launch async jobs for the retention policy
@@ -49,11 +43,21 @@ type Launcher interface {
 }
 
 // NewLauncher returns an instance of Launcher
-func NewLauncher() Launcher {
+func NewLauncher(projectMgr project.Manager, repositoryMgr repository.Manager,
-	return &launcher{}
+	retentionMgr Manager, retentionClient Client) Launcher {
+	return &launcher{
+		projectMgr:      projectMgr,
+		repositoryMgr:   repositoryMgr,
+		retentionMgr:    retentionMgr,
+		retentionClient: retentionClient,
+	}
 }
 
 type launcher struct {
+	retentionMgr    Manager
+	retentionClient Client
+	projectMgr      project.Manager
+	repositoryMgr   repository.Manager
 }
 
 type jobData struct {
@@ -81,7 +85,7 @@ func (l *launcher) Launch(ply *policy.Metadata, executionID int64) (int64, error
 	var err error
 	if level == "system" {
 		// get projects
-		projectCandidates, err = getProjects()
+		projectCandidates, err = getProjects(l.projectMgr)
 		if err != nil {
 			return 0, launcherError(err)
 		}
@@ -111,7 +115,7 @@ func (l *launcher) Launch(ply *policy.Metadata, executionID int64) (int64, error
 		var repositoryCandidates []*res.Candidate
 		// get repositories of projects
 		for _, projectCandidate := range projectCandidates {
-			repositories, err := getRepositories(projectCandidate.NamespaceID)
+			repositories, err := getRepositories(l.projectMgr, l.repositoryMgr, projectCandidate.NamespaceID)
 			if err != nil {
 				return 0, launcherError(err)
 			}
@@ -152,7 +156,7 @@ func (l *launcher) Launch(ply *policy.Metadata, executionID int64) (int64, error
 	// create task records
 	jobDatas := []*jobData{}
 	for repository, policy := range repositoryRules {
-		taskID, err := mgr.CreateTask(&Task{
+		taskID, err := l.retentionMgr.CreateTask(&Task{
 			ExecutionID: executionID,
 		})
 		if err != nil {
@@ -167,7 +171,7 @@ func (l *launcher) Launch(ply *policy.Metadata, executionID int64) (int64, error
 
 	allFailed := true
 	for _, jobData := range jobDatas {
-		_, err := client.SubmitTask(jobData.taskID, jobData.repository, jobData.policy)
+		_, err := l.retentionClient.SubmitTask(jobData.taskID, jobData.repository, jobData.policy)
 		if err != nil {
 			log.Error(launcherError(fmt.Errorf("failed to submit task %d: %v", jobData.taskID, err)))
 			continue
@@ -184,8 +188,8 @@ func launcherError(err error) error {
 	return errors.Wrap(err, "launcher")
 }
 
-func getProjects() ([]*res.Candidate, error) {
+func getProjects(projectMgr project.Manager) ([]*res.Candidate, error) {
-	projects, err := project.Mgr.List()
+	projects, err := projectMgr.List()
 	if err != nil {
 		return nil, err
 	}
@@ -199,14 +203,14 @@ func getProjects() ([]*res.Candidate, error) {
 	return candidates, nil
 }
 
-func getRepositories(projectID int64) ([]*res.Candidate, error) {
+func getRepositories(projectMgr project.Manager, repositoryMgr repository.Manager, projectID int64) ([]*res.Candidate, error) {
 	var candidates []*res.Candidate
-	project, err := project.Mgr.Get(projectID)
+	project, err := projectMgr.Get(projectID)
 	if err != nil {
 		return nil, err
 	}
 	// get image repositories
-	imageRepositories, err := repository.Mgr.ListImageRepositories(projectID)
+	imageRepositories, err := repositoryMgr.ListImageRepositories(projectID)
 	if err != nil {
 		return nil, err
 	}
@@ -219,7 +223,7 @@ func getRepositories(projectID int64) ([]*res.Candidate, error) {
 		})
 	}
 	// get chart repositories
-	chartRepositories, err := repository.Mgr.ListChartRepositories(projectID)
+	chartRepositories, err := repositoryMgr.ListChartRepositories(projectID)
 	for _, repository := range chartRepositories {
 		candidates = append(candidates, &res.Candidate{
 			Namespace:  project.Name,

src/pkg/retention/launcher_test.go

@@ -137,6 +137,10 @@ func (f *fakeRetentionManager) ListHistories(executionID int64, query *q.Query)
 
 type launchTestSuite struct {
 	suite.Suite
+	projectMgr      project.Manager
+	repositoryMgr   repository.Manager
+	retentionMgr    Manager
+	retentionClient Client
 }
 
 func (l *launchTestSuite) SetupTest() {
@@ -144,11 +148,11 @@ func (l *launchTestSuite) SetupTest() {
 		ProjectID: 1,
 		Name:      "library",
 	}
-	project.Mgr = &fakeProjectManager{
+	l.projectMgr = &fakeProjectManager{
 		projects: []*models.Project{
 			pro,
 		}}
-	repository.Mgr = &fakeRepositoryManager{
+	l.repositoryMgr = &fakeRepositoryManager{
 		imageRepositories: []*models.RepoRecord{
 			{
 				Name: "library/image",
@@ -160,12 +164,12 @@ func (l *launchTestSuite) SetupTest() {
 			},
 		},
 	}
-	client = &fakeClient{}
+	l.retentionMgr = &fakeRetentionManager{}
-	mgr = &fakeRetentionManager{}
+	l.retentionClient = &fakeClient{}
 }
 
 func (l *launchTestSuite) TestGetProjects() {
-	projects, err := getProjects()
+	projects, err := getProjects(l.projectMgr)
 	require.Nil(l.T(), err)
 	assert.Equal(l.T(), 1, len(projects))
 	assert.Equal(l.T(), int64(1), projects[0].NamespaceID)
@@ -173,7 +177,7 @@ func (l *launchTestSuite) TestGetProjects() {
 }
 
 func (l *launchTestSuite) TestGetRepositories() {
-	repositories, err := getRepositories(1)
+	repositories, err := getRepositories(l.projectMgr, l.repositoryMgr, 1)
 	require.Nil(l.T(), err)
 	assert.Equal(l.T(), 2, len(repositories))
 	assert.Equal(l.T(), "library", repositories[0].Namespace)
@@ -185,7 +189,7 @@ func (l *launchTestSuite) TestGetRepositories() {
 }
 
 func (l *launchTestSuite) TestLaunch() {
-	launcher := NewLauncher()
+	launcher := NewLauncher(l.projectMgr, l.repositoryMgr, l.retentionMgr, l.retentionClient)
 	var ply *policy.Metadata
 	// nil policy
 	n, err := launcher.Launch(ply, 1)