Add api to delete blob and manifest #12006
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12006 +/- ##
==========================================
- Coverage 55.79% 55.77% -0.03%
==========================================
Files 915 917 +2
Lines 53108 53234 +126
Branches 1797 1797
==========================================
+ Hits 29633 29690 +57
- Misses 20174 20230 +56
- Partials 3301 3314 +13
|
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
50e4333
to
f02e21a
Compare
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
f02e21a
to
3703768
Compare
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
4 times, most recently
from
27140ff
to
83b2195
Compare
Enable the capability of registry controller to delete blob and manifest Signed-off-by: wang yan <wangyan@vmware.com>
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
83b2195
to
c9fa405
Compare
src/go.mod
Outdated
| @@ -83,3 +87,5 @@ require ( | |||
| k8s.io/client-go v0.17.3 | |||
| k8s.io/helm v2.16.3+incompatible | |||
| ) | |||
|
|
|||
| replace github.com/Azure/go-autorest => github.com/Azure/go-autorest v13.3.3+incompatible | |||
There was a problem hiding this comment.
why we need the replace here?
why can't we put the replace statements together?
There was a problem hiding this comment.
to resolve the azure go-autotest versioning issue, similar things in k8s world:
https://github.com/kubernetes-sigs/external-dns/blob/master/go.mod#L68
https://github.com/kubernetes/kubernetes/blob/master/go.mod#L157
|
I find there are more files that need to be vendored than I expected. |
| } | ||
| var tags []string | ||
| v := r.URL.Query() | ||
| queryTags := v.Get("tags") |
There was a problem hiding this comment.
the tags query is not mentioned in
goharbor/community#133
Would it be better if we write some doc for the rest api? It may or may not be swagger as we don't plan to allow user to write client at least in 2.1.0
There was a problem hiding this comment.
Do we need tags here? Since 2.0, tags may exist in harbor db but not exist in registry.
There was a problem hiding this comment.
The fundamental method needs this pararmeter, and it can be a null. Leave it at here will not impact GC deletes manifest via digest only.
And it's a query string.
There was a problem hiding this comment.
I feel this may reflect a misunderstanding here, I don't think the parm "tags" makes sense for this API...
There was a problem hiding this comment.
ok, I will remove it.
| "github.com/goharbor/harbor/src/registryctl/api/registry/gc" | ||
| ) | ||
|
|
||
| // const definition |
There was a problem hiding this comment.
This is the client core used to call registryctl's API right?
We should attach request ID and extract request ID on the server side.
If you don't want to handle it in this PR could you open an issue?
There was a problem hiding this comment.
I will raise an issue for it, and handle it in another PR.
| @@ -100,4 +101,9 @@ func (c *Configuration) loadEnvs() { | |||
| c.LogLevel = loggerLevel | |||
| } | |||
|
|
|||
| registryConf := os.Getenv("REGISTRY_CONFIG") | |||
There was a problem hiding this comment.
is this necessary?
Is this for making testing easier?
There was a problem hiding this comment.
it's easier for testing &debugging.
| ) | ||
|
|
||
| // StorageDriver ... | ||
| var StorageDriver storagedriver.StorageDriver |
There was a problem hiding this comment.
Why not make storagedriver part of the config of registryctl?
There was a problem hiding this comment.
The configuraitons of the driver are arleady in the registry configuration file, and the driver is inited on registryctl container launch. I think it's ok to use a varaible here.
There was a problem hiding this comment.
I strongly think putting in in config is a better design.
I'll let @heww chime in ,if he's OK with this I'm fine
There was a problem hiding this comment.
If putting StorageDriver in the config of the registryctl, it will be clear in the main of the registryctl.
In the Load method of the registryctl conf, we can resolve configuration of the distrubtion and create StorageDriver from the configurtion. The result is that is subpackage config/registry can be removed.
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
09b833f
to
b237db7
Compare
Signed-off-by: wang yan <wangyan@vmware.com>
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
b237db7
to
fb3f06c
Compare
|
@reasonerjt @heww code is updated and comments added, can you help to review? |
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
2 times, most recently
from
5c8d61f
to
30b9367
Compare
Signed-off-by: wang yan <wangyan@vmware.com>
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
30b9367
to
d9e1b48
Compare
| _ "github.com/docker/distribution/registry/storage/driver/middleware/redirect" | ||
| _ "github.com/docker/distribution/registry/storage/driver/oss" | ||
| _ "github.com/docker/distribution/registry/storage/driver/s3-aws" | ||
| _ "github.com/docker/distribution/registry/storage/driver/swift" |
There was a problem hiding this comment.
These drivers can be move to config pkg as the storage driver is created there.
There was a problem hiding this comment.
a blank import should be only in a main or test package
Signed-off-by: wang yan <wangyan@vmware.com>
Signed-off-by: wang yan <wangyan@vmware.com>
wy65701436
force-pushed
the
non-blocking-gc-api-blob-delete
branch
from
f9380cd
to
ebecc8b
Compare
* Add api to delete blob and manifest Enable the capability of registry controller to delete blob and manifest Signed-off-by: wang yan <wangyan@vmware.com> Signed-off-by: Ye Liu <ye.liu@hp.com>
* Add api to delete blob and manifest Enable the capability of registry controller to delete blob and manifest Signed-off-by: wang yan <wangyan@vmware.com> Signed-off-by: molinber <bertrand.molin@credit-agricole.net>
* Add api to delete blob and manifest Enable the capability of registry controller to delete blob and manifest Signed-off-by: wang yan <wangyan@vmware.com>
Enable the capability of registry controller to delete blob and manifest
Signed-off-by: wang yan wangyan@vmware.com