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

KEP-24: Enhanced Operator Parameters #1356

Merged
merged 8 commits into from
Mar 2, 2020
Merged

Conversation

nfnt
Copy link
Member

@nfnt nfnt commented Feb 21, 2020

What this PR does / why we need it:
This KEP aims to improve the use-cases that can be covered by parameters. By supporting additional types, templates can access lists and dictionaries. Furthermore, validation can be implemented for known types.

As needed by #1349, #988, #951
Related: #1221; this KEP would allow to iterate over the items of a dict to create YAML, but having a toYAML template function would still be beneficial

This KEP aims to improve the use-cases that can be covered by parameters. By supporting additional types, templates can access lists and dictionaries. Furthermore, validation can be implemented for known types.

Signed-off-by: Jan Schlicht <jan@d2iq.com>
@nfnt nfnt added kind/kep Kudo Enhancement kep/24 Map and array parameter types labels Feb 21, 2020
@nfnt nfnt self-assigned this Feb 21, 2020
Signed-off-by: Jan Schlicht <jan@d2iq.com>
keps/0024-parameter-enhancement.md Show resolved Hide resolved
keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
keps/0024-parameter-enhancement.md Show resolved Hide resolved

### Provide Parameter Validation

Adding types for lists and dictionaries provides access to the items of these structures in templates. Adding types like integers won't provide benefits for templating, but would allow for input value validation. KUDO could, e.g., validate that an integer isn't too large, that a list doesn't have too many entries, or that a string isn't too long. Adding parameter validation is independent from adding parameter types.
Copy link
Member

Choose a reason for hiding this comment

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

Another thing useful to a user would be to verify that more complex objects passed as parameters have correct structure, even before it's merged with the rest of the (potentially huge) template (at which point it might be a little overwhelming to figure out what an error message means).

keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
Signed-off-by: Jan Schlicht <jan@d2iq.com>
@@ -0,0 +1,146 @@
---
kep-number: 24
Copy link
Member

Choose a reason for hiding this comment

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

This should be draft-YYYYMMDD according to the KEP process doc. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, not necessarily 🤦‍♂️

Signed-off-by: Jan Schlicht <jan@d2iq.com>
Copy link
Member

@akirillov akirillov left a comment

Choose a reason for hiding this comment

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

Thanks, @nfnt. I have a generic question regarding the passing of enhanced parameters at the moment of installation of an operator. Looking at the examples in #1349 with complex NODE_TOPOLOGY parameter, or at nodeSelector/podAnnotations from #1221, how users are supposed to pass these parameters via -p flags of KUDO CLI?

Supporting some sort of value files with defaults override can be a possible solution. Is this KEP addressing complex values specification by users?

Signed-off-by: Jan Schlicht <jan@d2iq.com>
@nfnt
Copy link
Member Author

nfnt commented Feb 26, 2020

@akirillov That's a good question! @porridge asked that one as well. I added some notes that it's necessary to better support these parameter values. See also #1364 which is a KEP about adding support for loading parameter values from a file.

Signed-off-by: Jan Schlicht <jan@d2iq.com>
@porridge
Copy link
Member

Supporting some sort of value files with defaults override can be a possible solution. Is this KEP addressing complex values specification by users?

@akirillov as mentioned by @nfnt #1364 addresses passing complex values.
The other side of the coin - actually being able to conveniently use those complex values - is covered by the KEP in #1363 .


### Support Optional Parameter Types

By adding an optional `type` field to the list of parameters, parameter types can be specified. If this field isn't present, it defaults to the `string` type. Other types like `list`, `dict` or `integer` are possible. KUDO's renderer uses the type information to convert the string value to the respective type. For example, for the `dict` type, the input value would be YAML that is unmarshalled to a `map[string]interface{}`. Go's template engine allows to access and iterate list and dictionary items.
Copy link
Member

Choose a reason for hiding this comment

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

➕to defaults to string type for a strong retro-compatibility

What about the types of elements of list and dict? We will always use them as string? In that case, should we mention it here

Copy link
Member

Choose a reason for hiding this comment

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

I don't think types of elements of list and dict should be guaranteed to be strings.
Keeping them arbitrary is necessary to be able to support #1221 (and #951).

Copy link
Member

Choose a reason for hiding this comment

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

Then we should allow users to specify the structure of those elements to be a custom struct or Integer or String. Or at least specify how it would work.
cc: @nfnt

Copy link
Member Author

Choose a reason for hiding this comment

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

Elements of list and dicts can have other types than string. Because they will be provided as YAML and YAML supports multiple types.

Copy link
Member

@zmalik zmalik left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@porridge porridge left a comment

Choose a reason for hiding this comment

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

This is awesome, I added a few inline suggestions.

keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
keps/0024-parameter-enhancement.md Outdated Show resolved Hide resolved
Jan Schlicht and others added 2 commits February 28, 2020 11:38
Co-Authored-By: Marcin Owsiany <mowsiany@D2iQ.com>
Signed-off-by: Jan Schlicht <jan@d2iq.com>
@nfnt nfnt merged commit 3b159d5 into master Mar 2, 2020
@nfnt nfnt deleted the nfnt/kep-24-parameter-enhancements branch March 2, 2020 09:08
runyontr pushed a commit that referenced this pull request Mar 11, 2020
This KEP aims to improve the use-cases that can be covered by parameters. By supporting additional types, templates can access lists and dictionaries. Furthermore, validation can be implemented for known types.

Co-Authored-By: Marcin Owsiany <mowsiany@D2iQ.com>
Signed-off-by: Jan Schlicht <jan@d2iq.com>
Signed-off-by: Thomas Runyon <runyontr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kep/24 Map and array parameter types kind/kep Kudo Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants