Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

Use label to replace workloadRef #4

Closed
resouer opened this issue Apr 22, 2020 · 9 comments
Closed

Use label to replace workloadRef #4

resouer opened this issue Apr 22, 2020 · 9 comments

Comments

@resouer
Copy link
Contributor

resouer commented Apr 22, 2020

Since a Trait CR 1:1 maps to Workload CR, maybe we can consider using label/annotation to replace trait.workloadRef?

e.g.

apiVersion: core.oam.dev/v1alpha2
kind: ManualScalerTrait
metadata:
  labels:
    core.oam.dev/workload: core.oam.dev/v1alpha2.ContainerizedWorkload
    core.oam.dev/workloadName: foo

The benefit is as Trait could also be brought by user, we don't want to require user modify CRD before using Crossplane.

(Optional) The link can also be Component if we agree on Component won't be shared in oam-dev/spec#350.

/cc @negz @ryanzhang-oss

@resouer
Copy link
Contributor Author

resouer commented Apr 22, 2020

(Optional) I am also thinking we can use labels to track the bi-directional links between dev facing objects (Component/Workload) with ops facing objects (Trait/Scope).

@negz
Copy link
Member

negz commented Apr 22, 2020

I think it's more common to use references to track one-to-one relationships than labels, which are typically used to track one-to-many relationships. It seems like the rationale here is more about not imposing requirements on the schema of a trait CR than because labels are inherently more appropriate for this case than references. Could you illustrate some cases in which we might want to use an existing, OAM-unaware Kubernetes CR as an OAM trait?

@resouer
Copy link
Contributor Author

resouer commented Apr 22, 2020

@negz Yes, happy to illustrate.

In the wild, many Operators are "Workload + Trait" style by natural. For instance, backup capability of etcd-operator:

Workload:

apiVersion: "etcd.database.coreos.com/v1beta2"
kind: "EtcdCluster"
metadata:
 name: "example-etcd-cluster"
 ## Adding this annotation make this cluster managed by clusterwide operators
 ## namespaced operators ignore it
 # annotations:
 #   etcd.database.coreos.com/scope: clusterwide
spec:
 size: 3
 version: "3.2.13"

Trait:

apiVersion: "etcd.database.coreos.com/v1beta2"
kind: "EtcdBackup"
metadata:
  name: example-etcd-cluster-backup
spec:
  etcdEndpoints: [<etcd-cluster-endpoints>]
  storageType: S3
  s3:
    # The format of "path" must be: "<s3-bucket-name>/<path-to-backup-file>"
    # e.g: "mybucket/etcd.backup"
    path: <full-s3-path>
    awsSecret: <aws-secret>

Similar patterns also exist in serverless solutions such as knative configuration + route.

Also, with the community begin to focus on application centric, many existing capabilities are re-designed as higher level abstractions (decoupled from underlying workload) so they can be used directly as OAM trait, for example Ingress v2 API.

These patterns may not be the majority for now but I think it's a trend that we can consider support at day 0 by loose the enforcement on CRD.

@resouer
Copy link
Contributor Author

resouer commented Apr 22, 2020

Btw, since we still need to add GVK + name in labels, it's seems no implication for "one-to-many" relationship.

@wonderflow
Copy link
Member

We can't enforce existing CRD/Operators to add workloadRef in their spec, however some of them could be very good OAM traits. Using label could add them into OAM system without any change.

What's more, I suggest to use some unique id to be label, for example: uid: 5da53b29-49ae-4466-83ea-b0c34405ee58. Then their match can be very easy and no need to use GVK+name.

@hasheddan
Copy link
Member

@resouer @wonderflow I like the idea of being able to bring arbitrary CRD types to the OAM model and allow them to serve as traits. Labels seem like a nice way to do this as they will be available to set on any type. However, I want to make sure we consider all alternatives here. The first that would come to mind would be implementing a separate type for binding traits to workloads (similar to trait-injector). Do you have thoughts on the pros and cons of these two models?

@hongchaodeng
Copy link
Member

Here is a list of options:

  1. WorkloadRef in Trait CR.
  2. Labels to ref workload CR in Trait CR.
  3. have a separate CR to correlate Trait CR and Workload CR.

If the only requirement is not to add any field into existing Trait CR, both 2 and 3 should work.

@ryanzhang-oss
Copy link
Collaborator

@ryanzhang-oss
Copy link
Collaborator

close this as we have decided to use field

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants