This repository has been archived by the owner on Jun 8, 2022. It is now read-only.
Implement traits ConflictsWith #309
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zzxwill
force-pushed
the
feature-trait-conflicts
branch
2 times, most recently
from
0af56ec
to
00b3cfc
Compare
When one trait conflicts with others, it should block the process of traits rendering no matter they are trying to apply all in one or one by one Signed-off-by: zzxwill <[email protected]>
Signed-off-by: zzxwill <[email protected]>
zzxwill
force-pushed
the
feature-trait-conflicts
branch
from
00b3cfc
to
74d3fef
Compare
zzxwill
changed the title
|
Currently API group or labelSelector are not yet supported. |
Signed-off-by: zzxwill <[email protected]>
zzxwill
force-pushed
the
feature-trait-conflicts
branch
from
74d3fef
to
4862b07
Compare
wonderflow
reviewed
Nov 24, 2020
| @@ -114,6 +114,12 @@ type TraitDefinitionSpec struct { | |||
| // +optional | |||
| AppliesToWorkloads []string `json:"appliesToWorkloads,omitempty"` | |||
|
|
|||
| // ConflictsWith specifies the list of traits which could not apply to the | |||
There was a problem hiding this comment.
Suggested change
| // ConflictsWith specifies the list of traits which could not apply to the | |
| // ConflictsWith specifies the list of traits(CRD name, Definition name, CRD group) which could not apply to the |
wonderflow
reviewed
Nov 24, 2020
| @@ -325,7 +325,8 @@ func renderWorkload(data []byte, p ...Parameter) (*unstructured.Unstructured, er | |||
| return &unstructured.Unstructured{Object: w.UnstructuredContent()}, nil | |||
| } | |||
|
|
|||
| func renderTrait(data []byte, _ ...Parameter) (*unstructured.Unstructured, error) { | |||
| // RenderTrait renders Trait to *unstructured.Unstructured format | |||
| func RenderTrait(data []byte, _ ...Parameter) (*unstructured.Unstructured, error) { | |||
| // TODO(negz): Is there a better decoder to use here? | |||
There was a problem hiding this comment.
we should create a common function for it instead of changing it public, and this common function could be in helper lib or anywhere else instead of the main logic lib.
wonderflow
reviewed
Nov 24, 2020
| @@ -110,6 +126,15 @@ func ValidateTraitObject(obj *v1alpha2.ApplicationConfiguration) field.ErrorList | |||
| allErrs = append(allErrs, field.Invalid(fldPath, content, | |||
| fmt.Sprintf("the trait data missing GVK, api = %s, kind = %s,", trait.GetAPIVersion(), trait.GetKind()))) | |||
| } | |||
| compatibleTraits = append(compatibleTraits, preAppliedTraits...) | |||
There was a problem hiding this comment.
根本不需要那个 preAppliedTraits . 每次 compatibbleTraits append 一个当前检查通过的trait就行了。
wonderflow
reviewed
Nov 24, 2020
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if conflict == compatibleTraitDef.Name { |
There was a problem hiding this comment.
四个条件都要有，你这里只检查了第一条：
- CRD 名称定义冲突
- services.k8s.io # API resource/crd name
- api group 定义冲突，这个是CRD 名称的增强版，前面用
*.前缀
- *.networking.k8s.io # API group
- label 定义冲突，前面固定前缀
labelSelector， label检查的是trait definition的label
- labelSelector:foo=bar
- Definition 名称定义冲突，与第一条类似，只不过这里写的是definition的名字而不是CRD名字
wonderflow
reviewed
Nov 24, 2020
|
|
||
| for _, conflict := range traitDef.Spec.ConflictsWith { | ||
| for j := range compatibleTraits { | ||
| compatibleTraitDef, err := util.FetchTraitDefinition(ctx, client, dm, &compatibleTraits[j]) |
There was a problem hiding this comment.
这里每次重复去fetch调用RPC请求性能太差了，预先获取所有涉及到的trait definition
wonderflow
reviewed
Nov 24, 2020
| "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" | ||
| ) | ||
|
|
||
| var _ = Describe("Trait conflict", func() { |
There was a problem hiding this comment.
这个改成单测吧，e2e现在太重了，不好检查，这个webhook的逻辑也没那么复杂，单测足够了。 参考： #310
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When one trait conflicts with others, it should block the process
of traits rendering no matter they are trying to apply all in one
or one by one.
To fix #141