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

initial template and parameterization proposal #18215

Merged
merged 1 commit into from Feb 9, 2016

Conversation

bparees
Copy link
Contributor

@bparees bparees commented Dec 4, 2015

@bgrant0607 @brendandburns @smarterclayton @derekwaynecarr

This is a proposal to define a syntax for authoring templates that represent k8s object topologies, while allowing those templates to be customized prior to instantiation via a parameterization mechanism.

Related discussions/template proposals:
#14918
#14993
#4210
#10408 (comment)
#11492

@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-github-robot k8s-github-robot added kind/design Categorizes issue or PR as related to design. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 4, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 4, 2015

GCE e2e test build/test passed for commit 5da37eb6715bc34e5ac314ddd7fff51d8b33a54a.

@bgrant0607
Copy link
Member

Thanks. WTAL

@bgrant0607
Copy link
Member

cc @kubernetes/sig-config

### Use cases for templates in general

* Providing a full baked application experience in a single portable object that can be repeatably deployed in different environments.
* eg Wordpress deployment with separate standalone database pod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: a standalone pod is an anti-pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean a pod w/o a replication controller? yeah sorry i was a bit sloppy with my language here. I was only trying to distinguish from "single pod containing both a web app an db container", not imply there wasn't also a replication controller managing the DB pod. i'll clarify in the next revision.


*It should be possible to validate templates and template parameters, both values and the schema.*

