oam-dev / spec

Open Application Model (OAM).
https://oam.dev
Other
3.04k stars 246 forks source link

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

Open resouer opened 4 years ago

resouer commented 4 years ago

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 commented 4 years ago

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

resouer commented 4 years ago

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

hongchaodeng commented 4 years ago

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
ryanzhang-oss commented 4 years ago

LGTM on a high level, here are some minor details

resouer commented 4 years ago

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

wonderflow commented 4 years ago

LGTM with using label

kminder commented 3 years ago

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.

resouer commented 3 years ago

@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?