Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow bring-your-own revision name #276

Closed
sixolet opened this issue Jul 19, 2019 · 7 comments
Closed

Allow bring-your-own revision name #276

sixolet opened this issue Jul 19, 2019 · 7 comments
Assignees
Labels
kind/feature New feature or request
Milestone

Comments

@sixolet
Copy link
Contributor

sixolet commented Jul 19, 2019

In what area(s)?

Supported parameters

Describe the feature:

v1beta1 style (even in v1alpha1) allows "bring your own" revision names. Let's allow users to bring 'em in service creation .

--revision-name or --revision-name-prefix (or even --name-prefix) can be the name of the flag.

@sixolet sixolet added the kind/feature New feature or request label Jul 19, 2019
@sixolet sixolet self-assigned this Jul 19, 2019
@duglin
Copy link
Contributor

duglin commented Jul 19, 2019

why the "prefix" variants? It's not a prefix w.r.t. naming the revision.

Of those I like --revision-name even thought it's kind of long. --name would be nice but that's probably not clear enough on which thing we're naming. --revname might be too short, but it is an option.

kn service create myapp --image ns/repo --revname myapp-v1

re: prefix... what's interesting is that if we don't ask for the full name, if anything it would be postfix since I think all revNames must start with a prefix that matches the service name.

kn service create myapp --image ns/repo --revpostfix=v1 ?
kn service create myapp --image ns/repo --posfix=v1 ?
kn service create myapp --image ns/repo --name-postfix=v1 ?
kn service create myapp --image ns/repo --revision=v1 ?
kn service create myapp --image ns/repo --version=v1 ?

We just need to make sure that whatever we use it'll be an obvious mapping to the revision name in the traffic section.

kn service create myapp --image ns/repo --version v1 --traffic v1:50 --traffic v2:50
kn service create myapp --image ns/repo --revision v1 --traffic v1:50 --traffic v2:50

I can't decide if it's really helpful to not force them to duplicate the KnService name over and over, or hiding too much. But it that does feel nicer than something like:
kn service create myapp --image ns/repo --revision-name myapp-v1 --traffic myapp-v1:50 --traffic myapp-v2:50

So far, I like this one the best:
kn service create myapp --image ns/repo --revision v1 --traffic v1:50 --traffic v2:50

@rhuss
Copy link
Contributor

rhuss commented Jul 19, 2019

I can't decide if it's really helpful to not force them to duplicate the KnService name over and over, or hiding too much.

Think about kn revision list (or kubectl get revision) with multiple services active. Of course, you have a service column but the list is sorted after revision name. Having a common prefix like the service name groups them nicely in such name sorted lists which is quite common.

@duglin
Copy link
Contributor

duglin commented Jul 19, 2019

yup - wasn't questioning it in lists like "revision list" - was just wondering about it on the cmd line for specifying traffic or revision name. E.g.
kn service create myapp --image ns/repo --revision-name myapp-v1 --traffic myapp-v1:50 --traffic myapp-v2:50
it's not clear asking for "myapp" 3 extra times (when they aren't allowed to put anything else) is useful.

@sixolet
Copy link
Contributor Author

sixolet commented Jul 19, 2019

Do y'all think it'd be a good idea to have "anywhere a revision name is needed, you can specify only the part after myapp- if you want"?

@rhuss
Copy link
Contributor

rhuss commented Jul 19, 2019

Yes, but only when the revision trunk name is specified on the very same command line, too, and you can still use the full revision name and there is no ambiguity.

In the situation above we have to probe for allowed names anyway (like whether you specified a tag instead of a revision name if we want to avoid the confusing triple-spec of traffic targets like suggested in @navidshaikh proposal), so I would be fine if we have clearly defined and easy to understand probe strategy.

As we should always support the full revision name anyway, I would start with that and then decide later whether we want to allow a kind of shortcut.

@duglin
Copy link
Contributor

duglin commented Jul 19, 2019

Do y'all think it'd be a good idea to have "anywhere a revision name is needed, you can specify only the part after myapp- if you want"?

Perhaps but I think we need to look at each case individually to see if it makes sense and feels natural. The case I mentioned above would qualify for me.

@rhuss's idea of supporting both seems reasonable - we'd just need to watch for cases like myapp-foo because technically they could really mean myapp-myapp-foo. But it's kind of a weird case.

@navidshaikh
Copy link
Collaborator

Fixed with #282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants