Conversation
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`) ``sh $ 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: ```sh $ 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 :(
|
closes fnproject/fn#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. |
|
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 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. |
|
|
||
| // prefix FN_REGISTRY, if provided (important) AND if the image does not | ||
| // itself contain a registry. | ||
| if len(strings.SplitN(fname, "/", 3)) < 3 { |
There was a problem hiding this comment.
localhost:5000/mygroup/myimage would match this though?
There was a problem hiding this comment.
this one works https://play.golang.org/p/eY2LSGhSDr7 but i think ultimately this logic is flawed trying to have FN_REGISTRY either be a username, a registry, or a registry with username. it seems like the proper fix is making FN_REGISTRY be a registry, and having an additional FN_REGISTRY_USERNAME for cli convenience shenanigans, maybe something slightly easier there but I'm not sure.
There was a problem hiding this comment.
yeah I'm all for moving to more explicit values for those vars. 👍
|
I think making FN_REGISTRY fully qualified makes sense, eg: 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 |
|
What if we don't allow NAMESPACE in the function names (shouldn't really be there anyways)? |
|
I don't really like the whole FN_REGISTRY experience to begin with, asking the wrong dude. ya got the wrong lebowski |
|
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 |
|
@rdallman seems this should be closed off - any objections? |
|
afaik this still doesn't work, there's an ambiguity to resolve around |
previously, we if the following value for FN_REGISTRY was provided, we ignored
it if the image contained a slash, see example:
(
func.yamlhasimage: 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 :(