[functions] Allow functions to pass runtime specific options #5400

addisonj · 2019-10-16T22:07:58Z

Motivation

This commit adds a new argument for functions, customRuntimeOptions, which is passed to
funcions (as well as sources/sinks) that enables the ability to
customize the runtime.

This is added primarily to support the KubernetesManifestCustomizer
interface. This interface is intended, as the name indicates, allows for
customizing how the kubernetes manifests are generated before they
are sent off to the k8s cluster.

This interface has a default implementation, which allows for
changing the namespace, labels, nodeSelector labels, and toleratations per function.

This interface is also pluggable, allowing for more customized
implementations. For example, the functions for a given tenant could be
mapped to different pools of compute for isolation.

Modifications

For the CLI and protobufs, the modifications just involve plumbing through the new option, customRuntimeOptions through the relevant code.

For kubernetes runtime, the modifications are fairly straight forward, adding a new configuration option, kubernetesManifestCustomizerClassName and kubernetesManifestCustomizerConfig which are options under the kubernetes runtime.

Verifying this change

Make sure that the change passes the CI checks.

This commit adds some tests:

Adds tests for the kubernetes runtime with the default and custom implementation that validates the manifest files generated
passes existing unit tests

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): no
The public API: no
The schema: no
The default values of configurations: no
The wire protocol: yes, only of functions protobufs, adds a new field, customRuntimeOptions, which is a simple string
The rest endpoints: no
The admin cli options: yes, adds custom-runtime-options flags to the create/update commands for functions, sources, and sinks, which populates the above protobuf field
Anything that affects deployment: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? yes
If yes, how is the feature documented? yes, docs
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

addisonj · 2019-10-16T22:08:37Z

This conflicts with #5398, will need rebased if that is merged first

...ions/runtime/src/main/java/org/apache/pulsar/functions/runtime/KubernetesRuntimeFactory.java

addisonj · 2019-10-21T20:36:29Z

okay @jerrypeng I changed that, will see if I can get the tests happy, let me know if you have any other feedback!

sijie · 2019-10-24T08:32:53Z

@addisonj Can you rebase this pull request?

addisonj · 2019-10-26T22:03:49Z

I started in on re-working this... the changes introduced in #5404 broke one of the assumptions this made, which was that we create new instances of the functionAuthProvider, with a specific one for any overridden namespace. @jerrypeng do you see any problem with pushing the creation of the FunctionAuthProvider (via reflection) into the KubernetesRuntimeFactory? Or should we move the namespace used elsewhere? This provides a lot more flexibility to end users as being able to isolate various tenants to their own k8s namespace for functions can be use for finer grain isolation of underlying resources.

Would love to get this one through this week, but just want to make sure I am aligned in how to go about this

addisonj · 2019-11-01T04:40:53Z

rerun java8 tests

addisonj · 2019-11-01T06:53:57Z

rerun integration tests

addisonj · 2019-11-01T15:15:04Z

@jerrypeng @sijie I reworked this to account for the changes from #5404, I think it seems pretty reasonable.

If you could give another review and let me know if there is anything else you want to see here.

sijie · 2019-11-05T03:31:23Z

@addisonj LGTM. Great change!

btw can you rebase this pull request?

addisonj · 2019-11-08T20:04:50Z

rerun java8 tests

addisonj · 2019-11-08T23:17:53Z

@sijie @jerrypeng this is ready for another look, let me know if you have any questions!

...unctions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java

...e/src/main/java/org/apache/pulsar/functions/runtime/kubernetes/KubernetesRuntimeFactory.java

...ime/src/main/java/org/apache/pulsar/functions/runtime/BasicKubernetesManifestCustomizer.java

jerrypeng

@addisonj thanks for working on this! We need something like this. I left a couple of comments on changes I don't think is relevant to this PR.

I would recommend we have a generic interface for customizing runtime configurations. The current implementation really deals with kubernetes but I see this feature could be useful for other runtimes.

I would suggest we have a base interface something like below:

interface RuntimeCustomizer {
public void init(Function.FunctionDetails funcDetails)

}

