cli icon indicating copy to clipboard operation
cli copied to clipboard

fixes registry logic for case:

Open rdallman opened this issue 7 years ago • 9 comments

previously, we if the following value for FN_REGISTRY was provided, we ignored it if the image contained a slash, see example:

(func.yaml has image: rdallman/yodawg)

$ FN_REGISTRY=localhost:5000/ cli deploy --app myapp
Deploying rdallman/yodawg to app: myapp at path: /hello
Bumped to version 0.0.7
Building image rdallman/yodawg:0.0.7 ...
Pushing rdallman/yodawg:0.0.7 to docker registry...The push refers to a repository [docker.io/rdallman/yodawg]

with this patch, it's fixed:

$ FN_REGISTRY=localhost:5000/ cli deploy --app myapp
Deploying rdallman/yodawg to app: myapp at path: /hello
Bumped to version 0.0.11
Building image localhost:5000/rdallman/yodawg:0.0.11 .
Pushing localhost:5000/rdallman/yodawg:0.0.11 to docker registry...The push refers to a repository [localhost:5000/rdallman/yodawg]

this does mean that anyone relying on the logic of their image having a slash in it not using an FN_REGISTRY they provided (seems odd) will be broken, the code kind of implies this was the case but maybe not. anyway, this behavior seems like what we want, i.e. 'if a REGISTRY_URL is provided, prepend it to everything unless it's a fully qualified image that specifies a registry URL itself'. to enumerate:

FN_REGISTRY!="" && image=="yodawg" -> image = FN_REGISTRY || image FN_REGISTRY!="" && image=="yo/dawg" -> image = FN_REGISTRY || image FN_REGISTRY!="" && image=="localhost:5000/yo/dawg" -> image = image

of course, if FN_REGISTRY is not provided, the image is not modified. shame this thing exists :(

rdallman avatar Jan 19 '18 21:01 rdallman

closes https://github.com/fnproject/fn/issues/699 -- the image was never pushed to the registry. i can confirm that using this version of the cli, an image provided with a registry ends up in there, and fn server also pulls it and runs it correctly.

rdallman avatar Jan 19 '18 21:01 rdallman

meh, this FN_REGISTRY idea is dumb.

a hostname (without a port) and a user name will both look the same, i'm not sure it's possible to move forward...

FN_REGISTRY is a host and a host or username, prepend FN_REGISTRY is a user name and image is a single word (no slash), prepend FN_REGISTRY is a user name and image has username (slash), don't prepend

since host and user name can look the same... there's no identifiable way around this. can we toss out the user name concept please? a user is not a registry. I understand the thinking around it. FN_REGISTRY_USERNAME makes this all make a whole lot more sense.

rdallman avatar Jan 19 '18 21:01 rdallman

I think making FN_REGISTRY fully qualified makes sense, eg: FN_REGISTRY=docker.io/NAMESPACE .

Would that be enough to resolve all of this? Docker refers to it as namespace since it can be an org too, not just a username.

treeder avatar Feb 12 '18 16:02 treeder

I think making FN_REGISTRY fully qualified makes sense, eg: FN_REGISTRY=docker.io/NAMESPACE .

Would that be enough to resolve all of this? Docker refers to it as namespace since it can be an org too, not just a username.

good idea, that might work. the only confusing case then is if the function 'name' (image) is NAMESPACE/image and a user provides my.registry.io/path/to/NAMESPACE we have to blow up, since if FN_REGISTRY is required to be fully qualified a user can't simply use FN_REGISTRY=my.registry.io. allowing / suggesting FN_REGISTRY to contain NAMESPACE is mostly the source of issues, but I'm not sure there's any getting around it completely.

rdallman avatar Feb 12 '18 18:02 rdallman

What if we don't allow NAMESPACE in the function names (shouldn't really be there anyways)?

treeder avatar Feb 12 '18 19:02 treeder

I don't really like the whole FN_REGISTRY experience to begin with, asking the wrong dude. ya got the wrong lebowski

rdallman avatar Feb 12 '18 19:02 rdallman

it's already relatively confusing that function name becomes an image name, i'm not sure that makes that any better. the other point would be it's a total pain if you're on a team with a shared repo you'd have to set FN_REGISTRY in some build script or your env to toggle between various things. in a team that uses different orgs/repo names to box stuff like dev/hello stage/hello in a shared registry, this becomes a real pain since that can no longer be encoded into the function/image name. it makes sense to me to allow a full my.registry.io/my/docker as an image name, i maintain it's confusing this is a function name esp in that context. personally i prefer not using FN_REGISTRY at all since i have various functions that go to fnproject or to rdallman on dhub. would be nice to resolve all this for sure..

rdallman avatar Feb 12 '18 19:02 rdallman

@rdallman seems this should be closed off - any objections?

rikgibson avatar Aug 28 '18 17:08 rikgibson

afaik this still doesn't work, there's an ambiguity to resolve around FN_REGISTRY's definition, per https://github.com/fnproject/cli/pull/144#issuecomment-359091261 -- maybe it's fixed now?

rdallman avatar Aug 28 '18 17:08 rdallman