-
Notifications
You must be signed in to change notification settings - Fork 251
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
Resource Dependency in OAM #326
Conversation
LGTM. |
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.
really cool proposal !
### Case 1: MySQL and Web | ||
|
||
Two components, MySQL and Web exists in the same AppConfig and Web depends on MySQL that: | ||
|
||
1. MySQL needs to start first; | ||
2. MySQL connection info needs to be injected into Web's environment variables or files; | ||
|
||
### Case 2: NAS FileSystem and MountTarget | ||
|
||
Two components, NAS_FileSystem and NAS_MountTarget exists in the same AppConfig and NAS_MountTarget depends on NAS_FileSystem that: | ||
|
||
1. NAS_FileSystem needs to be created first; | ||
2. NAS_MountTarget needs filesystemID from NAS_FileSystem to fill in NAS_MountTarget's spec. | ||
|
||
## Proposal |
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.
Can you explain a bit on why these two cases are different?
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.
Let me explain more.
The first case represents different OAM implementation cooperate with each other. In other word, system of Web component don't know what will output from system of Mysql component.
The second case represents two components are in the same system, they can passthrough arguments with each other. One can output a key and finally fill in to the other one's spec.
|
||
Here a component can describe its dependencies, or dependent components. | ||
|
||
For Case 1, we also suggest to use ServiceBinding in [trait-injector](https://github.com/oam-dev/trait-injector) to inject secret. It is a trait that should be applied before component starts. We propose to add `ordering` to express trait's ordering w.r.t the component. |
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.
I wonder if the ServiceBinding trait should be part of OAM spec? If so, should it be a core or a standard trait?
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.
I think it should be a core trait. ServiceBinding will no doubt be one of the shining point of OAM.
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.
I think this document would be more informative to newbies like me if it either included or linked to some detail about trait-injector and the ServiceBinding
trait. I've looked at the README and the examples in the trait-injector repo, but I'm not quite sure how they work. It looks like the goal is mostly to inject a reference to a Secret
into a workload rendered by an ApplicationConfiguration
?
filePath: /input/mysql-conn | ||
``` | ||
|
||
### Parameter Passing |
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.
Is there any reason why we can't use serviceBinding trait here too?
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.
I have explained why these two cases are different. So parameter passing here is more likely to be used as internal arguments delivery, while service binding aims to be used in a more common way.
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.
More than that, we don't want to make service binding be coupled with parameters. Parameters are invented for application developers to tell what they need to application operators, while app operator can override the parameter. We could take parameter passing as an advanced override, it allows app operator override parameter from component output.
In this view, they have nothing to do with service binding, right?
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.
If I follow correctly, the problem we're solving here is that when workload A is created by appconfig X it may want to depend on some property of workload B, which is also created by appconfig X. It's possible that the property workload A depends on will be sensitive (e.g. a shared secret).
It seems like we're proposing two ways of addressing this:
- The
ServiceBinding
trait, which requests that the data from a named secret (which is presumably produced by worklad B) be mounted at a named path within workload A. - Parameter passing, which requests that a parameter used to create workload A be set to the value of a fieldpath within workload B.
It seems like parameter passing is simpler, but ServiceBinding
is more flexible. Would OAM be simpler overall if we supported only ServiceBinding
and not parameter passing?
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.
parameter passing
is also required as it gives us a way to pass through parameters directly between components. By using ServiceBinding
trait, we are able to pass through object
level information between components, while parameter passing
can support spec field
level information.
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.
Could you help me understand what the difference between object level and spec field level information?
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.
For example:
Assuming below is an existing CRD + Operator which can receive filesystemID as part of spec in CR.
apiVersion: ros.aliyun.com/v1alpha2
kind: NASMountTarget
spec:
filesystemID: "" # to be filled
If we give a secret object to it, it has no way to consume this secret.
While if the workload is K8s deployment, it has the way to consume secret object:
apiVersion: apps/v1
kind: Deployment
spec:
template:
spec:
containers:
envFrom:
- secretRef:
name: my-secret
|
||
Two components, MySQL and Web exists in the same AppConfig and Web depends on MySQL that: | ||
|
||
1. MySQL needs to start first; |
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.
Do we really need MySQL to start first, or could we rely on eventual consistency here? i.e. We start MySQL and the consumer of MySQL at the same time, and the consumer becomes healthy when MySQL becomes available. I imagine this would be less complex than attempting to model dependencies.
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.
It requires the consumer has the ability to recovery. But we do have cases that user's workload can't self-healing. So in the view of platform builder, we could add a little more into our application model.
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.
Hi @negz. We have the same conservation even back to the beginning: #23.
Initially, we want to rely on component itself to restart or hold back. But there are many existing scenarios that requires this dependency functionality for their apps to work properly. For example, we have OAM users whose apps can't start and trigger alerts. I think this feature is a major step to migrate as many existing apps as we could.
|
||
Here a component can describe its dependencies, or dependent components. | ||
|
||
For Case 1, we also suggest to use ServiceBinding in [trait-injector](https://github.com/oam-dev/trait-injector) to inject secret. It is a trait that should be applied before component starts. We propose to add `ordering` to express trait's ordering w.r.t the component. |
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.
I think this document would be more informative to newbies like me if it either included or linked to some detail about trait-injector and the ServiceBinding
trait. I've looked at the README and the examples in the trait-injector repo, but I'm not quite sure how they work. It looks like the goal is mostly to inject a reference to a Secret
into a workload rendered by an ApplicationConfiguration
?
spec: | ||
bindings: | ||
- from: | ||
secret: |
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.
Is this referring to a Kubernetes kind: Secret
, or the generic platform-agnostic concept of named secret data?
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.
I think it's generic platform-agnostic concept of named secret data for OAM spec. And in OAM K8s implementation it's kind:Secret
filePath: /input/mysql-conn | ||
``` | ||
|
||
### Parameter Passing |
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.
If I follow correctly, the problem we're solving here is that when workload A is created by appconfig X it may want to depend on some property of workload B, which is also created by appconfig X. It's possible that the property workload A depends on will be sensitive (e.g. a shared secret).
It seems like we're proposing two ways of addressing this:
- The
ServiceBinding
trait, which requests that the data from a named secret (which is presumably produced by worklad B) be mounted at a named path within workload A. - Parameter passing, which requests that a parameter used to create workload A be set to the value of a fieldpath within workload B.
It seems like parameter passing is simpler, but ServiceBinding
is more flexible. Would OAM be simpler overall if we supported only ServiceBinding
and not parameter passing?
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.
I have one comment only.
dependencies: | ||
- mysql | ||
traits: | ||
- ordering: preStart # applied before component starts |
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.
Having dependency information makes sense. On the other hand, specifying at which stage something should be applied is imperative and not declarative. IMO, OAM should keep a declarative config as much as possible.
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.
Could you provide a suggestion for declarative approach?
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.
I was thinking on a way to infer when applying the config that it needs to run in preStart phase. Is there a scenario binding will happen in another phase? If not, we can simply automatically apply bindings on preStart - always. Then, the ordering
attribute can be removed from the spec.
- mysql | ||
traits: | ||
- ordering: preStart # applied before component starts | ||
trait: |
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.
trait ordering defined in TraitDefinition?
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.
I like that idea but I wonder if there are any cases that a trait can be both pre and post component?
Summarize the discussion in today's meeting:
|
I believe it would also be useful for this design to consider and discuss how a runtime would actually enforce/enact trait ordering. I think this has overlap with the topics we've discussed in #323. What does a component "starting" mean? How do we know when the component is started? How do we prevent the component starting until the trait is applied? |
@hongchaodeng I wonder if this could be an alternative to the ---
# The Component that represents the server process.
apiVersion: core.oam.dev/v1alpha2
kind: Component
metadata:
name: coolcw
spec:
workload:
apiVersion: core.oam.dev/v1alpha2
kind: ContainerizedWorkload
spec:
containers:
- name: coolcontainer
image: cool/container:latest
# The app developer declares that they expect the connection details for
# MySQL to exist at filesystem path /input/mysql-conn. They do not need
config:
- path: /input/mysql-conn
# We allow the OAM spec to specify that config files (and env vars)
# can be populated from a 'Secret'. This does not mean OAM is aware of
# Kubernetes kind: Secret, but it does mean OAM runtimes must support
# some concept like it. We can omit fromSecret here because it will be
# set by a parameter.
#
# fromSecret: mysql-secret
parameters:
- name: instance-name
description: The name of the containerized workload.
required: true
fieldPaths:
- "metadata.name"
# The app developer allows the app operator to specify the name of the secret
# that will be mounted inside the container at /input/mysql-conn.
- name: mysql-secret
description: |
The secret from which the containerized workload should read its
connection details, such as username, password, and endpoint.
required: true
fieldPaths:
- "spec.containers[0].config[0].fromSecret"
---
# The Component that represents the MySQL database (i.e the default database of
# the requested MySQL instance).
apiVersion: core.oam.dev/v1alpha2
kind: Component
metadata:
name: coolsql
spec:
workload:
apiVersion: database.crossplane.io/v1alpha1
kind: MySQLInstance
spec:
engineVersion: 5.6
# Crossplane managed resources typically generate and write credentials,
# and other connection details (e.g. endpoints) rather than reading them.
# This configures which secret Crossplane should write the connection
# details for this MySQL instance to. We can omit it here because it will
# be set by a parameter.
#
# writeConnectionSecretToRef:
# name: mysql-secret
parameters:
- name: instance-name
description: The name of the MySQL instance.
required: true
fieldPaths:
- "metadata.name"
# The app developer allows the app operator to specify the name of the secret
# that will be written by Crossplane when the MySQLInstance becomes available.
- name: mysql-secret
description: |
The secret to which the MySQL instance should write its connection
details, such as username, password, and endpoint.
required: true
fieldPaths:
- "spec.writeConnectionSecretToRef.name"
---
apiVersion: core.oam.dev/v1alpha2
kind: ApplicationConfiguration
metadata:
name: coolapp
spec:
components:
- componentName: coolsql
parameterValues:
- name: instance-name
value: coolapp-sql
# The app operator specifies where to write the secret...
- name: mysql-secret
value: coolsecret
- componentName: coolcw
parameterValues:
- name: instance-name
value: coolapp-server
# ...and where to read the secret.
- name: mysql-secret
value: coolsecret This seems like a less complicated way to support passing sensitive connection details between infrastructure and applications than the For Kubernetes runtimes it should be quite easy to translate this to the containers array of a |
We have discussed about trait ordering within our platform teams and have agreement that trait ordering should be in TraitDefinition because:
|
@negz Actually we proposed something similar before and found there was one pitfall for us. First of all, let me diff on your idea (abbr. new idea) and the current proposal:
The impact of point 2 has impacted us in two ways:
But generally, I like the new idea and I think it is more natural to OAM and we could align on that in the core spec. The current ServiceBinding could be a standard Trait which doesn't conflict. |
@negz I wonder if this also replaced the parameter passing method? If not, can you explain the differences so that I can understand the nuances? Another question is, as I eluded to in the meeting, I wonder how can this apply to workloads that are maintained by a third party team that refused to change their schema to include such fields in their workload definition? |
One concern from @negz's proposal is the readability of the reference inversion where fromSecret (the consuming end) is set at the resource declaration instead (param). I honestly found it hard to navigate. Same theme I provided feedback on params for v1alpha2. Second, secret management seems to be a common use case that it deserves its own concept, not just another Trait defintion. Dependency management is a separate problem than secret management. Dependency management is only applicable in the second scenario since the creation of the second resource might not be possible without the first. It means that OAM implementations must implement a workflow engine. SQL and WebI would remove the explicit dependency and let the app itself handle dependency health. For secret, OAM spec would have explicit secret store attribute (which is a new concept called SecretStore) and keep the fromSecret for paramaterValues. The SecretStores would be provided by the runtime implementation (just like Traits).
If fieldPaths is a must (to keep consistency), then the secretStore would declare each secret to be consumed and where it should be populated. This would be the same as above but with fieldPaths:
|
@ryanzhang-oss In my proposal we presume workloads communicate the values they need to share via some kind of secret store in which a secret can be uniquely identified by name - possibly just a Kubernetes The parameter passing proposal allows workload B to depend on any API field of workload A. This is more flexible because it allows us to reference any field, even those that are not written to the secret store. However parameter passing cannot help with sensitive data, which we don't want to expose as a workload field (e.g. under All in all, I like the parameter passing API but am wary of the complexity it would bring to the AppConfig controller implementation. It seems like it would require either implicit or explicit dependency ordering (if workload B uses a field from workload A then workload B could not created until workload A was created). It also seems like it would expand the responsibility of the AppConfig controller to include reading back the workloads it creates in order to reference their fields. |
3a14982
to
f8041ca
Compare
Updated the doc.
I would argue that this is inaccurate. Let me explain more with a real world example in the following: Let's say I have a ML app that has three components:
This is true for most ML frameworks and they have strict ordering as follows:
Without the previous stage finished, the next could not start due to lack of data. |
I'm still not convinced that the OAM application model has to handle these concepts. It seems like it would be possible for the workloads involved to model this via eventual consistency. Are there existing Kubernetes CRs that model ML flows we could look to for guidance? |
For dependency, Not only the ML app, but also all legacy apps which won't have self healing need this dependency feature. We can't say OAM won't support these apps unless they have self healing feature. However, I think we could split this proposal into small pieces so we could discuss with more focus |
Referring a similar request in oam-dev mailing list: https://groups.google.com/forum/#!topic/oam-dev/X4npG1ay-sc |
I could agree with you from technical perspective. But many real world apps I have seen require such feature. Kubeflow is one example. I will attach more details into the proposal later. |
@wonderflow Could you clarify what you mean by "small pieces"? |
I mean should we split this proposal into three pieces?
|
I agree but isn't the entire v1alpha2 spec change done to allow existing applications to adopt OAM WITHOUT having to change? We have many internal applications that we want to bring onboard to the OAM model. It will be more difficult for us to convince them if more than minimal changes are required. |
Created an issue crossplane/oam-kubernetes-runtime#11. We will start with annotation-based implementation based on this design in crossplane. |
parameterValues: | ||
- name: filesystem-id | ||
from: | ||
compoent: nas-file-system |
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.
nit: component
|
||
In this design, we want to help applications reduce the burden of handling fault tolerance and dependency health. | ||
By putting this work in the OAM platform layer, OAM users can define explicit dependency and parameter passing in a standard way | ||
without carign about the implementation details. |
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.
typo carign > caring
It's a wonderful proposal.There is a scenario as below: a nsq cluster is composed with three components, that is nsqd nsqlookup and nsqadmin. That is, nsqlookup consumes the provide of nsqd component instance, nsqadmin consumes the provide of nsqlookup component instance. If we couldn't describe the dependencies order between the components, the application couldn't run properly. This is similarly to kubernetes/kubernetes#65502 , the issue describes dependencies between containers in the same Pod, we are discussing dependencies between components. |
I have updated the proposal with latest design. |
5fbcdcb
to
ee2ee8b
Compare
63454ce
to
c411efa
Compare
c411efa
to
86e3c69
Compare
Since it's no change on OAM spec and more implementation focused, we are moving the design into oam-kubernetes-runtime: crossplane/oam-kubernetes-runtime#24 |
86e3c69
to
ae1bf0c
Compare
We'd better keep the old proposal as a ref and make this new one implemented first. Then in And we can provide an upgrade path if one day we will make dependency into spec instead of trait. |
@hongchaodeng Should we close this PR? I went to review the PR you opened against oam-kubernetes-runtime, but accidentally started writing all my comments this one. |
@negz Yeah. Let's converge the work to crossplane/oam-kubernetes-runtime#24 |
moved to crossplane/oam-kubernetes-runtime#24
In #171, we have discussed our dependency stories and started to explore solutions.
Now that we have designed and implemented mechanism to satisfy dependency requirements, we are proposing to add them into OAM spec.
Fixes #23