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

using trait type instead of hardcode word 'trait' to create trait CR #305

Merged
merged 3 commits into from
Nov 21, 2020
Merged

using trait type instead of hardcode word 'trait' to create trait CR #305

merged 3 commits into from
Nov 21, 2020

Conversation

zzxwill
Copy link
Member

@zzxwill zzxwill commented Nov 17, 2020

Fix #294

@zzxwill
Copy link
Member Author

zzxwill commented Nov 17, 2020 •
edited
Loading

I also use replace in KubeVela to manually test vela trait attach, the generated trait name s1-autoscaler-67cd45674 is to our expectation.

replace (
	github.com/crossplane/oam-kubernetes-runtime v0.3.3-0.20201112082656-22b7738dcdf3 => /Users/zhouzhengxi/Programming/golang/src/github.com/crossplane/oam-kubernetes-runtime
)

➜  /Users/zhouzhengxi/Programming/golang/src/github.com/crossplane/oam-kubernetes-runtime git:(issue-trait-cr) ✗ vela autoscale a1 --min 1 --max 5 --cpu-percent 5
Adding autoscale for app s1
⠏ Checking Status ...
✅ Application Deployed Successfully!
  - Name: s1
    Type: webservice
    HEALTHY Ready:1/1
    Traits:
      - ✅ autoscale: type: cpu     cpu-utilization(target/current): 5%/0%	replicas(min/max/current): 1/5/0
    Last Deployment:
      Created at: 2020-11-17 19:26:57 +0800 CST
      Updated at: 2020-11-17T19:40:57+08:00
➜  /Users/zhouzhengxi/Programming/golang/src/github.com/crossplane/oam-kubernetes-runtime git:(issue-trait-cr) ✗ k get autoscaler
NAME                      AGE
s1-autoscaler-67cd45674   8s

wonderflow
wonderflow suggested changes Nov 17, 2020
View reviewed changes
Copy link
Member

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure this is a compatible upgrade, which means, if user has already created a trait with the old naming rule and the spec didn't change , we should use the old name. Only the newly created trait will have the new name.

I didn't see any compatibility in this PR.

pkg/controller/v1alpha2/applicationconfiguration/render.go Outdated
@@ -654,7 +654,7 @@ func getTraitName(ac *v1alpha2.ApplicationConfiguration, componentName string,
}

if len(traitName) == 0 {
traitName = util.GenTraitName(componentName, ct.DeepCopy())
traitName = util.GenTraitName(componentName, ct.DeepCopy(), kind)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use kind? shouldn't we use traitdefinition name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In pure OAM Kubernetes Runtime, the traitdefinition name is normally very long, like autoscalers.standard.oam.dev, but for both runtime and KubeVela, the Kind is short and can stand for the traitdefinition name.

How about I cutting the first part split by . as the trait type in runtime? In Kubevela the name is short and clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cutting the first part to be the name is fine for me.
Please make sure the naming rule is clearly documented somewhere.

@zzxwill zzxwill force-pushed the issue-trait-cr branch 4 times, most recently from d8bfd25 to 92cb4a0 Compare November 18, 2020 08:31
zzxwill added a commit to zzxwill/kubevela that referenced this pull request Nov 18, 2020
@zzxwill
Componenttrait composing and trait CR naming design
0121222 
Add the design docs for OAM Kubernetes Runtime PR
crossplane/oam-kubernetes-runtime#305
@zzxwill zzxwill mentioned this pull request Nov 18, 2020
Merged
zzxwill added a commit to zzxwill/kubevela that referenced this pull request Nov 20, 2020
@zzxwill @wonderflow
Componenttrait composing and trait CR naming design
e79a940 
Add the design docs for OAM Kubernetes Runtime PR
crossplane/oam-kubernetes-runtime#305

Co-authored-by: Jianbo Sun <[email protected]>
@wonderflow wonderflow merged commit 2cf8f63 into crossplane:master Nov 21, 2020
@zzxwill zzxwill deleted the issue-trait-cr branch November 21, 2020 08:28
wonderflow added a commit to kubevela/kubevela that referenced this pull request Nov 23, 2020
@zzxwill @wonderflow
ComponentTrait composing and trait CR naming design (#628)
83ed11e 
* Componenttrait composing and trait CR naming design

Add the design docs for OAM Kubernetes Runtime PR
crossplane/oam-kubernetes-runtime#305

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

* Update design/vela-core/componenttrait-composing-and-trait-CR-naming.md

* Update design/vela-core/componenttrait-composing-and-trait-CR-naming.md

* Update design/vela-core/componenttrait-composing-and-trait-CR-naming.md

Co-authored-by: Jianbo Sun <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wonderflow wonderflow wonderflow approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] using trait type instead of hardcode word 'trait' to create trait CR
2 participants
@zzxwill @wonderflow