-
Notifications
You must be signed in to change notification settings - Fork 103
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 for adding toggle behaviour to KudoOperator #1794
Open
asekretenko
wants to merge
8
commits into
main
Choose a base branch
from
asekretenko/kep_toggle_behavior_in_kudo_operator_task
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
febd5d6
KEP for adding toggle behaviour to KudoOperator
asekretenko 04a1e44
Use a single line per paragraph
asekretenko 477c1c7
Reword Summary, Motivation, Goals and Alternatives
asekretenko 0979e92
Extend Non-Goals
asekretenko 9deb66d
Rename KEP file
asekretenko a9e1187
Update `last-updated`
asekretenko 73e82cd
Avoid using the word "dependency"; clarify non-goals and other points
asekretenko e0319bf
Remove the subsection on upgrade handling
asekretenko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
--- | ||
kep-number: 36 | ||
short-desc: Allowing existence of a child operator to be conditional on a parameter value | ||
title: Conditional Operator Dependencies | ||
authors: | ||
- "@asekretenko" | ||
owners: | ||
- "@asekretenko" | ||
editor: @asekretenko | ||
creation-date: 2021-04-30 | ||
last-updated: 2021-05-27 | ||
status: provisional | ||
see-also: | ||
- KEP-23 | ||
- KEP-29 | ||
--- | ||
|
||
# Conditional Operator Dependencies | ||
|
||
## Table of Contents | ||
* [Summary](#summary) | ||
* [Motivation](#motivation) | ||
* [Goals](#goals) | ||
* [Non-Goals](#non-goals) | ||
* [Proposal](#proposal) | ||
* [Interface](#interface) | ||
* [Validation](#validation) | ||
* [Changing enablingParameter on upgrade](#changing-enablingparameter-on-upgrade) | ||
* [Implementation Notes](#implementation-notes) | ||
* [Risks and Mitigations](#risks-and-mitigations) | ||
* [Drawbacks](#drawbacks) | ||
* [Alternatives](#alternatives) | ||
|
||
|
||
## Summary | ||
|
||
This KEP aims to make it possible to create operators with optionally existing child operator instances. Namely, as a result of implementing this KEP, it will be possible to create a task, execution of which, depending on a value of a specified parameter, will ensure either | ||
* that a child operator instance exists, with appropriate parameters and operator version | ||
* or that the child instance does not exist (by cleaning it up if it does) | ||
|
||
## Motivation | ||
|
||
`KudoOperator` task type greatly simplifies handling operator dependencies by introducing the notion of a child operator. However, `KudoOperator` cannot be used directly when the existence of a child operator would have to be optional. Good examples of such cases are running Spark with an optional History Server or Kafka with an optional Schema Registry. | ||
|
||
In both cases, it would be more convenient to control the optional features via separate child operators; however, as of today, there are only two ways to have such a component installed optionally: either to include templates as-is into the parent operator and to control the contents at the template level, or to implement a job (via a `Toggle` task) which deploys/removes the optional component based on a parameter value. Cascading updates/upgrades become complicated in these scenarios. (See [#1775](https://github.com/kudobuilder/kudo/issues/1775)). | ||
|
||
### Goals | ||
|
||
* Add an ability to toggle a `KudoOperator` task between ensuring existence (as it does now) and ensuring **non-existence** of the child operator instance (and its children) via a parent operator parameter. Toggling in both directions will be possibe. | ||
|
||
### Non-Goals | ||
|
||
* Adding a way to turn a child instance into an independent one (i.e. an ability to "switch off" an existing dependency relationship between two instances). | ||
|
||
* Adding a way to turn an independent instance into a child instance. | ||
|
||
* Preventing users from modifying parent operator parameters that are only used to configure the child operator when the child instance does not exist. In the Spark History Server example, history server options will be configured via a parameter of a parent operator; switching off the history server operator will have no effect on the user's ability to change the value of the "history server options" parameter of the parent operator. | ||
|
||
* Displaying whether the child operator is enabled or disabled in an output of `kudo plan status`. As for now, the latter only descends to the status of individual plan steps of the parent instance and tells nothing about the child instances; this KEP does not aim to change this. | ||
|
||
* Chain validation of child instances. As for now, `KudoOperator` task executions that result in failed instance admission of a child do not fail parent instance admission, only plan execution. Functionality introduced by this proposal is also affected by this: for example, if a parent switches on a child that uses a non-existent parameter for toggling a grandchild, parent instance admission will succeed, only the corresponding parent plan will fail. | ||
|
||
* OperatorVersion validation. This proposal adds one more way to create an operator version that cannot be installed regardless of instance parameter values. | ||
|
||
* Introducing a task that unconditionally removes a child operator instance. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume a non-goal includes: Ability to take ownership of an existing operator as a child There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added to non-goals, thanks! |
||
|
||
* Conditional optionality. If, for example, an operator A has two optional children B and C, but C is not needed if B is not enabled, the user will need to disable C separately, which does not provide a good user experience. However, in this specific example operator developer could resolve the problem by turning C into a child of B or, if they have no control over the code of B, by adding a layer of indirection: a conditionally installed operator B\* that will unconditionally install B and conditionally install C. Conversely, if enabling B always requires C to be enabled, then B could be turned into a child of C. | ||
|
||
## Proposal | ||
|
||
### Interface | ||
An optional field `enablingParameter` will be added to the `KudoOperator` task: | ||
``` | ||
- name: child | ||
kind: KudoOperator | ||
spec: | ||
package: "./packages/child" | ||
parameterFile: child-params.yaml | ||
enablingParameter: "installChild" | ||
``` | ||
In this example, when the parameter `installChild` equals to `true`, the task will be handled as a normal, unconditional `KudoOperator`. When the parameter equals to `false`, execution of the task will ensure that the corresponding operator instance is not installed (uninstalling it if necessary). | ||
|
||
#### Validity criteria | ||
A `KudoOperator` task specifying a non-existing parent parameter via `enablingParameter` will be treated as invalid: operator package validation, instance admission and task execution will all fail in this case. | ||
|
||
Instance admission and task execution will require that the type of the specified parameter is either "boolean" or a string convertible to boolean according to rules used by Go's `strconv.ParseBool()`. | ||
|
||
### Implementation Notes | ||
|
||
Implementation is relatively straightforward, including the instance removal case: the child instance to be removed will be identified via the same mechanism as one used for identifying the instance to patch on upgrade/update. | ||
|
||
## Risks and Mitigations | ||
|
||
## Drawbacks | ||
|
||
This proposal makes the operator developer API even less robust with regards to developer errors than it is now (see [Non-Goals](#non-goals)). | ||
|
||
## Alternatives | ||
|
||
Currently existing alternatives: | ||
* Pulling the would-be contents of a child operator into the main operator as `Apply` tasks with heavily parametrized templates, or as `Toggle` tasks. This becomes complicated when the would-be child operator has to implement something more complex than a simple unconditional set of `Apply`/`Toggle` tasks. | ||
* Conditionally creating/deleting child operator instance by means of `Toggle` tasks. This is extremely cumbersome in the existing form, but might become much more convenient if we are to implement a stable API for managing operators via a single custom resource (aka CRD-based installation; KEP yet to be filed). However, one might argue that `KudoOperator` is still going to provide a much more convenient framework for cases when a parent operator has multiple children that are never used on their own. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this is still very confusing... when it says "ensuring existence", I think what you mean is that it takes ownership of the operator through provisioning, upgrading etc. It is very confusing to say that we want to ensure the non-existence of a child operator... It seems like we want the existence to be owned and managed external to kudo... right? or are you literally saying that this child operator should not exist at all?