-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Allow stored templates to be referenced from osc process #1269
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 stored templates to be referenced from osc process #1269
Conversation
|
@bparees @smarterclayton PTAL |
|
Add a hack/test-cmd and you'll get sign off :) |
0e342b6 to
053bb3e
Compare
|
@smarterclayton done, waiting for travis |
|
LGTM [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1145/) (Image: devenv-fedora_1009) |
|
Evaluated for origin up to 053bb3e |
Merged by openshift-bot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you need to check this error in case the template didn't exist?
|
@smarterclayton @mfojtik given that @fabianofranz is implementing the "create from template" CLI with a full wizard flow, do we really want to have two code paths doing the same thing? |
|
Eh, templates are lower than apps. Apps won't give you all the flow that process does, nor the same flexibility.
|
|
My understanding was that what fabiano was doing was create from template, not create an app, and that it would have wizard flows, which would be as much or more flexibility than this offers. If that's incorrect then we probably should not have deleted the "create from template cli" card this morning that we decided was a dupe. Ben Parees | OpenShift -----Original Message----- Eh, templates are lower than apps. Apps won't give you all the flow that process does, nor the same flexibility.
Reply to this email directly or view it on GitHub: |
|
He would be integrating into new-app, which already has partial support for create from template. You can use templates as pieces parts and put them together.
|
|
@smarterclayton @bparees Originally the story targeted creating from templates in a wizardy way, but it's pretty old and surely some of the original targets might be deprecated. I know that at some point we decided to avoid user prompts and wizards in places other than We should adapt the work to what makes more sense today, either involving |
|
I'm out of the pocket but Cesar knows the vision
|
|
@smarterclayton I don't see anything in new-app that deals with create from template... can you elaborate on what's there? @csrwng welcome to the party. |
|
@smarterclayton you give me too much credit. I wish I knew the vision :-) Is the thinking that a template would be a source for components so that new-app can help put them together with other things? |
|
Just a bit left - it would be wired in like a component type and have equal weight with image repository names (above the docker registry).
|
|
Yes
|
|
Template would be a single component. So "mysql" template would have slightly higher priority over "mysql" image
|
|
@smarterclayton my fundamental confusion is over your initial statement that: "Apps won't give you all the flow that process does, nor the same flexibility" the only flexibility process gives you today is that you can pass -v to override template parameters, and i don't think we have any additional flexibility in mind to add. so i'm still not really seeing how "new-app mytemplate" and "process mytemplate" are going to be meaningfully different, unless you're stating that "new-app" should under no circumstance allow the explicilt specification/overriding of parameter values. |
|
(keeping in mind the "osc process mytemplate" doesn't actually create anything, but neither does "osc new-app mytemplate -o json") |
|
|
@smarterclayton that's fair, but in the case that i just run "new-app mytemplate" with no other parameters there is no guesswork for it to do (short of the question of whether there happens to be also be a docker image named "mytemplate"), so it should have the same outcome as "process mytemplate | create -f -" |
|
Yes, it should.
|
|
Except for labeling.
|
|
new-app isn't going to create labels when it processes the template? isn't that a function of the process endpoint? |
|
It is - label decisions are made at the app label and filtered down into the components (including the templates) in a way that is not the same as how process works.
|
|
@smarterclayton so what i keep hearing is "we'll have two slightly different ways to accomplish basically the same thing with minor variations of useful, but non-overlapping, capabilities". I think it should all be in new-app since that is the most feature rich interface right now. (ie parameter overriding ought to be added to new-app and then we can remove the osc process support for template resources) |
|
Reviving an old thread here, because it's related to the discussion (and I kept the tab open until then). Now that In my opinion the current I'd propose something like: Thoughts? @smarterclayton @bparees @mfojtik @csrwng |
|
If you want to script templates and inject your own custom rules you still want that as an admin. Power user does not mean "goes under openshift admin". Probably just means "not in the regular help list for new users".
|
|
I think that's too drastic of a change in osc create behavior at this point in time. osc create is tied to the rest interface to match with osc get, osc delete, etc. i'm also not convinced new-app is simpler or less powerful. Granted, process/create allows total control since you can manipulate the json, but then so does new-app since it can consume template json too. And new-app has a lot of other subtle functionality that process/create does not in terms of creating services/deployments directly from Images, ImageStreams, etc, that you cannot do with process/create (without writing json). If there's any argument here, i'd say it's that "osc process" could go away since new-app can consume templates now. (as long as new-app provides the Parameter overriding behavior that process offers, and if new-app could consume templates from disk not just storage) |
|
new-app changes the template as well, i don't think it would reduce to the process equivalent (today). Ben's points are valid. Create is "cat" or "vi", new-app is "emacs". new-app interprets "intent", process makes no such assumption.
|
|
Ok, I got your points. The osc/osadm proposal had actually the main intent of making clear to end users what the entry point for "creating stuff" is. Not displaying create and process in the main path, say in the regular help list for end users, would also attend that intention. That said, I'm still not confident about the "new-app" name - we are exposing the concept of "application" that we, ourselves, are trying to avoid by making "project" our container for components. But there are both @bparees new-app supports parameter overriding already, through the BTW as mentioned by Ben, do we plan to support templates from disk in |
|
|
@smarterclayton Meaning we do want to |
|
@smarterclayton I mean probably ok from an "advanced creation" screen, just not exposed as a regular path in the current creation screen, for example. |
|
As a small link or button in the corner "Have your own template?" only, yes.
|
|
Not right now. Maybe after 3.0
|
…service-catalog/' changes from 06b897d198..7011d9e816 7011d9e816 origin build: add origin tooling f6eac6e Merge branch 'pr/1322' 6fb9fe8 Drop TPR storage support d337ec4 Moving from global api.Scheme to local Scheme (openshift#1297) a2a9fcc Add referential integrity check on ServiceBroker for Service/Plan (openshift#1317) 1563a74 Revert "Update dependencies to Kubernetes 1.7.6" (openshift#1316) 70e546b Fix json tags for parameters fields (openshift#1312) dcde551 Update to Kubernetes 1.7.6 (openshift#1262) 48e1e53 Add spec.serviceBrokerName field to plan (openshift#1307) 2c43744 add unit tests specifically for resolveReferences (openshift#1314) 6409e2f Resolve instance refs in ReconcileServiceInstance (openshift#1305) bee6afe Fix http verbs supported by servicebrokers/status (openshift#1294) 54199f8 Update walkthrough and correct use of secrets in parameters (openshift#1308) d00f2e3 Add note advising users on content of contrib/pkg (openshift#1277) 66f72b4 v0.0.22 updates (openshift#1306) 1e52673 use canary images when developing (openshift#1260) b26491e Fix HTTP verbs supported by serviceinstances/status (openshift#1302) 44ff690 Fix HTTP verbs supported by serviceinstancecredentials/status (openshift#1304) bbd4d05 Correct JSON tags in v1alpha1 fields (openshift#1301) 766311e Add missing JSON tags (openshift#1295) 3c672ba openshift#1278 - Flaky TestBasicFlowsWithOriginatingIdentity (openshift#1281) 904c236 Add warning to Parameters field doc about sensitive information (openshift#1287) 0c0035c Store in-progress and external properties in instance and binding status. (openshift#1250) 6ab6492 add instance orphan mitigation (openshift#1248) d9d0ea8 Make build of user-broker dependent upon files in contrib/pkg/broker (openshift#1282) 5c99c28 Updated bin/e2e.test to depend on all source (openshift#1280) 454645e Implement changes to k8s naming of ServiceClass and ServicePlan (openshift#1249) 1c81228 remove redundant 'old' validation in UPDATE (openshift#1269) 9c29cfe Wait for test-broker Service endpoint to be available (openshift#1270) 8663c0a Add unstructured serialization test (openshift#1263) 957477f Merge branch 'pr/1274' 1de013b fix http error nil dereference (openshift#1273) 36ba252 Fix race condition in admission controllers that use ServicePlan (openshift#1272) e3c1e86 Remove fmt.Println statements from serialization test 94f8f63 Make controller-manager health check less chatty in logs (openshift#1267) bd70dd4 Move pkg/brokerapi to contrib/pkg/brokerapi (openshift#1255) a38209c openshift#1149 - block concurrent updates to ServiceInstance and ServiceInstanceCredential (openshift#1213) 14dda52 Follow on work from 1252 (openshift#1264) 776fce1 docs/install-1.7.md: clarify that RBAC is optional (openshift#1245) 9064bf3 Add Spec to ServiceClass and ServicePlan (openshift#1252) 277abcd Add option to helm charts for enabling OriginatingIdentity feature (openshift#1251) c9a19c8 Clarify ready condition and event reasons when deprovision is blocked by existing ServiceInstanceCredentials (openshift#1258) d911a5e Add missing test for ServicePlans (openshift#1257) e7cc973 Remove (unused) test dependency on pkg/brokerapi/fake (openshift#1253) 010d6e1 Merge branch 'pr/1240' 170aab5 split plans off of service classes (openshift#1106) a3c6fc7 Chart updates for 0.0.21 (openshift#1244) b55c94e Remove dependency on pkg/brokerapi REVERT: 06b897d198 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 7011d9e81649fb3e3f563375b69a9b2f79916b9a
Examples: