spec icon indicating copy to clipboard operation
spec copied to clipboard

Define workload types, replicable, and podspecable and make appliesTo work properly

Open resouer opened this issue 4 years ago • 8 comments

Consider adding operational hints (workload characteristic) on workload definition:

workload.oam.dev/exposed: true # exposes service endpoints
workload.oam.dev/replicable: true # can be scale
workload.oam.dev/daemonized: true # should be auto restart when stop
workload.oam.dev/podspecable: true # can be addressed by k8s podSpec

The appliesTo will work like this:

kind: WorkloadDefinition
metadata:
  labels:
    workload.oam.dev/replicable=true
kind: TraitDefinition
...
spec:
  appliestTo:
    - workload.oam.dev/replicable=true # operational hints label
    - workload.oam.dev/type=server # arbitrary workload type
    - foo=bar  # arbitrary label
    - *.apps.k8s.io # api group
    - cloneset.apps.alibaba-inc.com # arbitrary API resource

Also related: https://github.com/crossplane/oam-kubernetes-runtime/issues/174

resouer avatar Aug 19 '20 18:08 resouer

/cc @vturecek @hongchaodeng @ryanzhang-oss @artursouza @wonderflow for review.

resouer avatar Aug 19 '20 18:08 resouer

We should not assume Definition always has same name with defitionRef.

resouer avatar Aug 21 '20 00:08 resouer

We should probably make appliesTo more intelligent:

  appliestTo:
  - group: apps
    kinds: Deployment, StatefulSet
    labelSelector:
      workload.oam.dev/podSpecable=true
  - ...

Then in each WorkloadDefinition we could add the hints into their labels:

kind: WorkloadDefinition
metadata:
  labels:
    workload.oam.dev/replicable: true # can be scale
    workload.oam.dev/podspecable: true # can be addressed by k8s podSpec

hongchaodeng avatar Aug 21 '20 01:08 hongchaodeng

LGTM on a high level, here are some minor details

  • I would not call those "hint" as they are not optional or suggestive, they are more like "characteristics" or "attributes".
  • It would be good for the "appliesTo" field content to be more structured, something similar to the way @hongchaodeng described.
  • Just to clarify, this also allows multiple workloadDefinition refer to the same workload schema. We can have two workloads that behave differently while sharing the same schema. Thus, we need to decouple the name of the workloadDefinition from the name of the schema (CRD) it refers to.

ryanzhang-oss avatar Aug 21 '20 02:08 ryanzhang-oss

+1 to use label/selector instead of introducing new field.

resouer avatar Aug 21 '20 16:08 resouer

LGTM with using label

wonderflow avatar Aug 24 '20 04:08 wonderflow

It seems important to be able to filter on the workload's definitionRef (or something equivalent). This aligns with the initial comment from @resouer above. However the mixing of actual labels with API resources as suggested there seem problematic. On the other hand, being more explicit and using labelSelector seems to eliminate the possibility of limiting trait applicability by API resource. Unless perhaps there was some automatically applied workloadDefinition label (e.g. workload.oam.dev/resource=cloneset.apps.alibaba-inc.com). Another option would be for the TraitDefinition to have both a labelSelector and some for of resource selector or matchExpressions construct.

kminder avatar Feb 17 '21 19:02 kminder

@kminder Hi Kevin, just to confirm, you are suggesting using structured fields, or a specific object (e.g. WorkloadType) instead of labels to carry these characteristic (i.e. replicable) data, right? This was the original plan, but we didn't see much value except extra burden for one more object. Would you like to elaborate your insights a bit?

resouer avatar Feb 17 '21 23:02 resouer