oam-kubernetes-runtime icon indicating copy to clipboard operation
oam-kubernetes-runtime copied to clipboard

[BUG] OAM Runtime should implement the structured form of trait list

Open resouer opened this issue 5 years ago • 3 comments

See: https://github.com/oam-dev/spec/blob/master/7.application_configuration.md#traitdata.

For traitData section, we now only implemented the unstructured approach. I'd propose we also implement the structured approach like below:

apiVersion: core.oam.dev/v1alpha2
kind: ApplicationConfiguration
metadata:
  name: myapp
spec:
  - component: frontend
    traits:
      - name: autoscaler # use trait name instead of GVK
        spec:
          min: 10
          max: 100
          cpuUsage: 60
      - name: route
        spec:
          route: /index
      - name: expose
        spec:
          - port: 8001
            target: 80
            type: loadbalancer
          - port: 8081
            target: 8080
            type: nodeport
apiVersion: core.oam.dev/v1alpha2
kind: TraitDefinition
metadata:
  name: route # trait name
spec:
  appliesTo:
    - *.apps.k8s.io
  definitionRef: routes.networking.oam.dev # crd name

This could be handled with a non-break change: runtime checktraitData[i].trait field as a flag to support both forms.

This new form matches naturally to the original design that "traits are types not instances", makes TraitDefinition a MUST have, and also aligns better to the needs of OAM app engine. We could consider deprecating traitData[i].trait form in long term.

resouer avatar Aug 23 '20 05:08 resouer

What about:

apiVersion: core.oam.dev/v1alpha2
kind: ApplicationConfiguration
metadata:
  name: myapp
spec:
  - component: frontend
    traits:
      - trait:
           apiVersion: alibaba.com/v1
           kind: HPA
          spec:
            min: 10
            max: 100
            cpuUsage: 60
      - type: routes.api.alibaba.com
        properties:
           route: /index
      - type: expose
        properties:
          ports:
           - port: 8001
             target: 80
             type: loadbalancer
           - port: 8081
             target: 8080
             type: nodeport

wonderflow avatar Aug 25 '20 03:08 wonderflow

@wonderflow properties in most cases indicates key-value pairs. I would prefer spec instead.

Also, I prefer name than type because trait is already a type. That is to say, if we decide to remove GVK from workload, it will be type:

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

But it seems no need to touch workload for now.

resouer avatar Aug 25 '20 03:08 resouer

But it seems no need to touch workload for now.

Actually, I think there is real need to touch trait for now but we do need to touch workload. We need to have different workloadDefinitions pointing to the same workload.

ryanzhang-oss avatar Sep 03 '20 23:09 ryanzhang-oss