From ccfde716508adadc42fa4c36f29f96725e8a8563 Mon Sep 17 00:00:00 2001 From: zhuhuijun Date: Tue, 7 Jul 2020 15:07:30 +0800 Subject: [PATCH] fix(trait):define the default trait name Make the default trait name not the same as the component name Fixes #66 Signed-off-by: zhuhuijun --- go.mod | 2 + go.sum | 2 + .../applicationconfiguration/render.go | 2 +- .../applicationconfiguration/render_test.go | 4 +- pkg/oam/util/helper.go | 41 ++++++++++++- pkg/oam/util/helper_test.go | 59 +++++++++++++++++++ 6 files changed, 105 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 51ba4935..c3825ed0 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( github.com/crossplane/crossplane-runtime v0.8.0 + github.com/davecgh/go-spew v1.1.1 github.com/gertd/go-pluralize v0.1.7 github.com/go-logr/logr v0.1.0 github.com/google/go-cmp v0.4.0 @@ -13,6 +14,7 @@ require ( github.com/rs/xid v1.2.1 github.com/stretchr/testify v1.4.0 golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect + gopkg.in/go-playground/assert.v1 v1.2.1 // indirect k8s.io/api v0.18.5 k8s.io/apiextensions-apiserver v0.18.2 k8s.io/apimachinery v0.18.5 diff --git a/go.sum b/go.sum index e3017581..9cf84d02 100644 --- a/go.sum +++ b/go.sum @@ -570,6 +570,8 @@ gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qS gopkg.in/cheggaaa/pb.v1 v1.0.27/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw= gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= +gopkg.in/go-playground/assert.v1 v1.2.1 h1:xoYuJVE7KT85PYWrN730RguIQO0ePzVRfFMXadIrXTM= +gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE= gopkg.in/inf.v0 v0.9.0/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 743eaa6f..ee400fef 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -165,7 +165,7 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati } func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait, namespace, componentName string, ref *metav1.OwnerReference, dag *dependency.DAG) (*unstructured.Unstructured, *v1alpha2.TraitDefinition, error) { - traitName := util.GenTraitName(componentName) + traitName := util.GenTraitName(componentName, ct.DeepCopy(), nil) t, err := r.trait.Render(ct.Trait.Raw) if err != nil { return nil, nil, errors.Wrapf(err, errFmtRenderTrait, traitName) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 06f1329e..546f8650 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -173,7 +173,7 @@ func TestRenderComponents(t *testing.T) { }, args: args{ac: ac}, want: want{ - err: errors.Wrapf(errBoom, errFmtRenderTrait, componentName), + err: errors.Wrapf(errBoom, errFmtRenderTrait, util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy(), nil)), }, }, "Success": { @@ -670,7 +670,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) { t.Run(name, func(t *testing.T) { r := &components{tc.fields.client, nil, tc.fields.params, tc.fields.workload, tc.fields.trait} got, _ := r.Render(tc.args.ctx, tc.args.ac) - if len(got) == 0 || len(got[0].Traits) == 0 || got[0].Traits[0].GetName() != componentName { + if len(got) == 0 || len(got[0].Traits) == 0 || got[0].Traits[0].GetName() != util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy(), nil) { t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, "Trait name is NOT"+ "automatically set.") } diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 83ab015e..3e7827fb 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -2,12 +2,17 @@ package util import ( "context" + "encoding/binary" "encoding/json" "fmt" + "hash" + "hash/fnv" "reflect" "strings" "time" + "github.com/davecgh/go-spew/spew" + cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" plur "github.com/gertd/go-pluralize" "github.com/go-logr/logr" @@ -16,6 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -204,6 +210,37 @@ func Object2Map(obj interface{}) (map[string]interface{}, error) { } // GenTraitName generate trait name -func GenTraitName(componentName string) string { - return fmt.Sprintf("%s-%s", TraitPrefixKey, componentName) +func GenTraitName(componentName string, ct *v1alpha2.ComponentTrait, collisionCount *int32) string { + return fmt.Sprintf("%s-%s-%s", componentName, TraitPrefixKey, ComputeHash(ct, collisionCount)) +} + +// ComputeHash returns a hash value calculated from pod template and +// a collisionCount to avoid hash collision. The hash will be safe encoded to +// avoid bad words. +func ComputeHash(trait *v1alpha2.ComponentTrait, collisionCount *int32) string { + componentTraitHasher := fnv.New32a() + DeepHashObject(componentTraitHasher, *trait) + + // Add collisionCount in the hash if it exists. + if collisionCount != nil { + collisionCountBytes := make([]byte, 8) + binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) + _, _ = componentTraitHasher.Write(collisionCountBytes) + } + + return rand.SafeEncodeString(fmt.Sprint(componentTraitHasher.Sum32())) +} + +// DeepHashObject writes specified object to hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { + hasher.Reset() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + _, _ = printer.Fprintf(hasher, "%#v", objectToWrite) } diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index 01647084..03199ee4 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -3,6 +3,7 @@ package util_test import ( "context" "fmt" + "math" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -292,3 +293,61 @@ var _ = Describe("Test unstructured related helper utils", func() { } }) }) + +var _ = Describe("Test GenTraitName helper utils", func() { + It("Test generate trait name", func() { + + collisionCount := int32(1) + otherCollisionCount := int32(2) + maxCollisionCount := int32(math.MaxInt32) + + tests := []struct { + name string + template *v1alpha2.ComponentTrait + collisionCount *int32 + otherCollisionCount *int32 + }{ + { + name: "simple", + template: &v1alpha2.ComponentTrait{}, + collisionCount: &collisionCount, + otherCollisionCount: &otherCollisionCount, + }, + { + name: "using math.MaxInt64", + template: &v1alpha2.ComponentTrait{}, + collisionCount: nil, + otherCollisionCount: &maxCollisionCount, + }, + } + + for _, test := range tests { + hash := util.ComputeHash(test.template, test.collisionCount) + otherHash := util.ComputeHash(test.template, test.otherCollisionCount) + Expect(hash).ShouldNot(Equal(otherHash)) + } + + test2s := []struct { + name string + template *v1alpha2.ComponentTrait + collisionCount *int32 + otherCollisionCount *int32 + }{ + { + name: "simple", + template: &v1alpha2.ComponentTrait{}, + collisionCount: &collisionCount, + otherCollisionCount: &collisionCount, + }, + } + for _, test := range test2s { + hash := util.ComputeHash(test.template, test.collisionCount) + name := fmt.Sprintf("%s-%s-%s", test.name, util.TraitPrefixKey, util.ComputeHash(test.template, &collisionCount)) + otherHash := util.ComputeHash(test.template, test.otherCollisionCount) + Expect(hash).Should(Equal(otherHash)) + traitName := util.GenTraitName(test.name, test.template, test.collisionCount) + Expect(traitName).Should(Equal(name)) + } + + }) +})