Split the registry client out so it can be used in more places #17994
Conversation
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/XL
smarterclayton
force-pushed
the
split_registry_client
branch
from
8c54b64
to
f2b5369
Compare
openshift-ci-robot
added
size/XXL
|
+1 |
| @@ -0,0 +1,86 @@ | |||
| package registryclient | |||
There was a problem hiding this comment.
You're copy&pasting this (at least parts) from pkg/image/importer/credentials.go but I don't see that file (those bits) being removed?
| type noopCredentialStore struct{} | ||
|
|
||
| func (s *noopCredentialStore) Basic(url *url.URL) (string, string) { | ||
| glog.Infof("asked to provide Basic credentials for %s", url) |
There was a problem hiding this comment.
While you're at it please remove these debug statements here and in the next few functions.
|
@smarterclayton is the intention to use the client in registry as well (I think registry also has its own client...?). If so, don't we want to extract this into separate repo or something that is vendorable? |
image-registry uses this client already (by vendoring origin) as i understand it. So yes, moving it to a shared repository so it's vendorable would be ideal but that's also true for a bunch of other code and i don't think we're in a position to take on that refactor 5 days before feature cut. |
The client was previously in pkg/image/importer but isn't specific to import. It's really a factory for working with arbitrary registries for both push and pull, so move it into its own package and trim its dependency set down to only docker/distribution, which makes it more portable to different codebases.
smarterclayton
force-pushed
the
split_registry_client
branch
from
f2b5369
to
6e076dd
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Updated. I split out more so the dependency tree is a lot clearer. registry client depends on docker distribution. registryclient/dockercredentials depends on kubernetes/pkg/credentialprovider. image/importer has anything kube api dependent. |
| _ "github.com/openshift/origin/pkg/api/install" | ||
| ) | ||
|
|
||
| type mockKeyring struct { |
There was a problem hiding this comment.
Same as before, please remove these duplicates from pkg/image/importer/credentials_test.go
There was a problem hiding this comment.
These are local private types. They aren't duplicates, since we can't import them.
|
Tagging based on not being able to export these things. |
|
Automatic merge from submit-queue (batch tested with PRs 17737, 18023, 17994). |
The client was previously in pkg/image/importer but isn't specific to
import. It's really a factory for working with arbitrary registries for
both push and pull, so move it into its own package and trim its
dependency set down to only docker/distribution, which makes it more
portable to different codebases.
@bparees helps use this code in more places