continue updating code per review comments

Signed-off-by: wang yan <wangyan@vmware.com>

src/registryctl/api/registry/blob/blob.go

@@ -3,19 +3,23 @@ package blob
 import (
 	"errors"
 	"github.com/docker/distribution/registry/storage"
+	storagedriver "github.com/docker/distribution/registry/storage/driver"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/registryctl/api"
-	regConf "github.com/goharbor/harbor/src/registryctl/config/registry"
 	"github.com/gorilla/mux"
 	"net/http"
 )
 
 // NewHandler returns the handler to handler blob request
-func NewHandler() http.Handler {
-	return &handler{}
+func NewHandler(storageDriver storagedriver.StorageDriver) http.Handler {
+	return &handler{
+		storageDriver: storageDriver,
+	}
 }
 
-type handler struct{}
+type handler struct {
+	storageDriver storagedriver.StorageDriver
+}
 
 // ServeHTTP ...
 func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
@@ -35,7 +39,7 @@ func (h *handler) delete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	// don't parse the reference here as RemoveBlob does.
-	cleaner := storage.NewVacuum(r.Context(), regConf.StorageDriver)
+	cleaner := storage.NewVacuum(r.Context(), h.storageDriver)
 	if err := cleaner.RemoveBlob(ref); err != nil {
 		log.Infof("failed to remove blob: %s, with error:%v", ref, err)
 		api.HandleError(w, err)

src/registryctl/api/registry/blob/blob_test.go

@@ -46,7 +46,7 @@ func TestDeletionBlob(t *testing.T) {
 	varMap["reference"] = test.GetKeys(randomLayers1)[0].String()
 	req = mux.SetURLVars(req, varMap)
 
-	blobHandler := NewHandler()
+	blobHandler := NewHandler(regConf.StorageDriver)
 	rec := httptest.NewRecorder()
 	blobHandler.ServeHTTP(rec, req)
 	assert.True(t, rec.Result().StatusCode == 200)

src/registryctl/api/registry/manifest/manifest.go

@@ -2,22 +2,25 @@ package manifest
 
 import (
 	"github.com/docker/distribution/registry/storage"
+	storagedriver "github.com/docker/distribution/registry/storage/driver"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/registryctl/api"
-	regConf "github.com/goharbor/harbor/src/registryctl/config/registry"
 	"github.com/gorilla/mux"
 	"github.com/opencontainers/go-digest"
 	"net/http"
-	"strings"
 )
 
 // NewHandler returns the handler to handler manifest request
-func NewHandler() http.Handler {
-	return &handler{}
+func NewHandler(storageDriver storagedriver.StorageDriver) http.Handler {
+	return &handler{
+		storageDriver: storageDriver,
+	}
 }
 
-type handler struct{}
+type handler struct {
+	storageDriver storagedriver.StorageDriver
+}
 
 // ServeHTTP ...
 func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
@@ -46,12 +49,9 @@ func (h *handler) delete(w http.ResponseWriter, r *http.Request) {
 		api.HandleBadRequest(w, errors.New("no repository name specified"))
 		return
 	}
+	// let the tags as empty here, as it non-blocking GC. The tags deletion will be handled via DELETE /v2/manifest
 	var tags []string
-	v := r.URL.Query()
-	queryTags := v.Get("tags")
-	tags = strings.Split(queryTags, ",")
-
-	cleaner := storage.NewVacuum(r.Context(), regConf.StorageDriver)
+	cleaner := storage.NewVacuum(r.Context(), h.storageDriver)
 	if err := cleaner.RemoveManifest(repoName, dgst, tags); err != nil {
 		log.Infof("failed to remove manifest: %s, with error:%v", ref, err)
 		api.HandleInternalServerError(w, err)

src/registryctl/api/registry/manifest/manifest_test.go

@@ -57,7 +57,7 @@ func TestDeleteManifest(t *testing.T) {
 	varMap["name"] = fmt.Sprintf("%v", repo.Named())
 	req = mux.SetURLVars(req, varMap)
 
-	manifestHandler := NewHandler()
+	manifestHandler := NewHandler(regConf.StorageDriver)
 	rec := httptest.NewRecorder()
 	manifestHandler.ServeHTTP(rec, req)
 	assert.True(t, rec.Result().StatusCode == 200)

src/registryctl/config/config.go

@@ -15,6 +15,11 @@
 package config
 
 import (
+	"fmt"
+	"github.com/docker/distribution/configuration"
+	storagedriver "github.com/docker/distribution/registry/storage/driver"
+	"github.com/docker/distribution/registry/storage/driver/factory"
+	"github.com/goharbor/harbor/src/lib/log"
 	"io/ioutil"
 	"os"
 
@@ -33,7 +38,8 @@ type Configuration struct {
 		Cert string `yaml:"cert"`
 		Key  string `yaml:"key"`
 	} `yaml:"https_config,omitempty"`
-	RegistryConfig string `yaml:"registry_config"`
+	RegistryConfig string                      `yaml:"registry_config"`
+	StorageDriver  storagedriver.StorageDriver `yaml:"-"`
 }
 
 // Load the configuration options from the specified yaml file.
@@ -53,6 +59,30 @@ func (c *Configuration) Load(yamlFilePath string, detectEnv bool) error {
 		c.loadEnvs()
 	}
 
+	if err := c.setStorageDriver(); err != nil {
+		log.Errorf("failed to load storage driver, err:%v", err)
+		return err
+	}
+
+	return nil
+}
+
+// setStorageDriver set the storage driver according the registry's configuration.
+func (c *Configuration) setStorageDriver() error {
+	fp, err := os.Open(c.RegistryConfig)
+	if err != nil {
+		return err
+	}
+	defer fp.Close()
+	rConf, err := configuration.Parse(fp)
+	if err != nil {
+		return fmt.Errorf("error parsing registry configuration %s: %v", c.RegistryConfig, err)
+	}
+	storageDriver, err := factory.Create(rConf.Storage.Type(), rConf.Storage.Parameters())
+	if err != nil {
+		return err
+	}
+	c.StorageDriver = storageDriver
 	return nil
 }
 

src/registryctl/config/config_test.go

@@ -19,6 +19,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	_ "github.com/docker/distribution/registry/storage/driver/filesystem"
 )
 
 func TestConfigDoesNotExists(t *testing.T) {
@@ -38,7 +40,7 @@ func TestConfigLoadingWithEnv(t *testing.T) {
 	assert.Equal(t, "https", cfg.Protocol)
 	assert.Equal(t, "1000", cfg.Port)
 	assert.Equal(t, "DEBUG", cfg.LogLevel)
-	assert.Equal(t, "/etc/registry/config.yml", cfg.RegistryConfig)
+	assert.Equal(t, "../reg_conf_test.yml", cfg.RegistryConfig)
 }
 
 func TestConfigLoadingWithYml(t *testing.T) {
@@ -48,7 +50,8 @@ func TestConfigLoadingWithYml(t *testing.T) {
 	assert.Equal(t, "http", cfg.Protocol)
 	assert.Equal(t, "1234", cfg.Port)
 	assert.Equal(t, "ERROR", cfg.LogLevel)
-	assert.Equal(t, "/etc/registry/config.yml", cfg.RegistryConfig)
+	assert.Equal(t, "../reg_conf_test.yml", cfg.RegistryConfig)
+	assert.True(t, cfg.StorageDriver.Name() == "filesystem")
 }
 
 func TestGetLogLevel(t *testing.T) {

src/registryctl/config_test.yml

@@ -7,4 +7,4 @@ https_config:
   cert: "server.crt"
   key: "server.key"
 
-registry_config: "/etc/registry/config.yml"
+registry_config: "../reg_conf_test.yml"

src/registryctl/handlers/router.go

@@ -32,7 +32,7 @@ func newRouter(conf config.Configuration) http.Handler {
 	rootRouter.HandleFunc("/api/health", api.Health).Methods("GET")
 
 	rootRouter.Path("/api/registry/gc").Methods(http.MethodPost).Handler(gc.NewHandler(conf.RegistryConfig))
-	rootRouter.Path("/api/registry/blob/{reference}").Methods(http.MethodDelete).Handler(blob.NewHandler())
-	rootRouter.Path("/api/registry/{name}/manifests/{reference}").Methods(http.MethodDelete).Handler(manifest.NewHandler())
+	rootRouter.Path("/api/registry/blob/{reference}").Methods(http.MethodDelete).Handler(blob.NewHandler(conf.StorageDriver))
+	rootRouter.Path("/api/registry/{name}/manifests/{reference}").Methods(http.MethodDelete).Handler(manifest.NewHandler(conf.StorageDriver))
 	return rootRouter
 }

src/registryctl/main.go

@@ -17,8 +17,6 @@ package main
 import (
 	"crypto/tls"
 	"flag"
-	"github.com/docker/distribution/registry/storage/driver/factory"
-	regConf "github.com/goharbor/harbor/src/registryctl/config/registry"
 	"net/http"
 
 	common_http "github.com/goharbor/harbor/src/common/http"
@@ -69,7 +67,6 @@ func (s *RegistryCtl) Start() {
 }
 
 func main() {
-
 	configPath := flag.String("c", "", "Specify registryCtl configuration file path")
 	flag.Parse()
 
@@ -85,18 +82,5 @@ func main() {
 		ServerConf: *config.DefaultConfig,
 		Handler:    handlers.NewHandlerChain(*config.DefaultConfig),
 	}
-
-	// set the global driver
-	rConf, err := regConf.ResolveConfiguration(config.DefaultConfig.RegistryConfig)
-	if err != nil {
-		log.Error(err)
-		return
-	}
-	regConf.StorageDriver, err = factory.Create(rConf.Storage.Type(), rConf.Storage.Parameters())
-	if err != nil {
-		log.Error(err)
-		return
-	}
-
 	regCtl.Start()
 }