and all customizers would be based on this. The customizer would be created and initialized in FunctionRuntimeManager and the customizer would be passed into the configured runtime regardless of what runtime it is. In the runtime e.g. kubernetes we can check if this object implements a specific customizer for the runtime e.g. KubernetesRuntimeCustomizer. This will allow the customizer to be more generic. For thread and process runtime factory, for now we can pass in the customizer but just ignore it, maybe later we can add support for it.

Another thing is do we need this new customizer interface? or can we reuse the "KubernetesFunctionAuthProvider/FunctionAuthProvider" interface? All we really need is pass in custom options to the init method. This interface allows rewriting the whole statefulset.

addisonj · 2019-11-09T00:52:36Z

I considered such an approach, but wasn't sure if that was something that made sense for other providers. Will take a look at refactoring this to support that. However, I do believe the changes (or som other changes) to KubernetesFunctionAuthProvider are necessary such that the namespace in which the secret is created can be overridden by the customizer.

jerrypeng · 2019-11-09T00:57:14Z

@addisonj also see if modiying "KubernetesFunctionAuthProvider/FunctionAuthProvider" interface to take in custom options can satisfy your use case.

addisonj · 2019-11-09T01:04:10Z

Perhaps I am missing something, but I think there is a mismatch here. The FunctionRuntimeManager can manage the creation of the RuntimeCustomizer, but it couldn't be eagerly in it's constructor or initialization methods. It would need to be during the addAssignment and updateAssignment methods as that is the first place where we get the function details.

In other words, the FunctionRuntimeManager's lifecycle is such that it managed multiple functions and would need to have a factory method to create a RuntimeCustomizer for each function. This is essentially the problem with FunctionAuthProvider as it currently is created once for the whole runtime and instead needs to be per function.

Perhaps the RuntimeCustomizer could be responsible for the lifecycle of the FunctionAuthProvider as well, but their lifecycles would seem intertwined.

Let me know if I am missing something.

...s/runtime/src/main/java/org/apache/pulsar/functions/auth/KubernetesFunctionAuthProvider.java

addisonj · 2019-11-11T21:27:02Z

...r-functions/runtime/src/main/java/org/apache/pulsar/functions/auth/FunctionAuthProvider.java

The changes to these methods allow us the flexibility we need to have implementations of this interface, primarily the KubernetesSecretTokenAuthProvider, have all context it needs to make sure that secrets it generates match the kubenetes namespace the function actually gets deployed too.

This also seems like a reasonable thing to do so that implementations of this interface can have more context about the function for custom implementations

addisonj · 2019-11-11T21:27:52Z

pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeFactory.java

This change allows implementations of this interface to provide a more specific FunctionAuthProvider, which reduces the need for casting

addisonj · 2019-11-11T21:30:37Z

@jerrypeng I ended up re-working this a fair amount to be more in-line with some of your ideas. Primarily:

created the RuntimeCustomizer class and had the KubernetesManifestCustomizer extend that class
the RuntimeCustomizer now gets created in the FunctionRuntimeManager and is passed to all RuntimeFactory classes
reverted the changes to how the FunctionAuthProvider gets created to also be back in FunctionRuntimeManager

To make this work, it required a few changes to some of the interfaces, primarily:

The FunctionAuthProvider now takes the FunctionDetails, this gives us all the data we need to effectively have the FunctionAuthProvider call the KubernetesManifestCustomizer so that we could get at the customRuntimeOptions which may change the namespace that secrets are created under. Additionally, this seems like a sane thing to do as now implementors of that interface get more data, which could be useful for different types of implementations
Made some small tweaks to the RuntimeFactory such that the returned FunctionAuthProvider and RuntimeCustomzer can be sub-classes of those classes that are specialized by implementing interfaces
Changed the KubernetesManifestCustomizer such that it only needs one instance of it as all the data needed to customize is passed via the methods (basically added FunctionDetails being passed to all calls)

...r-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeCustomizer.java

jerrypeng · 2019-11-12T07:31:14Z