* Template objects are subject to standard api validation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except when the value does not validate - such as replacement in names ($( is not valid)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to validate the template field names.

As for values, if we had jsonschema-like validation specifications for parameters and/or for the resource fields, we could validate the values to some degree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except when the value does not validate - such as replacement in names ($( is not valid)

we don't validate the objects within the template definition, they are just runtime.Object, right? i'm just saying the template api object is validated as such, not that the things it contains are validated as whatever type they are. that does not happen (and as you note, really can't happen until after substitution happens. This situation will be even worse once we add support for substitution into integer fields)

@bgrant0607
Copy link
Member

I think we need something like this.

Kubernetes is about automating operations:
https://github.com/kubernetes/kubernetes/blob/master/docs/whatisk8s.md

Templates could also be used for a new generation of controllers #14961, like PetSet #18016, replacing the original idea of just pod templates #170.

We also have at least half a dozen cases in flight where we need parameter substitution.

solutions. (tldr: a syntax like ${FOO | int} could instruct the template processor to convert the json field to an integer when doing the
substitution. This could also support secret generation by using a syntax like `${FOO | base64}` which would take the value of parameter
`$FOO` and base64 encode it before setting the json field to the resulting value)
* Another option would be to add type information to the parameter definitions, making it slightly easier to reference a parameter within a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use the swagger to get the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only if template processing becomes schema aware which it currently is not. will add it as an option.

@bparees
Copy link
Contributor Author

bparees commented Dec 8, 2015

@bgrant0607 @smarterclayton addressed some of your comments, in particular changed to the $() syntax and added making template processing schema aware to the list of ways to address typed parameter substitution.

I have not addressed @bgrant0607's request to support a map of parameter values + template for instantiation, i'd like to see a little more discussion on the use case for that given that we've so far not seen a need for it in openshift. blind submission of parameter key/value pairs for a template seems likely to lead to failure for a user.

@k8s-bot
Copy link

k8s-bot commented Dec 8, 2015

GCE e2e test build/test passed for commit 2d0df6401f8f8921c71e7e57130825f8ae23fc9b.

* Utilizes multiple files with different syntax to define an application topology.
* Does not define a first class API object.
* Does not appear to natively offer generation logic for producing parameter values.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preceding statements about DM are inaccurate. Please read the DM documentation at https://github.com/kubernetes/deployment-manager, including the contents of the docs folder, and correct these statements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utilizes multiple files with different syntax to define an application topology.

this example uses 4 files including both yaml and jinja formats: https://github.com/kubernetes/deployment-manager/tree/master/examples/wordpress

the docs also indicate templates can be written in python. none of the examples/formats match k8s api objects/syntax.

Does not define a first class API object.

see above, what DM defines/consumes is not a first class k8s api object. Perhaps there's some confusion in what i meant by api object, I did not mean that DM doesn't define an api for itself, I meant it does not define api objects that are consumed by k8s. I can clarify this statement.

Does not appear to natively offer generation logic for producing parameter values.

I understand DM has parameters, i don't see native logic for defining how a parameter value should be generated (eg generate a random password). Presumably it's possible to make that happen via jinja or python, but I wouldn't consider that native support within DM, hence my statement. Again, I can clarify this to indicate value generation is deferred to jinja/python.

If I am still not understanding the design of DM, I'm afraid I'm going to need you to be more explicit about what is inaccurate/what I have misunderstood.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DM defines a single template usage format. It's a YAML based list of resource, each of which contains a name, type and parameters. True, the template definition can be written either in Jinja, which is simply marked up YAML, or Python, which produces YAML. A Jinja template for a k8s primitive is easily recognizable as a k8s configuration file with mustache markup.

Also, there's no requirement to use both languages in a single example, or to use multiple files, so both assertions are inaccurate.

DM can also take vanilla k8s configuration files as input. And, when a template is expanded, the result is always a flat list of vanilla k8s configuration files, which is readily accessible via the API.

DM doesn't define a k8s API object specifically because a native implementation baked into k8s has been discouraged by the preceding discussion. If that has somehow changed, we could easily make a DM template a native API object. However, we believe there are good reasons not to do that, not least of which is the well established precedent of publishing content in registries, rather than in runtimes. That said, when a configuration is deployed using DM, it is capture and stored in the cluster, providing a record of the deployment, including the parameters and expansion results.

If by native support for generation logic, you mean hard wired, as opposed to programmable, then no, DM doesn't have hard wired generators. We consider hard wired implementations to be an anti-pattern, in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there's no requirement to use both languages in a single example, or to use multiple files, so both assertions are inaccurate.

is it possible to write a single file, in k8s-compatible json syntax, which provides parameterization? That is what is achieved by the openshift template schema, and what I am trying to distinguish here as a desirable trait. Whether an example can be created which doesn't need additional syntax or files isn't really my point, the question is whether the full power of DM can be realized without branching out into multiple files and multiple syntaxes (above and beyond the k8s syntax).

My impression is that the answer is no. Is it a desirable goal (single file, k8s syntax, api object)? in my opinion, based on the user experience we have been able to deliver in openshift by integrating templates as a first class concept, is that it is, but I think we'll be hashing that out in the syntax/schema discussion once I update this PR per our discussion yesterday.

Regarding parameter generation, I agree with the concerns about it being compiled into the runtime and it would be nice to make it more flexible. On the other hand, by being a little opinionated here, we provided a generation function that covers probably 70% of the use cases for parameter generation (no one has come to us so far asking for additional generation functionality) in an extremely consumable format that minimizes complexity or requirements to learn new languages.

@bparees
Copy link
Contributor Author

bparees commented Feb 8, 2016

@bgrant0607: comments addressed in latest commit.

@k8s-bot
Copy link

k8s-bot commented Feb 8, 2016

GCE e2e test build/test passed for commit 246fd71db09197fc463a11f748710d171cf86807.

@bgrant0607
Copy link
Member

LGTM. Please rebase, run hack/update-generated-docs.sh, and squash, and then I'll apply the label for merging.

@bparees
Copy link
Contributor Author

bparees commented Feb 9, 2016

@bgrant0607 done.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit a2b5112aefc6cdba2ef5639a8ab8bdad5d952f8d.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit 89a6561.

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 9, 2016
@bgrant0607
Copy link
Member

LGTM!

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Feb 9, 2016
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 33636a3 into kubernetes:master Feb 9, 2016
@bparees
Copy link
Contributor Author

bparees commented Feb 9, 2016

thanks @bgrant0607. Hopefully we can find some people to start the implementation come early march.

@jimmycuadra
Copy link
Contributor

Not sure if this is the best place to discuss this now that the proposal has been merged or whether or I should start a new issue, but:

I've started on a CLI tool that attempts to implement client-side template processing using the format described in this proposal. You can find the program at https://github.com/InQuicker/ktmpl. It's written in Rust, but the details of the source code are not really important for discussion.

There are a few issues I've coming across while working on this implementation (some minor, some more significant):

  • The example shown in the proposal is not valid JSON. (Extra comma, I think?)
  • The example shown in the proposal uses the default value "password" for the parameter "MONGODB_USER" which is confusing and initially made me think my program was putting values in the wrong place until I went back and looked at the original text.
  • The example shown in the proposal does not include interpolations that cover all the cases described elsewhere in the proposal. I know it's just an example and not a comprehensive test suite, but without a more detailed specification, there wasn't much else to go on. Specifically, there are no examples of the two types of interpolation combined in one value, and there aren't examples of each of the types of interpolation surrounded by literal text.
  • ktmpl only supports YAML for now, since it's for my company's use specifically and our manifests will only be in YAML. When using the non-quoted interpolation, the type of the resulting value may be ambiguous without making ParameterType mandatory. If the template processor is implemented using simple text substitution, it may not be a problem to just output the value as a string without quotes and let the Kubernetes API figure it out, but in my case I'm actually walking the YAML document and transforming string values that include the interpolation syntax into their final type. Since the decision of whether or value will have quotes around it or not comes from the data type, there are cases (e.g. true vs. "true") where I'd need to know during substitution what the final type should be.
  • The comment above the ParameterType field says "one of string, int, bool, or base64" but that list doesn't quite make sense. Neither YAML nor JSON have a native base64 type (it'd just be a string) and a simple "int" does not cover how numbers are represented in either YAML or JSON. In YAML, there are many more number types, and it's unclear if or why a non-integer number should be rejected. In JSON, all numbers are the same type, so specifying "int" either seems like a mistake or implies some additional validation beyond what the the serialization format can represent.
  • The meaning of the Required field for a parameter is not well explained. It seems like it could be inferred from whether or not there is a default Value specified in the manifest. Or is Required supposed to indicate whether or not a literal null is acceptable?

@bparees
Copy link
Contributor Author

bparees commented Feb 25, 2016

thanks for the feedback, i've pushed fixes to the syntactical issues here:
#21981

Specifically, there are no examples of the two types of interpolation combined in one value

I believe there is:
// if both types of substitution exist, quoting is performed:

somefield: "prefix_$((FOO))_$(FOO)_suffix"  ->  somefield: "prefix_BAR_BAR_suffix"

and there aren't examples of each of the types of interpolation surrounded by literal text.

there can't be such an example because the format precludes using literal text (by which I assume you mean unquoted text). That's the purpose of the $(()) syntax, to strip the quotes where appropriate.

When using the non-quoted interpolation, the type of the resulting value may be ambiguous without making ParameterType mandatory. If the template processor is implemented using simple text substitution, it may not be a problem to just output the value as a string without quotes and let the Kubernetes API figure it out, but in my case I'm actually walking the YAML document and transforming string values that include the interpolation syntax into their final type.

There is an inherent assumption that the user who put the $() or $(()) reference into the field understood the type of the field and used the correct syntax/type. The purpose of the type field on the parameter definition is to allow clients to guide users with providing a value, not to aid in the actual substitution logic.

The comment above the ParameterType field says "one of string, int, bool, or base64" but that list doesn't quite make sense. Neither YAML nor JSON have a native base64 type (it'd just be a string) and a simple "int" does not cover how numbers are represented in either YAML or JSON.

"int" was probably overly shorthand(ie we should include other types), but the point of "base64" is again that this is intended to hint to the user of the template how the value is going to be used. Telling them the type is base64 tells them they need to provide a base64 encoded value for this parameter. The fact that ultimately that base64 value gets plugged in as a json string isn't relevant.

The meaning of the Required field for a parameter is not well explained. It seems like it could be inferred from whether or not there is a default Value specified in the manifest. Or is Required supposed to indicate whether or not a literal null is acceptable?

Required is intended to indicate that null/empty string is not an acceptable value for this parameter. The parameter must have a non-empty value either provided via the default, or provided by the user. I will update this in the above PR as well.

@jimmycuadra
Copy link
Contributor

Thank you, @bparees, that was very helpful! Your changes in that PR do well to clarify the things that tripped me up.

When I said "there are no examples of the two types of interpolation combined in one value and there aren't examples of each of the types of interpolation surrounded by literal text," I meant that the example template doesn't include those forms, so you don't see them in a realistic context and you can't use that example as a very complete test case. That's a minor issue, though, and I can adjust for my own needs.

Thanks again!

@jolestar
Copy link
Contributor

jolestar commented Sep 5, 2017

Not sure if this is the best place to ask that the proposal has been merged, but I can not find the progress of this proposal's implement anywhere. How about this proposal?

@bparees
Copy link
Contributor Author

bparees commented Sep 5, 2017

@jolestar it has not been implemented and at this point seems unlikely to be.

@bgrant0607
Copy link
Member

@jolestar A client-based implementation is mentioned above: https://github.com/InQuicker/ktmpl

Openshift also has an implementation.

I agree with @bparees that it is unlikely to be implemented at this point.

My current thinking on this and related topics is here:
https://goo.gl/T66ZcD

@jolestar
Copy link
Contributor

jolestar commented Sep 6, 2017

Thank you @bparees @bgrant0607 for reply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet