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

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

Open
resouer opened this issue Aug 23, 2020 · 3 comments
Open

Comments

@resouer
Copy link
Contributor

resouer commented Aug 23, 2020

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.

@wonderflow
Copy link
Member

wonderflow commented Aug 25, 2020

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

@resouer
Copy link
Contributor Author

resouer commented Aug 25, 2020

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

@ryanzhang-oss
Copy link
Collaborator

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.

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