@addisonj thanks for taking the time to rework some of this PR! It looks really good. I left some comments.

This commit adds a new argument for functions, customRuntimeOptions, which is passed to funcions (as well as sources/sinks) that enables the ability to customize the runtime via the `RuntimeCustomizer` class. This is added primarily to support the `KubernetesManifestCustomizer` interface. This interface has a default implementation, which allows for changing the namespace, labels, and annotations per function. For the other runtimes (Process and Thread runtimes) the `RuntimeCustomizer` is basically a no-op but is plumbed through for future customizations. This interface is also pluggable, allowing for more customizable implementations. For example, the functions for a given tenant could be mapped to different pools of compute for isolation.

addisonj · 2019-11-12T23:27:07Z

@jerrypeng changes made, let me know if you have anything else, glad it shaped up well :)

addisonj · 2019-11-13T16:50:06Z

rerun integration tests

addisonj · 2019-11-13T20:24:56Z

rerun integration tests

jerrypeng

@addisonj LGTM thanks again for working on this!

addisonj · 2019-11-14T07:17:08Z

@sijie if you have a second to merge this, that would be awesome :)

sijie · 2019-11-14T14:13:38Z

@addisonj great contribution!

…5400) This commit adds a new argument for functions, customRuntimeOptions, which is passed to funcions (as well as sources/sinks) that enables the ability to customize the runtime. This is added primarily to support the `KubernetesManifestCustomizer` interface. This interface is intended, as the name indicates, allows for customizing how the kubernetes manifests are generated before they are sent off to the k8s cluster. This interface has a default implementation, which allows for changing the namespace, labels, nodeSelector labels, and toleratations per function. This interface is also pluggable, allowing for more customized implementations. For example, the functions for a given tenant could be mapped to different pools of compute for isolation. For the CLI and protobufs, the modifications just involve plumbing through the new option, `customRuntimeOptions` through the relevant code. For kubernetes runtime, the modifications are fairly straight forward, adding a new configuration option, `kubernetesManifestCustomizerClassName` and `kubernetesManifestCustomizerConfig` which are options under the kubernetes runtime.

*Motivation* apache#5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`. *Tests* Add a unit test to ensure the parameters are set correctly.

…ng (#6101) *Motivation* Related to #6084 #5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`.

…ng (apache#6101) *Motivation* Related to apache#6084 apache#5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`.

…ng (apache#6101) *Motivation* Related to apache#6084 apache#5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`. (cherry picked from commit c6e258d)

…ng (#6101) *Motivation* Related to #6084 #5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`. (cherry picked from commit c6e258d)

…ng (apache#6101) *Motivation* Related to apache#6084 apache#5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`. (cherry picked from commit c6e258d)

…ng (apache#6101) *Motivation* Related to apache#6084 apache#5400 introduces `customRuntimeOptions` in function details. But the description was wrong. The mistake was probably introduced by bad merges. *Modification* Fix the argument and description for `deadletterTopic` and `customRuntimeOptions`.

This was referenced Oct 16, 2019

[functions] Distribute the CA for KubernetesSecretsTokenAuthProvider #5398

Merged

Multiple patches for fixes to pulsar instructure/pulsar#2

Merged

jerrypeng reviewed Oct 16, 2019

View reviewed changes

...ions/runtime/src/main/java/org/apache/pulsar/functions/runtime/KubernetesRuntimeFactory.java Outdated Show resolved Hide resolved

addisonj force-pushed the k8s_plugin branch from bc2e8d4 to e0ed679 Compare October 21, 2019 20:35

sijie assigned addisonj Oct 24, 2019

sijie added the area/function label Oct 24, 2019

sijie added this to the 2.5.0 milestone Oct 24, 2019

addisonj force-pushed the k8s_plugin branch 2 times, most recently from e106734 to 74e73cc Compare November 1, 2019 03:41

sijie approved these changes Nov 5, 2019

View reviewed changes

addisonj force-pushed the k8s_plugin branch from 74e73cc to 13110d1 Compare November 8, 2019 19:49