How to define two traits are conflict?

[wonderflow]

Our Traits are loosely coupled which means a user or platform builder can easily add a trait for some kinds of workload.

But we can't recognize whether a new trait will conflict with an existing trait.

For possible way to solve this is to add a label or something to mark, and traits with same marks will be recognize as conflict. For example:

apiVersion: core.oam.dev/v1alpha2
kind: TraitDefinition
metadata:
  name: manualscalertrait.core.oam.dev
spec:
  appliesToWorkloads:
    - core.oam.dev/v1alpha2.ContainerizedWorkload
  definitionRef:
    name: manualscalertrait.core.oam.dev
  conflictLabel:
    scaler: true

apiVersion: core.oam.dev/v1alpha2
kind: TraitDefinition
metadata:
  name: autoscalertrait.core.oam.dev
spec:
  appliesToWorkloads:
    - core.oam.dev/v1alpha2.ContainerizedWorkload
  definitionRef:
    name: autoscalertrait.core.oam.dev
  conflictLabel:
    scaler: true

When we find two traits which has same conflictLabel, we could say they are conflict and deny it from admission webhook.

[hongchaodeng]

I would recommend a design similar to inter-pod anti-affinity:

traitRelation:
  conflict:
  - labelSelector:
      scaler: true

[resouer]

We also need to define category of traits as we already did internal Alibaba.

A labelSelector style solution seems good. I will still suggest consider OPA as the policy engine though.

[zhangxiaoyu-zidif]

Could we use some banlist and allowlist to define the conflict traits slice？

[zzxwill]

@zhangxiaoyu-zidif As OAM supports extended traits contributed by other developers, the whole trait list is hard to be complete, nor will it easy to fill items in the whitelist and blacklist, I think.

[zhangxiaoyu-zidif]

@zzxwill Correct, so I fancy is that possible to create some custom traits(as custom resource does), and make some patterns to rule them. I can not picture how to work currently, but I will trace this issue and join related discussions later. :)

[zzxwill]

@zhangxiaoyu-zidif Maybe the Trait catalog can help:)
https://github.com/oam-dev/catalog/tree/master/traits

[linjiemiao]

Can you see review my specific implementation plan？ @wonderflow

1. modify core.oam.dev_traitdefinitions.yaml, we add the following definitions:

+              traitRelation:
+                description: traitRelation
+                properties:
+                  conflict:
+                    description: conflict
+                    properties:
+                      labelSelector:
+                        description: labelSelector description
+                        items:
+                          type: string
+                        type: array
+                    type: object
+                type: object

2. define the TraitDefinition of manualscalertraits as follows:

apiVersion: core.oam.dev/v1alpha2
kind: TraitDefinition
metadata:
  name: manualscalertraits.core.oam.dev
spec:
  workloadRefPath: spec.workloadRef
  definitionRef:
    name: manualscalertraits.core.oam.dev
  traitRelation:
    conflict:
      labelSelector:
      - hpascaler
      - cronscaler

3. Execute the following command

# kubectl get traitdefinition  manualscalertraits.core.oam.dev -oyaml|less
apiVersion: core.oam.dev/v1alpha2
kind: TraitDefinition
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"core.oam.dev/v1alpha2","kind":"TraitDefinition","metadata":{"annotations":{},"name":"manualscalertraits.core.oam.dev"},"spec":{"definitionRef":{"name":"manualscalertraits.core.oam.dev"},"traitRelation":{"conflict":{"labelSelector":["scaler","trait1"]}},"workloadRefPath":"spec.workloadRef"}}
    meta.helm.sh/release-name: core-runtime
    meta.helm.sh/release-namespace: oam-system
  creationTimestamp: "2020-10-21T03:09:12Z"
  generation: 5
  labels:
    app.kubernetes.io/managed-by: Helm
  name: manualscalertraits.core.oam.dev
  resourceVersion: "149129584"
  selfLink: /apis/core.oam.dev/v1alpha2/traitdefinitions/manualscalertraits.core.oam.dev
  uid: 87587e6e-033b-4162-89ed-2d294170ba2b
spec:
  definitionRef:
    name: manualscalertraits.core.oam.dev
  traitRelation:
    conflict:
      labelSelector:
      - hpatraits
      - cronhpatraits
  workloadRefPath: spec.workloadRef

You can see that the traitRelation field is filled。

4. We verify whether the component has trait conflicts in the webhook of ApplicationConfiguration.

1） We can find the Definition of the trait through the labelSelector field.
2） Then we can compare all related traits of a component with the traits of labelSelector, and deny if there is a conflict.
伪代码如下：

// ValidateTraitConflict validates the ApplicationConfiguration Traits Conflict on creation/update
func ValidateTraitConflict(ctx context.Context, r client.Reader, obj *v1alpha2.ApplicationConfiguration) field.ErrorList {
	klog.Info("validate applicationConfiguration trait conflict", "name", obj.Name)
	var allErrs field.ErrorList
	for cidx, comp := range obj.Spec.Components {
		traitDefNameList := make([]string, 1)
		traitConflictNameList := make([]string, 1)
		for idx, tr := range comp.Traits {
			fldPath := field.NewPath("spec").Child("components").Index(cidx).Child("traits").Index(idx).Child("trait")
			unstructuredTrait, err := util.Object2Unstructured(tr)
			if err != nil {
				klog.Error("ComponentTrait convert to Unstructed error", "error", err.Error())
				allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), err.Error()))
				return allErrs
			}
			traitDef, err := util.FetchTraitDefinition(ctx, r, unstructuredTrait)
			if err != nil {
				klog.Error("FetchTraitDefinition", "error", err.Error())
				allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), err.Error()))
				return allErrs
			}
			traitDefNameList = append(traitDefNameList, traitDef.Name)
			traitConflictNameList = append(traitConflictNameList, traitDef.Spec.TraitRelation.Conflict.LabelSelector...)
		}
		// TODO： 判断traitDefNameList 和 traitConflictNameList是否有交集, 如果发现有交集就Deny
		// 这里的traitConflict需要写全称，需要带上gvk信息，否则不好对比。
	}
	return allErrs
}

[wonderflow]

@linjiemiao Can you follow this design crossplane/oam-kubernetes-runtime#141 (comment)