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

[Feature]OAM Runtime should allow different workloadDefinitions to point to the same workload Schema #195

Open
ryanzhang-oss opened this issue Sep 3, 2020 · 5 comments

Comments

@ryanzhang-oss
Copy link
Collaborator

ryanzhang-oss commented Sep 3, 2020

Is your feature request related to a problem? Please describe.
Currently, we only allow one workloadDefinition to point to the one workload Schema by tying its name to the schema name of the workload. This leads to the problem of not able to express different types of workloads with the same schema.

Describe the solution you'd like
We will decouple the name of the workloadDefintion from the name of workload schema. In order to achieve this, we will propose to add an optional "type" field in the Component to point to the name of the "workloadDefinition" it is referring to. This is fully backward compatible with the current schema. Here is an example.

apiVersion: core.oam.dev/v1alpha2
kind: Component
metadata:
  name: frontend
spec:
  type: Server
  workload:
    spec:
      image: nginx
      port: 80

Describe alternatives you've considered
Add a workloadDefinition controller that watches all the resources of type "CRD" and add an owner reference to itself when it detects a new resource of type "CRD" is created. This is a pretty heavy process and can lead to user confusion.

@resouer
Copy link
Contributor

resouer commented Sep 4, 2020

The workload hierarchy on top of spec seems useless. Can we avoid that?

For example:

apiVersion: core.oam.dev/v1alpha2
kind: Component
metadata:
  name: frontend
spec:
  workload:
    type: Server
    spec:
      image: nginx
      port: 80

For backward compatible implementation, the go types in my mind will be:

type ComponentSpec struct {
	Workload WorkloadTemplate
	Parameters []ComponentParameter
}
type WorkloadTemplate struct {
	Type string
	Spec runtime.RawExtension
        runtime.RawExtension // for GVK object, this field is exclusive to Type
}

@resouer
Copy link
Contributor

resouer commented Sep 4, 2020

This also makes it right that workload section is a template of workload instances.

@ryanzhang-oss
Copy link
Collaborator Author

ryanzhang-oss commented Sep 4, 2020

I am not sure if this is compatible with the current structure. as the workload itself is already declared as a RawExtention. It might be doable but pretty tricky and not sure what does that buy. The below structure is fully backward compatible and easy to implenent.

type ComponentSpec struct {
        Type             String
	Workload        runtime.RawExtension 
	Parameters     []ComponentParameter 
}

@ryanzhang-oss
Copy link
Collaborator Author

After further discussion, we have decided to take the format below but keep the API structure. So we will just parse the rawExtension as is and if there is a "Type" top key then we will use that to locate the workloadDefinition and add the GVK back to the CR. In this case, the CR should not have GVK and metadata.

apiVersion: core.oam.dev/v1alpha2
kind: Component
metadata:
  name: frontend
spec:
  workload:
    type: Server
    spec:
      image: nginx
      port: 80

@hongchaodeng
Copy link
Member

This leads to the problem of not able to express different types of workloads with the same schema.

@ryanzhang-oss Can you provide some example scenarios for this problem?

zzxwill added a commit to zzxwill/oam-kubernetes-runtime that referenced this issue Sep 16, 2020
When WorkloadDefinition doesn't follow the pattern
<puraral-kind>.<resource-group> or there are different
workloadDefinitions pointing to the same workload Schema,
it will hit the issue of could not finding workloaddefinition.
Fix issue crossplane#207, feature crossplane#195.

Signed-off-by: zzxwill <[email protected]>
zzxwill added a commit to zzxwill/oam-kubernetes-runtime that referenced this issue Sep 17, 2020
When there is spec.type in Workload, an annotation will set for
the workload which can help find the WorkloadDefinition.

Fix issue crossplane#207, related to feature crossplane#195.

Co-authored-by: Sun Jianbo <[email protected]>
Signed-off-by: zzxwill <[email protected]>
zzxwill added a commit to zzxwill/oam-kubernetes-runtime that referenced this issue Sep 18, 2020
When there is spec.type in Workload, an annotation will set for
the workload which can help find the WorkloadDefinition.

Fix issue crossplane#207, related to feature crossplane#195.

Co-authored-by: Sun Jianbo <[email protected]>
Signed-off-by: zzxwill <[email protected]>
zzxwill added a commit to zzxwill/oam-kubernetes-runtime that referenced this issue Sep 18, 2020
When there is spec.type in Workload, an annotation will set for
the workload which can help find the WorkloadDefinition.

Fix issue crossplane#207, related to feature crossplane#195.

Co-authored-by: Sun Jianbo <[email protected]>
Signed-off-by: zzxwill <[email protected]>
zzxwill added a commit to zzxwill/oam-kubernetes-runtime that referenced this issue Sep 21, 2020
When there is spec.type in Workload, an annotation will set for
the workload which can help find the WorkloadDefinition.

Fix issue crossplane#207, related to feature crossplane#195.

Co-authored-by: Sun Jianbo <[email protected]>
Signed-off-by: zzxwill <[email protected]>
wonderflow added a commit that referenced this issue Sep 21, 2020
* Fix `could not find WorkloadDefinition` issue

When there is spec.type in Workload, an annotation will set for
the workload which can help find the WorkloadDefinition.

Fix issue #207, related to feature #195.

Co-authored-by: Sun Jianbo <[email protected]>
Signed-off-by: zzxwill <[email protected]>

* Address comments and add unit-test

Signed-off-by: zzxwill <[email protected]>

* Fix goimports issue

Signed-off-by: zzxwill <[email protected]>

* Change function name and add unit-test

Signed-off-by: zzxwill <[email protected]>

* Removed maker annotation `definition.oam.dev/name`

Signed-off-by: zzxwill [email protected]
Signed-off-by: zzxwill <[email protected]>

Co-authored-by: Sun Jianbo <[email protected]>
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

3 participants