From 499d2b588ddad9a8f1ca2fde6defe861c17a1d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Fri, 23 Oct 2020 10:44:17 +0800 Subject: [PATCH 1/4] refactor pkg/oam/util to not use ginkgo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 天元 --- pkg/oam/util/helper.go | 61 +- pkg/oam/util/helper_test.go | 1739 +++++++++++++++---------------- pkg/oam/util/test_utils_test.go | 381 ++++--- pkg/oam/util/util_suite_test.go | 22 - 4 files changed, 1081 insertions(+), 1122 deletions(-) delete mode 100644 pkg/oam/util/util_suite_test.go diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index bc07d6d8..5cbcfa8a 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -7,11 +7,13 @@ import ( "hash" "hash/fnv" "reflect" - "strings" "time" + "k8s.io/apimachinery/pkg/runtime/schema" + + "k8s.io/apimachinery/pkg/api/meta" + "github.com/davecgh/go-spew/spew" - plur "github.com/gertd/go-pluralize" "github.com/go-logr/logr" "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" @@ -113,11 +115,14 @@ func FetchWorkload(ctx context.Context, c client.Client, mLog logr.Logger, oamTr } // FetchScopeDefinition fetch corresponding scopeDefinition given a scope -func FetchScopeDefinition(ctx context.Context, r client.Reader, +func FetchScopeDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, scope *unstructured.Unstructured) (*v1alpha2.ScopeDefinition, error) { // The name of the scopeDefinition CR is the CRD name of the scope // TODO(wonderflow): we haven't support scope definition label type yet. - spName := GetDefinitionName(scope, "") + spName, err := GetDefinitionName(mapper, scope, "") + if err != nil { + return nil, err + } // the scopeDefinition crd is cluster scoped nn := types.NamespacedName{Name: spName} // Fetch the corresponding scopeDefinition CR @@ -129,10 +134,13 @@ func FetchScopeDefinition(ctx context.Context, r client.Reader, } // FetchTraitDefinition fetch corresponding traitDefinition given a trait -func FetchTraitDefinition(ctx context.Context, r client.Reader, +func FetchTraitDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, trait *unstructured.Unstructured) (*v1alpha2.TraitDefinition, error) { // The name of the traitDefinition CR is the CRD name of the trait - trName := GetDefinitionName(trait, oam.TraitTypeLabel) + trName, err := GetDefinitionName(mapper, trait, oam.TraitTypeLabel) + if err != nil { + return nil, err + } // the traitDefinition crd is cluster scoped nn := types.NamespacedName{Name: trName} // Fetch the corresponding traitDefinition CR @@ -144,10 +152,13 @@ func FetchTraitDefinition(ctx context.Context, r client.Reader, } // FetchWorkloadDefinition fetch corresponding workloadDefinition given a workload -func FetchWorkloadDefinition(ctx context.Context, r client.Reader, +func FetchWorkloadDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, workload *unstructured.Unstructured) (*v1alpha2.WorkloadDefinition, error) { // The name of the workloadDefinition CR is the CRD name of the component - wldName := GetDefinitionName(workload, oam.WorkloadTypeLabel) + wldName, err := GetDefinitionName(mapper, workload, oam.WorkloadTypeLabel) + if err != nil { + return nil, err + } // the workloadDefinition crd is cluster scoped nn := types.NamespacedName{Name: wldName} // Fetch the corresponding workloadDefinition CR @@ -160,9 +171,9 @@ func FetchWorkloadDefinition(ctx context.Context, r client.Reader, // FetchWorkloadChildResources fetch corresponding child resources given a workload func FetchWorkloadChildResources(ctx context.Context, mLog logr.Logger, r client.Reader, - workload *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { + mapper meta.RESTMapper, workload *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { // Fetch the corresponding workloadDefinition CR - workloadDefinition, err := FetchWorkloadDefinition(ctx, r, workload) + workloadDefinition, err := FetchWorkloadDefinition(ctx, r, mapper, workload) if err != nil { return nil, err } @@ -230,35 +241,23 @@ func PassLabelAndAnnotation(parentObj oam.Object, childObj labelAnnotationObject // the format of the definition of a resource is . // Now the definition name of a resource could also be defined as `definition.oam.dev/name` in `metadata.annotations` // typeLabel specified which Definition it is, if specified, will directly get definition from label. -func GetDefinitionName(u *unstructured.Unstructured, typeLabel string) string { +func GetDefinitionName(mapper meta.RESTMapper, u *unstructured.Unstructured, typeLabel string) (string, error) { if typeLabel != "" { if labels := u.GetLabels(); labels != nil { if definitionName, ok := labels[typeLabel]; ok { - return definitionName + return definitionName, nil } } } - group, _ := APIVersion2GroupVersion(u.GetAPIVersion()) - resources := []string{Kind2Resource(u.GetKind())} - if group != "" { - resources = append(resources, group) + groupVersion, err := schema.ParseGroupVersion(u.GetAPIVersion()) + if err != nil { + return "", err } - return strings.Join(resources, ".") -} - -// APIVersion2GroupVersion turn an apiVersion string into group and version -func APIVersion2GroupVersion(str string) (string, string) { - strs := strings.Split(str, "/") - if len(strs) == 2 { - return strs[0], strs[1] + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: groupVersion.Group, Kind: u.GetKind()}, groupVersion.Version) + if err != nil { + return "", err } - // core type - return "", strs[0] -} - -// Kind2Resource convert Kind to Resources -func Kind2Resource(str string) string { - return plur.NewClient().Plural(strings.ToLower(str)) + return mapping.Resource.Resource + "." + groupVersion.Group, nil } // Object2Unstructured convert an object to an unstructured struct diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index 85e48da6..2c93fdd1 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -6,14 +6,13 @@ import ( "fmt" "hash/adler32" "reflect" + "testing" "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/resource/fake" "github.com/crossplane/crossplane-runtime/pkg/test" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - gomegaTypes "github.com/onsi/gomega/types" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -21,16 +20,16 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) -var _ = Describe("Test LocateParentAppConfig helper utils", func() { +func TestLocateParentAppConfig(t *testing.T) { ctx := context.Background() - namespace := "oamNS" + const namespace = "oamNS" acKind := reflect.TypeOf(v1alpha2.ApplicationConfiguration{}).Name() mockVersion := "core.oam.dev/v1alpha2" acName := "mockAC" @@ -82,177 +81,174 @@ var _ = Describe("Test LocateParentAppConfig helper utils", func() { mockCompWithEmptyOwnerRef.ObjectMeta.OwnerReferences = nil getErr := fmt.Errorf("get error") + type fields struct { + getFunc test.ObjectFn + oamObj oam.Object + } + type want struct { + ac oam.Object + err error + } - It("Test LocateParentAppConfig", func() { - type fields struct { - getFunc test.ObjectFn - oamObj oam.Object - } - type want struct { - ac oam.Object - err error - } - - cases := map[string]struct { - fields fields - want want - }{ - "LocateParentAppConfig fail when getAppConfig fails": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - oamObj: &mockComp, - }, - want: want{ - ac: nil, - err: getErr, + cases := map[string]struct { + fields fields + want want + }{ + "LocateParentAppConfig fail when getAppConfig fails": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr }, + oamObj: &mockComp, }, + want: want{ + ac: nil, + err: getErr, + }, + }, - "LocateParentAppConfig fail when no ApplicationConfiguration in OwnerReferences": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - oamObj: &mockCompWithEmptyOwnerRef, - }, - want: want{ - ac: nil, - err: errors.Errorf(util.ErrLocateAppConfig), + "LocateParentAppConfig fail when no ApplicationConfiguration in OwnerReferences": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr }, + oamObj: &mockCompWithEmptyOwnerRef, }, - "LocateParentAppConfig success": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.ApplicationConfiguration) - ac := mockAC - *o = ac - return nil - }, - oamObj: &mockComp, - }, - want: want{ - ac: &mockAC, - err: nil, + want: want{ + ac: nil, + err: errors.Errorf(util.ErrLocateAppConfig), + }, + }, + "LocateParentAppConfig success": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.ApplicationConfiguration) + ac := mockAC + *o = ac + return nil }, + oamObj: &mockComp, + }, + want: want{ + ac: &mockAC, + err: nil, }, + }, + } + for name, tc := range cases { + tclient := test.MockClient{ + MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - for name, tc := range cases { - tclient := test.MockClient{ - MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), - } - got, err := util.LocateParentAppConfig(ctx, &tclient, tc.fields.oamObj) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - if tc.want.ac == nil { - Expect(tc.want.ac).Should(BeNil()) - } else { - Expect(tc.want.ac).Should(Equal(got)) - } + got, err := util.LocateParentAppConfig(ctx, &tclient, tc.fields.oamObj) + t.Log(fmt.Sprint("Running test: ", name)) + if tc.want.err == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, tc.want.err.Error(), err.Error()) } - }) -}) - -var _ = Describe(" Trait Controller Test", func() { - BeforeEach(func() { - logf.Log.Info("Set up resources before a unit test") - }) + if tc.want.ac == nil { + assert.Equal(t, tc.want.ac, nil) + } else { + assert.Equal(t, tc.want.ac, got) + } + } +} - AfterEach(func() { - logf.Log.Info("Clean up resources after a unit test") - }) +func TestFetchWorkloadTraitReference(t *testing.T) { - It("Test fetch the workload the trait is reference to", func() { - By("Setting up variables") - log := ctrl.Log.WithName("ManualScalarTraitReconciler") - noRefNameTrait := v1alpha2.ManualScalerTrait{ - TypeMeta: metav1.TypeMeta{ - APIVersion: v1alpha2.SchemeGroupVersion.String(), - Kind: v1alpha2.ManualScalerTraitKind, + t.Log("Setting up variables") + log := ctrl.Log.WithName("ManualScalarTraitReconciler") + noRefNameTrait := v1alpha2.ManualScalerTrait{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.ManualScalerTraitKind, + }, + Spec: v1alpha2.ManualScalerTraitSpec{ + ReplicaCount: 3, + WorkloadReference: v1alpha1.TypedReference{ + APIVersion: "apiversion", + Kind: "Kind", }, - Spec: v1alpha2.ManualScalerTraitSpec{ - ReplicaCount: 3, - WorkloadReference: v1alpha1.TypedReference{ - APIVersion: "apiversion", - Kind: "Kind", - }, + }, + } + // put the workload name back + manualScalar := noRefNameTrait + manualScalar.Spec.WorkloadReference.Name = "wokload-example" + ctx := context.Background() + wl := v1alpha2.ContainerizedWorkload{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.ContainerizedWorkloadKind, + }, + } + uwl, _ := util.Object2Unstructured(wl) + refErr := errors.New("no workload reference") + workloadErr := fmt.Errorf("workload errr") + + type fields struct { + trait oam.Trait + getFunc test.ObjectFn + } + type want struct { + wl *unstructured.Unstructured + err error + } + cases := map[string]struct { + fields fields + want want + }{ + "FetchWorkload fail with mal-structured workloadRef": { + fields: fields{ + trait: &noRefNameTrait, }, - } - // put the workload name back - manualScalar := noRefNameTrait - manualScalar.Spec.WorkloadReference.Name = "wokload-example" - ctx := context.Background() - wl := v1alpha2.ContainerizedWorkload{ - TypeMeta: metav1.TypeMeta{ - APIVersion: v1alpha2.SchemeGroupVersion.String(), - Kind: v1alpha2.ContainerizedWorkloadKind, + want: want{ + wl: nil, + err: refErr, }, - } - uwl, _ := util.Object2Unstructured(wl) - refErr := errors.New("no workload reference") - workloadErr := fmt.Errorf("workload errr") - - type fields struct { - trait oam.Trait - getFunc test.ObjectFn - } - type want struct { - wl *unstructured.Unstructured - err error - } - cases := map[string]struct { - fields fields - want want - }{ - "FetchWorkload fail with mal-structured workloadRef": { - fields: fields{ - trait: &noRefNameTrait, - }, - want: want{ - wl: nil, - err: refErr, + }, + "FetchWorkload fails when getWorkload fails": { + fields: fields{ + trait: &manualScalar, + getFunc: func(obj runtime.Object) error { + return workloadErr }, }, - "FetchWorkload fails when getWorkload fails": { - fields: fields{ - trait: &manualScalar, - getFunc: func(obj runtime.Object) error { - return workloadErr - }, - }, - want: want{ - wl: nil, - err: workloadErr, - }, + want: want{ + wl: nil, + err: workloadErr, }, - "FetchWorkload succeeds when getWorkload succeeds": { - fields: fields{ - trait: &manualScalar, - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*unstructured.Unstructured) - *o = *uwl - return nil - }, - }, - want: want{ - wl: uwl, - err: nil, + }, + "FetchWorkload succeeds when getWorkload succeeds": { + fields: fields{ + trait: &manualScalar, + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*unstructured.Unstructured) + *o = *uwl + return nil }, }, + want: want{ + wl: uwl, + err: nil, + }, + }, + } + for name, tc := range cases { + tclient := test.NewMockClient() + tclient.MockGet = test.NewMockGetFn(nil, tc.fields.getFunc) + gotWL, err := util.FetchWorkload(ctx, tclient, log, tc.fields.trait) + t.Log(fmt.Sprint("Running test: ", name)) + if tc.want.err == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, tc.want.err.Error(), err.Error()) } - for name, tc := range cases { - tclient := test.NewMockClient() - tclient.MockGet = test.NewMockGetFn(nil, tc.fields.getFunc) - gotWL, err := util.FetchWorkload(ctx, tclient, log, tc.fields.trait) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - Expect(tc.want.wl).Should(Equal(gotWL)) - } - }) -}) + assert.Equal(t, tc.want.wl, gotWL) + } +} + +func TestScopeRelatedUtils(t *testing.T) { -var _ = Describe("Test scope related helper utils", func() { ctx := context.Background() namespace := "oamNS" scopeDefinitionKind := "ScopeDefinition" @@ -302,66 +298,64 @@ var _ = Describe("Test scope related helper utils", func() { getErr := fmt.Errorf("get error") - It("Test FetchScopeDefinition", func() { - type fields struct { - getFunc test.ObjectFn - } - type want struct { - spd *v1alpha2.ScopeDefinition - err error - } + type fields struct { + getFunc test.ObjectFn + } + type want struct { + spd *v1alpha2.ScopeDefinition + err error + } - cases := map[string]struct { - fields fields - want want - }{ - "FetchScopeDefinition fail when getScopeDefinition fails": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - }, - want: want{ - spd: nil, - err: getErr, + cases := map[string]struct { + fields fields + want want + }{ + "FetchScopeDefinition fail when getScopeDefinition fails": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr }, }, + want: want{ + spd: nil, + err: getErr, + }, + }, - "FetchScopeDefinition Success": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.ScopeDefinition) - sd := mockScopeDefinition - *o = sd - return nil - }, - }, - want: want{ - spd: &mockScopeDefinition, - err: nil, + "FetchScopeDefinition Success": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.ScopeDefinition) + sd := mockScopeDefinition + *o = sd + return nil }, }, + want: want{ + spd: &mockScopeDefinition, + err: nil, + }, + }, + } + for name, tc := range cases { + tclient := test.MockClient{ + MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - for name, tc := range cases { - tclient := test.MockClient{ - MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), - } - got, err := util.FetchScopeDefinition(ctx, &tclient, unstructuredScope) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - Expect(tc.want.spd).Should(Equal(got)) - } - }) -}) + got, err := util.FetchScopeDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredScope) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.err, err) + assert.Equal(t, tc.want.spd, got) + } +} -var _ = Describe("Test trait related helper utils", func() { +func TestTraitHelper(t *testing.T) { ctx := context.Background() namespace := "oamNS" traitDefinitionKind := "TraitDefinition" mockVerision := "core.oam.dev/v1alpha2" - traiitDefinitionName := "mocktraits.core.oam.dev" - traiitDefinitionRefName := "mocktraits.core.oam.dev" - traiitDefinitionWorkloadRefPath := "spec.workloadRef" + traitDefinitionName := "mocktraits.core.oam.dev" + traitDefinitionRefName := "mocktraits.core.oam.dev" + traitDefinitionWorkloadRefPath := "spec.workloadRef" mockTraitDefinition := v1alpha2.TraitDefinition{ TypeMeta: metav1.TypeMeta{ @@ -369,15 +363,15 @@ var _ = Describe("Test trait related helper utils", func() { APIVersion: mockVerision, }, ObjectMeta: metav1.ObjectMeta{ - Name: traiitDefinitionName, + Name: traitDefinitionName, Namespace: namespace, }, Spec: v1alpha2.TraitDefinitionSpec{ Reference: v1alpha2.DefinitionReference{ - Name: traiitDefinitionRefName, + Name: traitDefinitionRefName, }, RevisionEnabled: false, - WorkloadRefPath: traiitDefinitionWorkloadRefPath, + WorkloadRefPath: traitDefinitionWorkloadRefPath, AppliesToWorkloads: nil, }, } @@ -397,60 +391,57 @@ var _ = Describe("Test trait related helper utils", func() { unstructuredTrait, _ := util.Object2Unstructured(mockTrait) getErr := fmt.Errorf("get error") + type fields struct { + getFunc test.ObjectFn + } + type want struct { + td *v1alpha2.TraitDefinition + err error + } - It("Test FetchTraitDefinition", func() { - type fields struct { - getFunc test.ObjectFn - } - type want struct { - td *v1alpha2.TraitDefinition - err error - } - - cases := map[string]struct { - fields fields - want want - }{ - "FetchTraitDefinition fail when getTraitDefinition fails": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - }, - want: want{ - td: nil, - err: getErr, + cases := map[string]struct { + fields fields + want want + }{ + "FetchTraitDefinition fail when getTraitDefinition fails": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr }, }, + want: want{ + td: nil, + err: getErr, + }, + }, - "FetchTraitDefinition Success": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.TraitDefinition) - td := mockTraitDefinition - *o = td - return nil - }, - }, - want: want{ - td: &mockTraitDefinition, - err: nil, + "FetchTraitDefinition Success": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.TraitDefinition) + td := mockTraitDefinition + *o = td + return nil }, }, + want: want{ + td: &mockTraitDefinition, + err: nil, + }, + }, + } + for name, tc := range cases { + tclient := test.MockClient{ + MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - for name, tc := range cases { - tclient := test.MockClient{ - MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), - } - got, err := util.FetchTraitDefinition(ctx, &tclient, unstructuredTrait) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - Expect(tc.want.td).Should(Equal(got)) - } - }) -}) + got, err := util.FetchTraitDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredTrait) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.err, err) + assert.Equal(t, tc.want.td, got) + } +} -var _ = Describe("Test workload related helper utils", func() { +func TestUtils(t *testing.T) { // Test common variables ctx := context.Background() namespace := "oamNS" @@ -459,7 +450,7 @@ var _ = Describe("Test workload related helper utils", func() { workloadAPIVersion := "core.oam.dev/v1" workloadDefinitionName := "containerizedworkloads.core.oam.dev" var workloadUID types.UID = "oamWorkloadUID" - log := ctrl.Log.WithName("ManualScalarTraitReconciler") + // workload CR workload := v1alpha2.ContainerizedWorkload{ ObjectMeta: metav1.ObjectMeta{ @@ -487,596 +478,611 @@ var _ = Describe("Test workload related helper utils", func() { getErr := fmt.Errorf("get failed") - BeforeEach(func() { - logf.Log.Info("Set up resources before a unit test") - }) - - AfterEach(func() { - logf.Log.Info("Clean up resources after a unit test") - }) - - It("Test fetch workloadDefinition", func() { - type fields struct { - getFunc test.ObjectFn - } - type want struct { - wld *v1alpha2.WorkloadDefinition - err error - } + type fields struct { + getFunc test.ObjectFn + } + type want struct { + wld *v1alpha2.WorkloadDefinition + err error + } - cases := map[string]struct { - fields fields - want want - }{ - "FetchWorkloadDefinition fail when getWorkloadDefinition fails": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - }, - want: want{ - wld: nil, - err: getErr, + cases := map[string]struct { + fields fields + want want + }{ + "FetchWorkloadDefinition fail when getWorkloadDefinition fails": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr }, }, + want: want{ + wld: nil, + err: getErr, + }, + }, - "FetchWorkloadDefinition Success": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.WorkloadDefinition) - w := workloadDefinition - *o = w - return nil - }, - }, - want: want{ - wld: &workloadDefinition, - err: nil, + "FetchWorkloadDefinition Success": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.WorkloadDefinition) + w := workloadDefinition + *o = w + return nil }, }, + want: want{ + wld: &workloadDefinition, + err: nil, + }, + }, + } + for name, tc := range cases { + tclient := test.MockClient{ + MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - for name, tc := range cases { - tclient := test.MockClient{ - MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), - } - got, err := util.FetchWorkloadDefinition(ctx, &tclient, unstructuredWorkload) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - Expect(tc.want.wld).Should(Equal(got)) - } - }) + got, err := util.FetchWorkloadDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredWorkload) + t.Log(fmt.Sprint("Running test: ", name)) - It("Test extract child resources from any workload", func() { - crkl := []v1alpha2.ChildResourceKind{ - { - Kind: "Deployment", - APIVersion: "apps/v1", + assert.Equal(t, tc.want.err, err) + assert.Equal(t, tc.want.wld, got) + } +} + +func TestChildResources(t *testing.T) { + var workloadUID types.UID = "oamWorkloadUID" + workloadDefinitionName := "containerizedworkloads.core.oam.dev" + namespace := "oamNS" + workloadName := "oamWorkload" + workloadKind := "ContainerizedWorkload" + workloadAPIVersion := "core.oam.dev/v1" + // workload CR + workload := v1alpha2.ContainerizedWorkload{ + ObjectMeta: metav1.ObjectMeta{ + Name: workloadName, + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: workloadAPIVersion, + Kind: workloadKind, + }, + } + workload.SetUID(workloadUID) + unstructuredWorkload, _ := util.Object2Unstructured(workload) + ctx := context.Background() + getErr := fmt.Errorf("get failed") + // workload Definition + workloadDefinition := v1alpha2.WorkloadDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: workloadDefinitionName, + }, + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: workloadDefinitionName, }, - { - Kind: "Service", - APIVersion: "v1", + }, + } + + log := ctrl.Log.WithName("ManualScalarTraitReconciler") + crkl := []v1alpha2.ChildResourceKind{ + { + Kind: "Deployment", + APIVersion: "apps/v1", + }, + { + Kind: "Service", + APIVersion: "v1", + }, + } + // cdResource is the child deployment owned by the workload + cdResource := unstructured.Unstructured{} + cdResource.SetOwnerReferences([]metav1.OwnerReference{ + { + Kind: util.KindDeployment, + UID: workloadUID, + }, + }) + // cdResource is the child service owned by the workload + cSResource := unstructured.Unstructured{} + cSResource.SetOwnerReferences([]metav1.OwnerReference{ + { + Kind: util.KindService, + UID: workloadUID, + }, + }) + // oResource is not owned by the workload + oResource := unstructured.Unstructured{} + oResource.SetOwnerReferences([]metav1.OwnerReference{ + { + UID: "NotWorkloadUID", + }, + }) + var nilListFunc test.ObjectFn = func(o runtime.Object) error { + u := &unstructured.Unstructured{} + l := o.(*unstructured.UnstructuredList) + l.Items = []unstructured.Unstructured{*u} + return nil + } + type fields struct { + getFunc test.ObjectFn + listFunc test.ObjectFn + } + type want struct { + crks []*unstructured.Unstructured + err error + } + + cases := map[string]struct { + fields fields + want want + }{ + "FetchWorkloadChildResources fail when getWorkloadDefinition fails": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + return getErr + }, + listFunc: nilListFunc, }, - } - // cdResource is the child deployment owned by the workload - cdResource := unstructured.Unstructured{} - cdResource.SetOwnerReferences([]metav1.OwnerReference{ - { - Kind: util.KindDeployment, - UID: workloadUID, + want: want{ + crks: nil, + err: getErr, }, - }) - // cdResource is the child service owned by the workload - cSResource := unstructured.Unstructured{} - cSResource.SetOwnerReferences([]metav1.OwnerReference{ - { - Kind: util.KindService, - UID: workloadUID, + }, + "FetchWorkloadChildResources return nothing when the workloadDefinition doesn't have child list": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.WorkloadDefinition) + *o = workloadDefinition + return nil + }, + listFunc: nilListFunc, }, - }) - // oResource is not owned by the workload - oResource := unstructured.Unstructured{} - oResource.SetOwnerReferences([]metav1.OwnerReference{ - { - UID: "NotWorkloadUID", + want: want{ + crks: nil, + err: nil, }, - }) - var nilListFunc test.ObjectFn = func(o runtime.Object) error { - u := &unstructured.Unstructured{} - l := o.(*unstructured.UnstructuredList) - l.Items = []unstructured.Unstructured{*u} - return nil - } - type fields struct { - getFunc test.ObjectFn - listFunc test.ObjectFn - } - type want struct { - crks []*unstructured.Unstructured - err error - } - - cases := map[string]struct { - fields fields - want want - }{ - "FetchWorkloadChildResources fail when getWorkloadDefinition fails": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - return getErr - }, - listFunc: nilListFunc, + }, + "FetchWorkloadChildResources Success": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.WorkloadDefinition) + w := workloadDefinition + w.Spec.ChildResourceKinds = crkl + *o = w + return nil }, - want: want{ - crks: nil, - err: getErr, + listFunc: func(o runtime.Object) error { + l := o.(*unstructured.UnstructuredList) + switch l.GetKind() { + case util.KindDeployment: + l.Items = append(l.Items, cdResource) + case util.KindService: + l.Items = append(l.Items, cSResource) + default: + return getErr + } + return nil }, }, - "FetchWorkloadChildResources return nothing when the workloadDefinition doesn't have child list": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.WorkloadDefinition) - *o = workloadDefinition - return nil - }, - listFunc: nilListFunc, - }, - want: want{ - crks: nil, - err: nil, + want: want{ + crks: []*unstructured.Unstructured{ + &cdResource, &cSResource, }, + err: nil, }, - "FetchWorkloadChildResources Success": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.WorkloadDefinition) - w := workloadDefinition - w.Spec.ChildResourceKinds = crkl - *o = w - return nil - }, - listFunc: func(o runtime.Object) error { - l := o.(*unstructured.UnstructuredList) - if l.GetKind() == util.KindDeployment { - l.Items = append(l.Items, cdResource) - } else if l.GetKind() == util.KindService { - l.Items = append(l.Items, cSResource) - } else { - return getErr - } - return nil - }, + }, + "FetchWorkloadChildResources with many resources only pick the child one": { + fields: fields{ + getFunc: func(obj runtime.Object) error { + o, _ := obj.(*v1alpha2.WorkloadDefinition) + w := workloadDefinition + w.Spec.ChildResourceKinds = crkl + *o = w + return nil }, - want: want{ - crks: []*unstructured.Unstructured{ - &cdResource, &cSResource, - }, - err: nil, + listFunc: func(o runtime.Object) error { + l := o.(*unstructured.UnstructuredList) + l.Items = []unstructured.Unstructured{oResource, oResource, oResource, oResource, + oResource, oResource, oResource} + if l.GetKind() == util.KindDeployment { + l.Items = append(l.Items, cdResource) + } else if l.GetKind() != util.KindService { + return getErr + } + return nil }, }, - "FetchWorkloadChildResources with many resources only pick the child one": { - fields: fields{ - getFunc: func(obj runtime.Object) error { - o, _ := obj.(*v1alpha2.WorkloadDefinition) - w := workloadDefinition - w.Spec.ChildResourceKinds = crkl - *o = w - return nil - }, - listFunc: func(o runtime.Object) error { - l := o.(*unstructured.UnstructuredList) - l.Items = []unstructured.Unstructured{oResource, oResource, oResource, oResource, - oResource, oResource, oResource} - if l.GetKind() == util.KindDeployment { - l.Items = append(l.Items, cdResource) - } else if l.GetKind() != util.KindService { - return getErr - } - return nil - }, - }, - want: want{ - crks: []*unstructured.Unstructured{ - &cdResource, - }, - err: nil, + want: want{ + crks: []*unstructured.Unstructured{ + &cdResource, }, + err: nil, }, + }, + } + for name, tc := range cases { + tclient := test.MockClient{ + MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), + MockList: test.NewMockListFn(nil, tc.fields.listFunc), } - for name, tc := range cases { - tclient := test.MockClient{ - MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), - MockList: test.NewMockListFn(nil, tc.fields.listFunc), - } - got, err := util.FetchWorkloadChildResources(ctx, log, &tclient, unstructuredWorkload) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.err).Should(util.BeEquivalentToError(err)) - Expect(tc.want.crks).Should(Equal(got)) - } - }) -}) - -var _ = Describe("Test unstructured related helper utils", func() { - It("Test get CRD name from an unstructured object", func() { - tests := map[string]struct { - u *unstructured.Unstructured - typeLabel string - exp string - }{ - "native resource": { - u: &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - }}, - exp: "deployments.apps", - }, - "extended resource": { - u: &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": "extend.oam.dev/v1alpha2", - "kind": "SimpleRolloutTrait", - }}, - exp: "simplerollouttraits.extend.oam.dev", - }, - "trait": { - u: &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": "extend.oam.dev/v1alpha2", - "kind": "SimpleRolloutTrait", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - oam.TraitTypeLabel: "rollout", - }, + got, err := util.FetchWorkloadChildResources(ctx, log, &tclient, mock.NewMockMapper(), unstructuredWorkload) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.err, err) + assert.Equal(t, tc.want.crks, got) + } +} + +func TestUnstructured(t *testing.T) { + tests := map[string]struct { + u *unstructured.Unstructured + typeLabel string + exp string + resource string + }{ + "native resource": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + }}, + resource: "deployments", + exp: "deployments.apps", + }, + "extended resource": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "extend.oam.dev/v1alpha2", + "kind": "SimpleRolloutTrait", + }}, + resource: "simplerollouttraits", + exp: "simplerollouttraits.extend.oam.dev", + }, + "trait": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "extend.oam.dev/v1alpha2", + "kind": "SimpleRolloutTrait", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + oam.TraitTypeLabel: "rollout", }, - }}, - typeLabel: oam.TraitTypeLabel, - exp: "rollout", - }, - "workload": { - u: &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - oam.WorkloadTypeLabel: "deploy", - }, + }, + }}, + typeLabel: oam.TraitTypeLabel, + exp: "rollout", + }, + "workload": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + oam.WorkloadTypeLabel: "deploy", }, - }}, - typeLabel: oam.WorkloadTypeLabel, - exp: "deploy", - }, - } - for name, ti := range tests { - got := util.GetDefinitionName(ti.u, ti.typeLabel) - By(fmt.Sprint("Running test: ", name)) - Expect(got).Should(Equal(ti.exp)) - } - }) -}) - -var _ = Describe("Test GenTraitName helper utils", func() { - It("Test generate trait name", func() { + }, + }}, + typeLabel: oam.WorkloadTypeLabel, + exp: "deploy", + }, + } + for name, ti := range tests { + mapper := mock.NewMockMapper() + mapper.MockRESTMapping = mock.NewMockRESTMapping(ti.resource) + got, err := util.GetDefinitionName(mapper, ti.u, ti.typeLabel) + assert.NoError(t, err) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, ti.exp, got) + } +} - mts := v1alpha2.ManualScalerTrait{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - Name: "sample-manualscaler-trait", - }, - Spec: v1alpha2.ManualScalerTraitSpec{ - ReplicaCount: 3, - }, - } +func TestGenTraitName(t *testing.T) { + mts := v1alpha2.ManualScalerTrait{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "sample-manualscaler-trait", + }, + Spec: v1alpha2.ManualScalerTraitSpec{ + ReplicaCount: 3, + }, + } - test := []struct { - name string - template *v1alpha2.ComponentTrait - exp string - }{ - { - name: "simple", - template: &v1alpha2.ComponentTrait{}, - exp: "simple-trait-67b8949f8d", - }, - { - name: "simple", - template: &v1alpha2.ComponentTrait{ - Trait: runtime.RawExtension{ - Object: &mts, - }, + test := []struct { + name string + template *v1alpha2.ComponentTrait + exp string + }{ + { + name: "simple", + template: &v1alpha2.ComponentTrait{}, + exp: "simple-trait-67b8949f8d", + }, + { + name: "simple", + template: &v1alpha2.ComponentTrait{ + Trait: runtime.RawExtension{ + Object: &mts, }, - exp: "simple-trait-5ddc8b7556", }, - } - for _, test := range test { - - got := util.GenTraitName(test.name, test.template) - By(fmt.Sprint("Running test: ", test.name)) - Expect(test.exp).Should(Equal(got)) - } - - }) -}) + exp: "simple-trait-5ddc8b7556", + }, + } + for _, test := range test { -var _ = Describe("Test ComputeHash helper utils", func() { - It("Test generate hash", func() { + got := util.GenTraitName(test.name, test.template) + t.Log(fmt.Sprint("Running test: ", test.name)) + assert.Equal(t, test.exp, got) + } +} - mts := v1alpha2.ManualScalerTrait{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - Name: "sample-manualscaler-trait", - }, - Spec: v1alpha2.ManualScalerTraitSpec{ - ReplicaCount: 3, - }, - } +func TestComputeHash(t *testing.T) { + mts := v1alpha2.ManualScalerTrait{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "sample-manualscaler-trait", + }, + Spec: v1alpha2.ManualScalerTraitSpec{ + ReplicaCount: 3, + }, + } - test := []struct { - name string - template *v1alpha2.ComponentTrait - exp string - }{ - { - name: "simple", - template: &v1alpha2.ComponentTrait{}, - exp: "67b8949f8d", - }, - { - name: "simple", - template: &v1alpha2.ComponentTrait{ - Trait: runtime.RawExtension{ - Object: &mts, - }, + test := []struct { + name string + template *v1alpha2.ComponentTrait + exp string + }{ + { + name: "simple", + template: &v1alpha2.ComponentTrait{}, + exp: "67b8949f8d", + }, + { + name: "simple", + template: &v1alpha2.ComponentTrait{ + Trait: runtime.RawExtension{ + Object: &mts, }, - exp: "5ddc8b7556", }, - } - for _, test := range test { - got := util.ComputeHash(test.template) - - By(fmt.Sprint("Running test: ", got)) - Expect(test.exp).Should(Equal(got)) - } - }) -}) + exp: "5ddc8b7556", + }, + } + for _, test := range test { + got := util.ComputeHash(test.template) -var _ = Describe("Test DeepHashObject helper utils", func() { - It("Test generate hash", func() { + t.Log(fmt.Sprint("Running test: ", got)) + assert.Equal(t, test.exp, got) + } +} - successCases := []func() interface{}{ - func() interface{} { return 8675309 }, - func() interface{} { return "Jenny, I got your number" }, - func() interface{} { return []string{"eight", "six", "seven"} }, - } +func TestDeepHashObject(t *testing.T) { + successCases := []func() interface{}{ + func() interface{} { return 8675309 }, + func() interface{} { return "Jenny, I got your number" }, + func() interface{} { return []string{"eight", "six", "seven"} }, + } - for _, tc := range successCases { - hasher1 := adler32.New() - util.DeepHashObject(hasher1, tc()) - hash1 := hasher1.Sum32() - util.DeepHashObject(hasher1, tc()) - hash2 := hasher1.Sum32() + for _, tc := range successCases { + hasher1 := adler32.New() + util.DeepHashObject(hasher1, tc()) + hash1 := hasher1.Sum32() + util.DeepHashObject(hasher1, tc()) + hash2 := hasher1.Sum32() - Expect(hash1).Should(Equal(hash2)) - } - }) -}) - -var _ = Describe("Test PatchCondition helper utils", func() { - It("Test PatchCondition", func() { - type args struct { - ctx context.Context - r client.StatusClient - workload util.ConditionedObject - condition []v1alpha1.Condition - } - patchErr := fmt.Errorf("eww") - tests := []struct { - name string - args args - expected error - }{ - { - name: "success", - args: args{ - ctx: context.Background(), - r: &test.MockClient{ - MockStatusPatch: test.NewMockStatusPatchFn(nil), - }, - workload: &fake.Claim{}, - condition: []v1alpha1.Condition{ - {}, - }, + assert.Equal(t, hash1, hash2) + } +} + +func TestPatchCondition(t *testing.T) { + type args struct { + ctx context.Context + r client.StatusClient + workload util.ConditionedObject + condition []v1alpha1.Condition + } + patchErr := fmt.Errorf("eww") + tests := []struct { + name string + args args + expected error + }{ + { + name: "success", + args: args{ + ctx: context.Background(), + r: &test.MockClient{ + MockStatusPatch: test.NewMockStatusPatchFn(nil), + }, + workload: &fake.Claim{}, + condition: []v1alpha1.Condition{ + {}, }, - expected: nil, }, - { - name: "fail", - args: args{ - ctx: context.Background(), - r: &test.MockClient{ - MockStatusPatch: test.NewMockStatusPatchFn(patchErr), - }, - workload: &fake.Claim{}, - condition: []v1alpha1.Condition{ - {}, - }, + expected: nil, + }, + { + name: "fail", + args: args{ + ctx: context.Background(), + r: &test.MockClient{ + MockStatusPatch: test.NewMockStatusPatchFn(patchErr), + }, + workload: &fake.Claim{}, + condition: []v1alpha1.Condition{ + {}, }, - expected: errors.Wrap(patchErr, util.ErrUpdateStatus), }, + expected: errors.Wrap(patchErr, util.ErrUpdateStatus), + }, + } + for _, tt := range tests { + err := util.PatchCondition(tt.args.ctx, tt.args.r, tt.args.workload, tt.args.condition...) + if tt.expected == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, tt.expected.Error(), err.Error()) } - for _, tt := range tests { - err := util.PatchCondition(tt.args.ctx, tt.args.r, tt.args.workload, tt.args.condition...) - if tt.expected == nil { - BeNil().Match(err) - } else { - Expect(err.Error()).Should(Equal(tt.expected.Error())) - } - } - }) -}) + } +} -var _ = Describe("Test get component helper utils", func() { +func TestComponentHelper(t *testing.T) { ctx := context.Background() - It("Test Get Component", func() { - type Case struct { - caseName string - acc v1alpha2.ApplicationConfigurationComponent - expectComponent *v1alpha2.Component - expectRevisionName string - expectErrorMatcher gomegaTypes.GomegaMatcher - } + type Case struct { + caseName string + acc v1alpha2.ApplicationConfigurationComponent + expectComponent *v1alpha2.Component + expectRevisionName string + expectErrorMatcher bool + } - namespace := "ns" - componentName := "newcomponent" - invalidComponentName := "invalidComponent" - revisionName := "newcomponent-aa1111" - revisionName2 := "newcomponent-bb1111" - unpackErrorRevisionName := "unpackErrorRevision" - errComponentNotFound := errors.New("component not found") - - componnet1 := v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, - Status: v1alpha2.ComponentStatus{}, - } + namespace := "ns" + componentName := "newcomponent" + invalidComponentName := "invalidComponent" + revisionName := "newcomponent-aa1111" + revisionName2 := "newcomponent-bb1111" + unpackErrorRevisionName := "unpackErrorRevision" + errComponentNotFound := errors.New("component not found") - component2 := v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "apiVersion": "New", - }, - }}}}, - Status: v1alpha2.ComponentStatus{ - LatestRevision: &v1alpha2.Revision{Name: revisionName2, Revision: 2}, + componnet1 := v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, + Status: v1alpha2.ComponentStatus{}, + } + + component2 := v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "apiVersion": "New", }, - } + }}}}, + Status: v1alpha2.ComponentStatus{ + LatestRevision: &v1alpha2.Revision{Name: revisionName2, Revision: 2}, + }, + } - client := &test.MockClient{MockGet: func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.Component); ok { - switch key.Name { - case componentName: - *o = component2 - case invalidComponentName: - return errComponentNotFound - default: - return nil - } + client := &test.MockClient{MockGet: func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { + if o, ok := obj.(*v1alpha2.Component); ok { + switch key.Name { + case componentName: + *o = component2 + case invalidComponentName: + return errComponentNotFound + default: + return nil } - if o, ok := obj.(*appsv1.ControllerRevision); ok { - switch key.Name { - case revisionName: - *o = appsv1.ControllerRevision{ - ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, - Data: runtime.RawExtension{Object: &componnet1}, - Revision: 1, - } - case unpackErrorRevisionName: - *o = appsv1.ControllerRevision{ - ObjectMeta: metav1.ObjectMeta{Name: unpackErrorRevisionName, Namespace: namespace}, - Data: runtime.RawExtension{}, - Revision: 1, - } - default: - return nil + } + if o, ok := obj.(*appsv1.ControllerRevision); ok { + switch key.Name { + case revisionName: + *o = appsv1.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, + Data: runtime.RawExtension{Object: &componnet1}, + Revision: 1, + } + case unpackErrorRevisionName: + *o = appsv1.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{Name: unpackErrorRevisionName, Namespace: namespace}, + Data: runtime.RawExtension{}, + Revision: 1, } + default: + return nil } - return nil - - }} - testCases := []Case{ - { - caseName: "get component by revisionName", - acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: revisionName}, - expectComponent: &componnet1, - expectRevisionName: revisionName, - expectErrorMatcher: BeNil(), - }, - { - caseName: "get component by componentName", - acc: v1alpha2.ApplicationConfigurationComponent{ComponentName: componentName}, - expectComponent: &component2, - expectRevisionName: revisionName2, - expectErrorMatcher: BeNil(), - }, - { - caseName: "not found error occurs when get by revisionName", - acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: "invalidRevisionName"}, - expectComponent: nil, - expectRevisionName: "", - expectErrorMatcher: Not(BeNil()), - }, - { - caseName: "unpack revison data error occurs when get by revisionName", - acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: unpackErrorRevisionName}, - expectComponent: nil, - expectRevisionName: "", - expectErrorMatcher: Not(BeNil()), - }, - { - caseName: "error occurs when get by componentName", - acc: v1alpha2.ApplicationConfigurationComponent{ComponentName: invalidComponentName}, - expectComponent: nil, - expectRevisionName: "", - expectErrorMatcher: Not(BeNil()), - }, - } - - for _, tc := range testCases { - By("Running:" + tc.caseName) - c, r, err := util.GetComponent(ctx, client, tc.acc, namespace) - Expect(c).Should(Equal(tc.expectComponent)) - Expect(r).Should(Equal(tc.expectRevisionName)) - Expect(err).Should(tc.expectErrorMatcher) } - }) + return nil + + }} + testCases := []Case{ + { + caseName: "get component by revisionName", + acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: revisionName}, + expectComponent: &componnet1, + expectRevisionName: revisionName, + expectErrorMatcher: true, + }, + { + caseName: "get component by componentName", + acc: v1alpha2.ApplicationConfigurationComponent{ComponentName: componentName}, + expectComponent: &component2, + expectRevisionName: revisionName2, + expectErrorMatcher: true, + }, + { + caseName: "not found error occurs when get by revisionName", + acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: "invalidRevisionName"}, + expectComponent: nil, + expectRevisionName: "", + expectErrorMatcher: false, + }, + { + caseName: "unpack revison data error occurs when get by revisionName", + acc: v1alpha2.ApplicationConfigurationComponent{RevisionName: unpackErrorRevisionName}, + expectComponent: nil, + expectRevisionName: "", + expectErrorMatcher: false, + }, + { + caseName: "error occurs when get by componentName", + acc: v1alpha2.ApplicationConfigurationComponent{ComponentName: invalidComponentName}, + expectComponent: nil, + expectRevisionName: "", + expectErrorMatcher: false, + }, + } -}) - -var _ = Describe("Test UnpackRevisionData helper util", func() { - It("Test unpack revision data", func() { - comp1 := v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}} - comp1Raw, _ := json.Marshal(comp1) - tests := map[string]struct { - rev *appsv1.ControllerRevision - expComp *v1alpha2.Component - expErr error - reason string - }{ - "controllerRevision with Component Obj": { - rev: &appsv1.ControllerRevision{Data: runtime.RawExtension{Object: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}}}, - expComp: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}, - reason: "controllerRevision should align with component object", - }, - "controllerRevision with Unknown Obj": { - rev: &appsv1.ControllerRevision{ObjectMeta: metav1.ObjectMeta{Name: "rev1"}, Data: runtime.RawExtension{Object: &runtime.Unknown{Raw: comp1Raw}}}, - reason: "controllerRevision must be decode into component object", - expErr: fmt.Errorf("invalid type of revision rev1, type should not be *runtime.Unknown"), - }, - "unmarshal with component data": { - rev: &appsv1.ControllerRevision{ObjectMeta: metav1.ObjectMeta{Name: "rev1"}, Data: runtime.RawExtension{Raw: comp1Raw}}, - reason: "controllerRevision should unmarshal data and align with component object", - expComp: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}, - }, + for _, tc := range testCases { + t.Log("Running:" + tc.caseName) + c, r, err := util.GetComponent(ctx, client, tc.acc, namespace) + assert.Equal(t, tc.expectComponent, c) + assert.Equal(t, tc.expectRevisionName, r) + if tc.expectErrorMatcher { + assert.NoError(t, err) + } else { + assert.Error(t, err) } - for name, ti := range tests { - By("Running: " + name) - comp, err := util.UnpackRevisionData(ti.rev) - if ti.expErr != nil { - Expect(err).Should(Equal(ti.expErr)) - } else { - Expect(err).Should(BeNil()) - Expect(comp).Should(Equal(ti.expComp)) - } + } +} + +func TestUnpackRevisionData(t *testing.T) { + comp1 := v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}} + comp1Raw, _ := json.Marshal(comp1) + tests := map[string]struct { + rev *appsv1.ControllerRevision + expComp *v1alpha2.Component + expErr error + reason string + }{ + "controllerRevision with Component Obj": { + rev: &appsv1.ControllerRevision{Data: runtime.RawExtension{Object: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}}}, + expComp: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}, + reason: "controllerRevision should align with component object", + }, + "controllerRevision with Unknown Obj": { + rev: &appsv1.ControllerRevision{ObjectMeta: metav1.ObjectMeta{Name: "rev1"}, Data: runtime.RawExtension{Object: &runtime.Unknown{Raw: comp1Raw}}}, + reason: "controllerRevision must be decode into component object", + expErr: fmt.Errorf("invalid type of revision rev1, type should not be *runtime.Unknown"), + }, + "unmarshal with component data": { + rev: &appsv1.ControllerRevision{ObjectMeta: metav1.ObjectMeta{Name: "rev1"}, Data: runtime.RawExtension{Raw: comp1Raw}}, + reason: "controllerRevision should unmarshal data and align with component object", + expComp: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp1"}}, + }, + } + for name, ti := range tests { + t.Log("Running: " + name) + comp, err := util.UnpackRevisionData(ti.rev) + if ti.expErr != nil { + assert.Equal(t, ti.expErr, err) + } else { + assert.NoError(t, err) + assert.Equal(t, ti.expComp, comp) } + } +} - }) - -}) - -var _ = Describe("TestPassThroughObjMeta", func() { +func TestPassThroughObjMeta(t *testing.T) { ac := &v1alpha2.ApplicationConfiguration{} labels := map[string]string{ @@ -1091,95 +1097,88 @@ var _ = Describe("TestPassThroughObjMeta", func() { ac.SetLabels(labels) ac.SetAnnotations(annotation) + t.Log("workload and trait have no labels and annotation") + var u unstructured.Unstructured + util.PassLabelAndAnnotation(ac, &u) + got := u.GetLabels() + want := labels + assert.Equal(t, want, got) + gotAnnotation := u.GetAnnotations() + wantAnnotation := annotation + assert.Equal(t, wantAnnotation, gotAnnotation) + t.Log("workload and trait contains overlapping keys") + existAnnotation := map[string]string{ + "key1": "exist value1", + "key3": "value3", + } + existLabels := map[string]string{ + "core.oam.dev/ns": "kube-system", + "core.oam.dev/kube-native": "deployment", + } + u.SetLabels(existLabels) + u.SetAnnotations(existAnnotation) - It("workload and trait have no labels and annotation", func() { - var u unstructured.Unstructured - util.PassLabelAndAnnotation(ac, &u) - got := u.GetLabels() - want := labels - Expect(got).Should(BeEquivalentTo(want)) - gotAnnotation := u.GetAnnotations() - wantAnnotation := annotation - Expect(gotAnnotation).Should(BeEquivalentTo(wantAnnotation)) - }) - - It("workload and trait contains overlapping keys", func() { - var u unstructured.Unstructured - existAnnotation := map[string]string{ - "key1": "exist value1", - "key3": "value3", - } - existLabels := map[string]string{ - "core.oam.dev/ns": "kube-system", - "core.oam.dev/kube-native": "deployment", - } - u.SetLabels(existLabels) - u.SetAnnotations(existAnnotation) - - util.PassLabelAndAnnotation(ac, &u) + util.PassLabelAndAnnotation(ac, &u) - gotAnnotation := u.GetAnnotations() - wantAnnotation := map[string]string{ - "key1": "exist value1", - "key2": "value2", - "key3": "value3", - } - Expect(gotAnnotation).Should(BeEquivalentTo(wantAnnotation)) + gotAnnotation = u.GetAnnotations() + wantAnnotation = map[string]string{ + "key1": "exist value1", + "key2": "value2", + "key3": "value3", + } + assert.Equal(t, wantAnnotation, gotAnnotation) - gotLabels := u.GetLabels() - wantLabels := map[string]string{ - "core.oam.dev/ns": "kube-system", - "core.oam.dev/kube-native": "deployment", - "core.oam.dev/controller": "oam-kubernetes-runtime", - } - Expect(gotLabels).Should(BeEquivalentTo(wantLabels)) + gotLabels := u.GetLabels() + wantLabels := map[string]string{ + "core.oam.dev/ns": "kube-system", + "core.oam.dev/kube-native": "deployment", + "core.oam.dev/controller": "oam-kubernetes-runtime", + } + assert.Equal(t, wantLabels, gotLabels) +} + +func TestAddLabels(t *testing.T) { + obj1 := new(unstructured.Unstructured) + wantObj1 := new(unstructured.Unstructured) + wantObj1.SetLabels(map[string]string{ + "newKey": "newValue", }) -}) - -var _ = Describe("TestAddLabels", func() { - It("Test AddLabels", func() { - obj1 := new(unstructured.Unstructured) - wantObj1 := new(unstructured.Unstructured) - wantObj1.SetLabels(map[string]string{ - "newKey": "newValue", - }) - obj2 := new(unstructured.Unstructured) - wantObj2 := new(unstructured.Unstructured) - obj2.SetLabels(map[string]string{ - "key": "value", - }) - wantObj2.SetLabels(map[string]string{ - "key": "value", - "newKey": "newValue", - }) - - cases := map[string]struct { - obj *unstructured.Unstructured - newLabels map[string]string - want *unstructured.Unstructured - }{ - "add labels to workload without labels": { - obj1, - map[string]string{ - "newKey": "newValue", - }, - wantObj1, + obj2 := new(unstructured.Unstructured) + wantObj2 := new(unstructured.Unstructured) + obj2.SetLabels(map[string]string{ + "key": "value", + }) + wantObj2.SetLabels(map[string]string{ + "key": "value", + "newKey": "newValue", + }) + + cases := map[string]struct { + obj *unstructured.Unstructured + newLabels map[string]string + want *unstructured.Unstructured + }{ + "add labels to workload without labels": { + obj1, + map[string]string{ + "newKey": "newValue", }, - "add labels to workload with labels": { - obj2, - map[string]string{ - "newKey": "newValue", - }, - wantObj2, + wantObj1, + }, + "add labels to workload with labels": { + obj2, + map[string]string{ + "newKey": "newValue", }, - } + wantObj2, + }, + } - for name, tc := range cases { - By("Running test case: " + name) - obj := tc.obj - wantObj := tc.want - util.AddLabels(obj, tc.newLabels) - Expect(obj.GetLabels()).Should(Equal(wantObj.GetLabels())) - } - }) -}) + for name, tc := range cases { + t.Log("Running test case: " + name) + obj := tc.obj + wantObj := tc.want + util.AddLabels(obj, tc.newLabels) + assert.Equal(t, wantObj.GetLabels(), obj.GetLabels()) + } +} diff --git a/pkg/oam/util/test_utils_test.go b/pkg/oam/util/test_utils_test.go index 37bdb2da..ded0bfbf 100644 --- a/pkg/oam/util/test_utils_test.go +++ b/pkg/oam/util/test_utils_test.go @@ -17,221 +17,204 @@ package util import ( "fmt" + "testing" + + "github.com/stretchr/testify/assert" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" - - logf "sigs.k8s.io/controller-runtime/pkg/log" ) -var _ = Describe("Test matchers", func() { - - BeforeEach(func() { - logf.Log.Info("Set up resources before a unit test") - }) - - AfterEach(func() { - logf.Log.Info("Clean up resources after a unit test") - }) - - It("Test AlreadyExistMatcher", func() { - type want struct { - success bool - err error - } - - cases := map[string]struct { - input interface{} - want want - }{ - "Matches": { - input: errors.NewAlreadyExists(schema.GroupResource{ - Group: "g", - Resource: "r", - }, "name"), - want: want{ - success: true, - err: nil, - }, +func TestAlreadyExistMatcher(t *testing.T) { + type want struct { + success bool + err error + } + + cases := map[string]struct { + input interface{} + want want + }{ + "Matches": { + input: errors.NewAlreadyExists(schema.GroupResource{ + Group: "g", + Resource: "r", + }, "name"), + want: want{ + success: true, + err: nil, }, - "Does not match": { - input: errors.NewNotFound(schema.GroupResource{ - Group: "g", - Resource: "r", - }, "name"), - want: want{ - success: false, - err: nil, - }, + }, + "Does not match": { + input: errors.NewNotFound(schema.GroupResource{ + Group: "g", + Resource: "r", + }, "name"), + want: want{ + success: false, + err: nil, }, - "Does not match nil": { - input: nil, - want: want{ - success: false, - err: nil, - }, + }, + "Does not match nil": { + input: nil, + want: want{ + success: false, + err: nil, }, + }, + } + matcher := AlreadyExistMatcher{} + for name, tc := range cases { + success, err := matcher.Match(tc.input) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.success, success) + if tc.want.err == nil { + assert.NoError(t, err) + } else { + assert.Error(t, tc.want.err, err) } - matcher := AlreadyExistMatcher{} - for name, tc := range cases { - success, err := matcher.Match(tc.input) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.success).Should(Equal(success)) - if tc.want.err == nil { - BeNil().Match(err) - } else { - Expect(tc.want.err).Should(Equal(err)) - } - } - - // Error messages - Expect("Expected\n : myerror\nto be already exist").Should( - Equal(matcher.FailureMessage("myerror"))) - Expect("Expected\n : myerror\nnot to be already exist").Should( - Equal(matcher.NegatedFailureMessage("myerror"))) - }) - - It("Test NotFoundMatcher", func() { - type want struct { - success bool - err error - } - - cases := map[string]struct { - input interface{} - want want - }{ - "Does not matche": { - input: errors.NewAlreadyExists(schema.GroupResource{ - Group: "g", - Resource: "r", - }, "name"), - want: want{ - success: false, - err: nil, - }, + } + + // Error messages + assert.Equal(t, "Expected\n : myerror\nto be already exist", matcher.FailureMessage("myerror")) + assert.Equal(t, "Expected\n : myerror\nnot to be already exist", matcher.NegatedFailureMessage("myerror")) +} + +func TestNotFoundMatcher(t *testing.T) { + type want struct { + success bool + err error + } + + cases := map[string]struct { + input interface{} + want want + }{ + "Does not matche": { + input: errors.NewAlreadyExists(schema.GroupResource{ + Group: "g", + Resource: "r", + }, "name"), + want: want{ + success: false, + err: nil, }, - "Matches": { - input: errors.NewNotFound(schema.GroupResource{ - Group: "g", - Resource: "r", - }, "name"), - want: want{ - success: true, - err: nil, - }, + }, + "Matches": { + input: errors.NewNotFound(schema.GroupResource{ + Group: "g", + Resource: "r", + }, "name"), + want: want{ + success: true, + err: nil, }, - "Does not match nil": { - input: nil, - want: want{ - success: false, - err: nil, - }, + }, + "Does not match nil": { + input: nil, + want: want{ + success: false, + err: nil, }, + }, + } + + matcher := NotFoundMatcher{} + for name, tc := range cases { + success, err := matcher.Match(tc.input) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.success, success) + if tc.want.err == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, tc.want.err, err) } - - matcher := NotFoundMatcher{} - for name, tc := range cases { - success, err := matcher.Match(tc.input) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.success).Should(Equal(success)) - if tc.want.err == nil { - BeNil().Match(err) - } else { - Expect(tc.want.err).Should(Equal(err)) - } - } - - // Error messages - Expect("Expected\n : myerror\nto be not found").Should( - Equal(matcher.FailureMessage("myerror"))) - Expect("Expected\n : myerror\nnot to be not found").Should( - Equal(matcher.NegatedFailureMessage("myerror"))) - }) - - It("Test ErrorMatcher", func() { - type input struct { - expected error - input error - } - - type want struct { - success bool - err error - failureMessage string - negatedFailureMessage string - } - - cases := map[string]struct { - input input - want want - }{ - "Matches": { - input: input{ - expected: fmt.Errorf("my error"), - input: fmt.Errorf("my error"), - }, - want: want{ - success: true, - err: nil, - failureMessage: "Expected\n : my error\nto equal\n : my error", - negatedFailureMessage: "Expected\n : my error\nnot to equal\n : my error", - }, + } + + // Error messages + assert.Equal(t, "Expected\n : myerror\nto be not found", matcher.FailureMessage("myerror")) + assert.Equal(t, "Expected\n : myerror\nnot to be not found", matcher.NegatedFailureMessage("myerror")) +} +func TestErrorMatcher(t *testing.T) { + type input struct { + expected error + input error + } + + type want struct { + success bool + err error + failureMessage string + negatedFailureMessage string + } + + cases := map[string]struct { + input input + want want + }{ + "Matches": { + input: input{ + expected: fmt.Errorf("my error"), + input: fmt.Errorf("my error"), }, - "Matches nil": { - input: input{ - expected: nil, - input: nil, - }, - want: want{ - success: true, - err: nil, - failureMessage: "Expected\n : nil\nto equal\n : nil", - negatedFailureMessage: "Expected\n : nil\nnot to equal\n : nil", - }, + want: want{ + success: true, + err: nil, + failureMessage: "Expected\n : my error\nto equal\n : my error", + negatedFailureMessage: "Expected\n : my error\nnot to equal\n : my error", }, - "Does not match": { - input: input{ - expected: fmt.Errorf("my error"), - input: fmt.Errorf("my other error"), - }, - want: want{ - success: false, - err: nil, - failureMessage: "Expected\n : my other error\nto equal\n : my error", - negatedFailureMessage: "Expected\n : my other error\nnot to equal\n : my error", - }, + }, + "Matches nil": { + input: input{ + expected: nil, + input: nil, }, - "Does not match nil": { - input: input{ - expected: fmt.Errorf("my error"), - input: nil, - }, - want: want{ - success: false, - err: nil, - failureMessage: "Expected\n : nil\nto equal\n : my error", - negatedFailureMessage: "Expected\n : nil\nnot to equal\n : my error", - }, + want: want{ + success: true, + err: nil, + failureMessage: "Expected\n : nil\nto equal\n : nil", + negatedFailureMessage: "Expected\n : nil\nnot to equal\n : nil", }, + }, + "Does not match": { + input: input{ + expected: fmt.Errorf("my error"), + input: fmt.Errorf("my other error"), + }, + want: want{ + success: false, + err: nil, + failureMessage: "Expected\n : my other error\nto equal\n : my error", + negatedFailureMessage: "Expected\n : my other error\nnot to equal\n : my error", + }, + }, + "Does not match nil": { + input: input{ + expected: fmt.Errorf("my error"), + input: nil, + }, + want: want{ + success: false, + err: nil, + failureMessage: "Expected\n : nil\nto equal\n : my error", + negatedFailureMessage: "Expected\n : nil\nnot to equal\n : my error", + }, + }, + } + for name, tc := range cases { + matcher := ErrorMatcher{ + ExpectedError: tc.input.expected, } - for name, tc := range cases { - matcher := ErrorMatcher{ - ExpectedError: tc.input.expected, - } - success, err := matcher.Match(tc.input.input) - By(fmt.Sprint("Running test: ", name)) - Expect(tc.want.success).Should(Equal(success)) - if tc.want.err == nil { - BeNil().Match(err) - } else { - Expect(tc.want.err).Should(Equal(err)) - } - - Expect(tc.want.failureMessage).Should(Equal(matcher.FailureMessage(tc.input.input))) - Expect(tc.want.negatedFailureMessage).Should(Equal(matcher.NegatedFailureMessage(tc.input.input))) + success, err := matcher.Match(tc.input.input) + t.Log(fmt.Sprint("Running test: ", name)) + assert.Equal(t, tc.want.success, success) + if tc.want.err == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, tc.want.err, err) } - }) -}) + + assert.Equal(t, tc.want.failureMessage, matcher.FailureMessage(tc.input.input)) + assert.Equal(t, tc.want.negatedFailureMessage, matcher.NegatedFailureMessage(tc.input.input)) + } +} diff --git a/pkg/oam/util/util_suite_test.go b/pkg/oam/util/util_suite_test.go deleted file mode 100644 index 37ce0095..00000000 --- a/pkg/oam/util/util_suite_test.go +++ /dev/null @@ -1,22 +0,0 @@ -package util_test - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -func TestUtil(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "OAM Util Suite") -} - -var _ = BeforeSuite(func(done Done) { - By("Bootstrapping OAM util test environment") - close(done) -}, 300) - -var _ = AfterSuite(func() { - By("Tearing down the OAM util test environment") -}) From 2cd7b9cef1b8c05ea9004ed4e044485319600376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Fri, 23 Oct 2020 10:47:38 +0800 Subject: [PATCH 2/4] fix related change for get crd interface change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 天元 --- build | 2 +- cmd/oam-kubernetes-runtime/main.go | 6 +- go.mod | 3 +- go.sum | 2 - .../applicationconfiguration.go | 16 +++- .../applicationconfiguration_test.go | 7 +- .../applicationconfiguration/apply.go | 7 +- .../applicationconfiguration/apply_test.go | 9 +- .../applicationconfiguration/render.go | 4 +- .../applicationconfiguration/render_test.go | 37 +++++--- .../manualscalertrait_controller.go | 20 ++++- pkg/oam/mock/mapper.go | 89 +++++++++++++++++++ pkg/webhook/v1alpha2/admit.go | 7 +- .../applicationconfiguration/handler_test.go | 5 +- .../applicationconfiguration/helper.go | 7 +- .../applicationconfiguration/helper_test.go | 5 +- .../validating_handler.go | 23 +++-- .../validating_handler_test.go | 4 +- test/e2e-test/suite_test.go | 2 +- 19 files changed, 211 insertions(+), 44 deletions(-) create mode 100644 pkg/oam/mock/mapper.go diff --git a/build b/build index 3c47eae1..e8fb77d6 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit 3c47eae15fc8ae3c3f2f00f11b551cb6c467610a +Subproject commit e8fb77d69aefc49dd2e9ead59da21bd719cacb78 diff --git a/cmd/oam-kubernetes-runtime/main.go b/cmd/oam-kubernetes-runtime/main.go index bd690fab..218d2f0e 100644 --- a/cmd/oam-kubernetes-runtime/main.go +++ b/cmd/oam-kubernetes-runtime/main.go @@ -88,7 +88,11 @@ func main() { if useWebhook { oamLog.Info("OAM webhook enabled, will serving at :" + strconv.Itoa(webhookPort)) - webhook.Add(mgr) + if err = webhook.Add(mgr); err != nil { + oamLog.Error(err, "unable to setup the webhook for core controller") + os.Exit(1) + } + } if err = appController.Setup(mgr, controllerArgs, logging.NewLogrLogger(oamLog)); err != nil { diff --git a/go.mod b/go.mod index 18c4621c..8207bb6d 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ 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/ghodss/yaml v1.0.0 github.com/go-logr/logr v0.1.0 github.com/google/go-cmp v0.4.0 @@ -15,7 +14,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.4.0 go.uber.org/zap v1.10.0 - golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect + golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 gopkg.in/natefinch/lumberjack.v2 v2.0.0 k8s.io/api v0.18.5 k8s.io/apiextensions-apiserver v0.18.2 diff --git a/go.sum b/go.sum index 2e9c4bb5..d293ee6d 100644 --- a/go.sum +++ b/go.sum @@ -108,8 +108,6 @@ github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= -github.com/gertd/go-pluralize v0.1.7 h1:RgvJTJ5W7olOoAks97BOwOlekBFsLEyh00W48Z6ZEZY= -github.com/gertd/go-pluralize v0.1.7/go.mod h1:O4eNeeIf91MHh1GJ2I47DNtaesm66NYvjYgAahcqSDQ= github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go index 0f4ac2d1..a37f88dd 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -18,17 +18,18 @@ package applicationconfiguration import ( "context" + "fmt" "strconv" "strings" "time" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" - "github.com/pkg/errors" + meta2 "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -41,6 +42,7 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/controller" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" ) const ( @@ -79,6 +81,10 @@ const ( // Setup adds a controller that reconciles ApplicationConfigurations. func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error { + mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + if err != nil { + return fmt.Errorf("create discovery mapper fail %v", err) + } name := "oam/" + strings.ToLower(v1alpha2.ApplicationConfigurationGroupKind) return ctrl.NewControllerManagedBy(mgr). @@ -89,7 +95,7 @@ func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error { Logger: l, RevisionLimit: args.RevisionLimit, }). - Complete(NewReconciler(mgr, + Complete(NewReconciler(mgr, mapper, WithLogger(l.WithValues("controller", name)), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))))) } @@ -164,12 +170,13 @@ func WithPosthook(name string, hook ControllerHooks) ReconcilerOption { // NewReconciler returns an OAMApplicationReconciler that reconciles ApplicationConfigurations // by rendering and instantiating their Components and Traits. -func NewReconciler(m ctrl.Manager, o ...ReconcilerOption) *OAMApplicationReconciler { +func NewReconciler(m ctrl.Manager, mapper meta2.RESTMapper, o ...ReconcilerOption) *OAMApplicationReconciler { r := &OAMApplicationReconciler{ client: m.GetClient(), scheme: m.GetScheme(), components: &components{ client: m.GetClient(), + mapper: mapper, params: ParameterResolveFn(resolve), workload: ResourceRenderFn(renderWorkload), trait: ResourceRenderFn(renderTrait), @@ -177,6 +184,7 @@ func NewReconciler(m ctrl.Manager, o ...ReconcilerOption) *OAMApplicationReconci workloads: &workloads{ client: resource.NewAPIPatchingApplicator(m.GetClient()), rawClient: m.GetClient(), + mapper: mapper, }, gc: GarbageCollectorFn(eligible), log: logging.NewNopLogger(), diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go index b7461f8d..e2505658 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go @@ -659,7 +659,7 @@ func TestReconciler(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := NewReconciler(tc.args.m, tc.args.o...) + r := NewReconciler(tc.args.m, nil, tc.args.o...) got, err := r.Reconcile(reconcile.Request{}) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -887,6 +887,8 @@ func TestDependency(t *testing.T) { t.Fatal(err) } + mapper := mock.NewMockMapper() + type args struct { components []v1alpha2.ApplicationConfigurationComponent wl *unstructured.Unstructured @@ -1297,6 +1299,7 @@ func TestDependency(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { c := components{ + mapper: mapper, client: &test.MockClient{ MockGet: test.MockGetFn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { if obj.GetObjectKind().GroupVersionKind().Kind == "Workload" { @@ -1623,7 +1626,7 @@ func TestUpdateStatus(t *testing.T) { }, } - r := NewReconciler(m) + r := NewReconciler(m, nil) ac := &v1alpha2.ApplicationConfiguration{} err := r.client.Get(context.Background(), types.NamespacedName{Name: "example-appconfig"}, ac) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/apply.go b/pkg/controller/v1alpha2/applicationconfiguration/apply.go index 39e53f9c..c0e17773 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/apply.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/apply.go @@ -19,6 +19,8 @@ package applicationconfiguration import ( "context" + meta2 "k8s.io/apimachinery/pkg/api/meta" + runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/meta" @@ -76,6 +78,7 @@ func (fn WorkloadApplyFns) Finalize(ctx context.Context, ac *v1alpha2.Applicatio type workloads struct { client resource.Applicator rawClient client.Client + mapper meta2.RESTMapper } func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus, w []Workload, ao ...resource.ApplyOption) error { @@ -183,7 +186,7 @@ func findDereferencedScopes(statusScopes []v1alpha2.WorkloadScope, scopes []unst func (a *workloads) applyScope(ctx context.Context, wl Workload, s unstructured.Unstructured, workloadRef runtimev1alpha1.TypedReference) error { // get ScopeDefinition - scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, &s) + scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &s) if err != nil { return errors.Wrapf(err, errFmtGetScopeDefinition, s.GetAPIVersion(), s.GetKind(), s.GetName()) } @@ -237,7 +240,7 @@ func (a *workloads) applyScopeRemoval(ctx context.Context, namespace string, wr return errors.Wrapf(err, errFmtApplyScope, s.Reference.APIVersion, s.Reference.Kind, s.Reference.Name) } - scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, &scopeObject) + scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &scopeObject) if err != nil { return errors.Wrapf(err, errFmtGetScopeDefinition, scopeObject.GetAPIVersion(), scopeObject.GetKind(), scopeObject.GetName()) } diff --git a/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go b/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go index 8da1a946..129bf9c5 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go @@ -21,6 +21,8 @@ import ( "fmt" "testing" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" + "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/resource" @@ -318,7 +320,9 @@ func TestApplyWorkloads(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - w := workloads{client: tc.client, rawClient: tc.rawClient} + mapper := mock.NewMockMapper() + + w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper} err := w.Apply(tc.args.ctx, tc.args.ws, tc.args.w) if diff := cmp.Diff(tc.want, err, test.EquateErrors()); diff != "" { @@ -463,7 +467,8 @@ func TestFinalizeWorkloadScopes(t *testing.T) { for _, tc := range cases { t.Run(tc.caseName, func(t *testing.T) { acTest := ac - w := workloads{client: tc.client, rawClient: tc.rawClient} + mapper := mock.NewMockMapper() + w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper} err := w.Finalize(ctx, &acTest) if diff := cmp.Diff(tc.wantErr, err, test.EquateErrors()); diff != "" { diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 75a17ab7..7b1c3122 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -27,6 +27,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -83,6 +84,7 @@ var _ ComponentRenderer = &components{} type components struct { client client.Reader + mapper meta.RESTMapper params ParameterResolver workload ResourceRenderer trait ResourceRenderer @@ -211,7 +213,7 @@ func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait setTraitProperties(t, traitName, ac.GetNamespace(), ref) - traitDef, err := util.FetchTraitDefinition(ctx, r.client, t) + traitDef, err := util.FetchTraitDefinition(ctx, r.client, r.mapper, t) if err != nil { return nil, nil, errors.Wrapf(err, errFmtGetTraitDefinition, t.GetAPIVersion(), t.GetKind(), t.GetName()) } diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 1bac0830..e2212e4d 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -21,6 +21,11 @@ import ( "encoding/json" "testing" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" + "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" @@ -498,7 +503,7 @@ func TestRenderComponents(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := &components{tc.fields.client, tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, mock.NewMockMapper(), tc.fields.params, tc.fields.workload, tc.fields.trait} got, _, err := r.Render(tc.args.ctx, tc.args.ac) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, diff) @@ -844,7 +849,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := &components{tc.fields.client, tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, mock.NewMockMapper(), 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].Object.GetName() != util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy()) { t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, "Trait name is NOT"+ @@ -855,31 +860,43 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) { } func TestGetDefinitionName(t *testing.T) { + tests := map[string]struct { - u *unstructured.Unstructured - exp string - reason string + u *unstructured.Unstructured + exp string + reason string + resource string }{ "native resource": { u: &unstructured.Unstructured{Object: map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", }}, - exp: "deployments.apps", - reason: "native resource match", + exp: "deployments.apps", + reason: "native resource match", + resource: "deployments", }, "extended resource": { u: &unstructured.Unstructured{Object: map[string]interface{}{ "apiVersion": "extend.oam.dev/v1alpha2", "kind": "SimpleRolloutTrait", }}, - exp: "simplerollouttraits.extend.oam.dev", - reason: "extend resource match", + exp: "simplerollouttraits.extend.oam.dev", + reason: "extend resource match", + resource: "simplerollouttraits", }, } for name, ti := range tests { t.Run(name, func(t *testing.T) { - got := util.GetDefinitionName(ti.u, "") + mapper := func(resource string) *mock.Mapper { + m := mock.NewMockMapper() + m.MockRESTMapping = func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + return &meta.RESTMapping{Resource: schema.GroupVersionResource{Resource: resource}}, nil + } + return m + }(ti.resource) + got, err := util.GetDefinitionName(mapper, ti.u, "") + assert.NoError(t, err) if got != ti.exp { t.Errorf("%s getCRDName want %s got %s ", ti.reason, ti.exp, got) } diff --git a/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go b/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go index 361f494b..e8ba7858 100644 --- a/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go +++ b/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go @@ -20,6 +20,9 @@ import ( "fmt" "strings" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/logging" @@ -51,9 +54,14 @@ const ( // Setup adds a controller that reconciles ContainerizedWorkload. func Setup(mgr ctrl.Manager, args controller.Args, log logging.Logger) error { + mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + if err != nil { + return err + } reconciler := Reconciler{ Client: mgr.GetClient(), DiscoveryClient: *discovery.NewDiscoveryClientForConfigOrDie(mgr.GetConfig()), + mapper: mapper, log: ctrl.Log.WithName("ManualScalarTrait"), record: event.NewAPIRecorder(mgr.GetEventRecorderFor("ManualScalarTrait")), Scheme: mgr.GetScheme(), @@ -65,6 +73,7 @@ func Setup(mgr ctrl.Manager, args controller.Args, log logging.Logger) error { type Reconciler struct { client.Client discovery.DiscoveryClient + mapper meta.RESTMapper log logr.Logger record event.Recorder Scheme *runtime.Scheme @@ -105,7 +114,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // Fetch the child resources list from the corresponding workload - resources, err := util.FetchWorkloadChildResources(ctx, mLog, r, workload) + resources, err := util.FetchWorkloadChildResources(ctx, mLog, r, r.mapper, workload) if err != nil { mLog.Error(err, "Error while fetching the workload child resources", "workload", workload.UnstructuredContent()) r.record.Event(eventObj, event.Warning(util.ErrFetchChildResources, err)) @@ -194,11 +203,14 @@ func (r *Reconciler) scaleResources(ctx context.Context, mLog logr.Logger, func locateReplicaField(document openapi.Resources, res *unstructured.Unstructured) bool { // this is the most common path for replicas fields replicaFieldPath := []string{"spec", "replicas"} - g, v := util.APIVersion2GroupVersion(res.GetAPIVersion()) + gv, err := schema.ParseGroupVersion(res.GetAPIVersion()) + if err != nil { + return false + } // we look up the resource schema definition by its GVK schema := document.LookupResource(schema.GroupVersionKind{ - Group: g, - Version: v, + Group: gv.Group, + Version: gv.Version, Kind: res.GetKind(), }) // we try to see if there is a spec.replicas fields in its definition diff --git a/pkg/oam/mock/mapper.go b/pkg/oam/mock/mapper.go new file mode 100644 index 00000000..a4392152 --- /dev/null +++ b/pkg/oam/mock/mapper.go @@ -0,0 +1,89 @@ +package mock + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var _ meta.RESTMapper = &Mapper{} + +// nolint +type KindFor func(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) + +// nolint +type KindsFor func(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) + +// nolint +type ResourceFor func(input schema.GroupVersionResource) (schema.GroupVersionResource, error) + +// nolint +type ResourcesFor func(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) + +// nolint +type RESTMapping func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) + +// nolint +type RESTMappings func(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) + +// nolint +type ResourceSingularizer func(resource string) (singular string, err error) + +// NewMockMapper for mock +func NewMockMapper() *Mapper { + return &Mapper{ + MockRESTMapping: NewMockRESTMapping(""), + } +} + +// NewMockRESTMapping for mock +func NewMockRESTMapping(resource string) RESTMapping { + return func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + return &meta.RESTMapping{Resource: schema.GroupVersionResource{Resource: resource}}, nil + } +} + +// Mapper for mock +type Mapper struct { + MockKindFor KindFor + MockKindsFor KindsFor + MockResourceFor ResourceFor + MockResourcesFor ResourcesFor + MockRESTMapping RESTMapping + MockRESTMappings RESTMappings + MockResourceSingularizer ResourceSingularizer +} + +// KindFor for mock +func (m *Mapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { + return m.MockKindFor(resource) +} + +// KindsFor for mock +func (m *Mapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { + return m.MockKindsFor(resource) +} + +// ResourceFor for mock +func (m *Mapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { + return m.MockResourceFor(input) +} + +// ResourcesFor for mock +func (m *Mapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { + return m.MockResourcesFor(input) +} + +// RESTMapping for mock +func (m *Mapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + return m.MockRESTMapping(gk, versions...) +} + +// RESTMappings for mock +func (m *Mapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { + return m.MockRESTMappings(gk, versions...) +} + +// ResourceSingularizer for mock +func (m *Mapper) ResourceSingularizer(resource string) (singular string, err error) { + return m.MockResourceSingularizer(resource) +} diff --git a/pkg/webhook/v1alpha2/admit.go b/pkg/webhook/v1alpha2/admit.go index c6925030..2c21f563 100644 --- a/pkg/webhook/v1alpha2/admit.go +++ b/pkg/webhook/v1alpha2/admit.go @@ -8,9 +8,12 @@ import ( ) // Add will be called in main and register all validation handlers -func Add(mgr manager.Manager) { - applicationconfiguration.RegisterValidatingHandler(mgr) +func Add(mgr manager.Manager) error { + if err := applicationconfiguration.RegisterValidatingHandler(mgr); err != nil { + return err + } applicationconfiguration.RegisterMutatingHandler(mgr) component.RegisterMutatingHandler(mgr) component.RegisterValidatingHandler(mgr) + return nil } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go index f87f0953..258fe24d 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -189,7 +191,8 @@ var _ = Describe("ApplicationConfiguration Admission controller Test", func() { }) It("Test validating handler", func() { - var handler admission.Handler = &ValidatingHandler{} + mapper := mock.NewMockMapper() + var handler admission.Handler = &ValidatingHandler{Mapper: mapper} decoderInjector := handler.(admission.DecoderInjector) decoderInjector.InjectDecoder(decoder) By("Creating valid trait") diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go index b924f70e..193ba385 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go @@ -5,6 +5,8 @@ import ( "encoding/json" "strings" + "k8s.io/apimachinery/pkg/api/meta" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" @@ -19,7 +21,8 @@ const ( ) // checkComponentVersionEnabled check whethter a component is versioning mechanism enabled -func checkComponentVersionEnabled(ctx context.Context, client client.Reader, acc *v1alpha2.ApplicationConfigurationComponent) (bool, error) { +func checkComponentVersionEnabled(ctx context.Context, client client.Reader, mapper meta.RESTMapper, + acc *v1alpha2.ApplicationConfigurationComponent) (bool, error) { if acc.RevisionName != "" { return true, nil } @@ -28,7 +31,7 @@ func checkComponentVersionEnabled(ctx context.Context, client client.Reader, acc if err := json.Unmarshal(ct.Trait.Raw, ut); err != nil { return false, errors.Wrap(err, errUnmarshalTrait) } - td, err := util.FetchTraitDefinition(ctx, client, ut) + td, err := util.FetchTraitDefinition(ctx, client, mapper, ut) if err != nil { return false, errors.Wrapf(err, errFmtGetTraitDefinition, ut.GetAPIVersion(), ut.GetKind(), ut.GetName()) } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go index 2043d52e..98b67dc4 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go @@ -5,6 +5,8 @@ import ( "fmt" "testing" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/stretchr/testify/assert" @@ -98,11 +100,12 @@ func TestCheckComponentVersionEnabled(t *testing.T) { result: false, }, } + mapper := mock.NewMockMapper() for _, tv := range tests { func(t *testing.T) { mockClient.MockGet = tv.mockGetFun - result, _ := checkComponentVersionEnabled(ctx, mockClient, &tv.acc) + result, _ := checkComponentVersionEnabled(ctx, mockClient, mapper, &tv.acc) assert.Equal(t, tv.result, result, fmt.Sprintf("Test case: %q", tv.caseName)) }(t) } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go index 73970f86..b7845f30 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go @@ -6,6 +6,10 @@ import ( "fmt" "net/http" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/util/validation/field" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" @@ -39,6 +43,7 @@ var appConfigResource = v1alpha2.SchemeGroupVersion.WithResource("applicationcon // ValidatingHandler handles CloneSet type ValidatingHandler struct { Client client.Client + Mapper meta.RESTMapper // Decoder decodes objects Decoder *admission.Decoder @@ -74,7 +79,7 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a if pass, reason := checkRevisionName(obj); !pass { return admission.ValidationResponse(false, reason) } - if pass, reason := checkWorkloadNameForVersioning(ctx, h.Client, obj); !pass { + if pass, reason := checkWorkloadNameForVersioning(ctx, h.Client, h.Mapper, obj); !pass { return admission.ValidationResponse(false, reason) } // TODO(wonderflow): Add more validation logic here. @@ -126,10 +131,11 @@ func checkRevisionName(appConfig *v1alpha2.ApplicationConfiguration) (bool, stri } // checkWorkloadNameForVersioning check whether versioning-enabled component workload name is empty -func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { +func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, mapper meta.RESTMapper, + appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { for _, v := range appConfig.Spec.Components { acc := v - vEnabled, err := checkComponentVersionEnabled(ctx, client, &acc) + vEnabled, err := checkComponentVersionEnabled(ctx, client, mapper, &acc) if err != nil { return false, fmt.Sprintf(errFmtCheckWorkloadName, err.Error()) } @@ -176,7 +182,14 @@ func (h *ValidatingHandler) InjectDecoder(d *admission.Decoder) error { } // RegisterValidatingHandler will register application configuration validation to webhook -func RegisterValidatingHandler(mgr manager.Manager) { +func RegisterValidatingHandler(mgr manager.Manager) error { server := mgr.GetWebhookServer() - server.Register("/validating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &ValidatingHandler{}}) + mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + if err != nil { + return err + } + server.Register("/validating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &ValidatingHandler{ + Mapper: mapper, + }}) + return nil } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go index e51d91f0..930d38ad 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go @@ -152,6 +152,8 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { Name: tName, }}) + mapper := mock.NewMockMapper() + tests := []struct { caseName string appConfig v1alpha2.ApplicationConfiguration @@ -376,7 +378,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { for _, tc := range tests { func(t *testing.T) { mockClient.MockGet = tc.mockGetFunc - result, reason := checkWorkloadNameForVersioning(ctx, mockClient, &tc.appConfig) + result, reason := checkWorkloadNameForVersioning(ctx, mockClient, mapper, &tc.appConfig) assert.Equal(t, tc.expectResult, result, fmt.Sprintf("Test case: %q", tc.caseName)) assert.Equal(t, tc.expectReason, reason, fmt.Sprintf("Test case: %q", tc.caseName)) }(t) diff --git a/test/e2e-test/suite_test.go b/test/e2e-test/suite_test.go index 38bb772c..715793cc 100644 --- a/test/e2e-test/suite_test.go +++ b/test/e2e-test/suite_test.go @@ -314,7 +314,7 @@ var _ = AfterSuite(func() { ObjectMeta: metav1.ObjectMeta{ Name: "foo.example.com", Labels: map[string]string{"crd": "dependency"}, - }, + } } Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) From 7b827e54b662cb90d3b8f34174bbc88db105a9fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Fri, 23 Oct 2020 15:35:48 +0800 Subject: [PATCH 3/4] add mapper package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 天元 --- pkg/oam/discoverymapper/mapper.go | 70 ++++++++++++++ pkg/oam/discoverymapper/suit_test.go | 134 +++++++++++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 pkg/oam/discoverymapper/mapper.go create mode 100644 pkg/oam/discoverymapper/suit_test.go diff --git a/pkg/oam/discoverymapper/mapper.go b/pkg/oam/discoverymapper/mapper.go new file mode 100644 index 00000000..abe2de3f --- /dev/null +++ b/pkg/oam/discoverymapper/mapper.go @@ -0,0 +1,70 @@ +package discoverymapper + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" +) + +type DiscoveryMapper interface { + GetMapper() (meta.RESTMapper, error) + Refresh() (meta.RESTMapper, error) + RESTMapping(gk schema.GroupKind, version ...string) (*meta.RESTMapping, error) +} + +var _ DiscoveryMapper = &DefaultDiscoveryMapper{} + +// DefaultDiscoveryMapper is a K8s resource mapper for discovery, it will cache the result +type DefaultDiscoveryMapper struct { + dc *discovery.DiscoveryClient + mapper meta.RESTMapper +} + +// New will create a new DefaultDiscoveryMapper by giving a K8s rest config +func New(c *rest.Config) (DiscoveryMapper, error) { + dc, err := discovery.NewDiscoveryClientForConfig(c) + if err != nil { + return nil, err + } + return &DefaultDiscoveryMapper{ + dc: dc, + }, nil +} + +// GetMapper will get the cached restmapper, if nil, it will create one by refresh +// Prefer lazy discovery, because resources created after refresh can not be found +func (d *DefaultDiscoveryMapper) GetMapper() (meta.RESTMapper, error) { + if d.mapper == nil { + return d.Refresh() + } + return d.mapper, nil +} + +// Refresh will re-create the mapper by getting the new resource from K8s API by using discovery client +func (d *DefaultDiscoveryMapper) Refresh() (meta.RESTMapper, error) { + gr, err := restmapper.GetAPIGroupResources(d.dc) + if err != nil { + return nil, err + } + d.mapper = restmapper.NewDiscoveryRESTMapper(gr) + return d.mapper, nil +} + +func (d *DefaultDiscoveryMapper) RESTMapping(gk schema.GroupKind, version ...string) (*meta.RESTMapping, error) { + mapper, err := d.GetMapper() + if err != nil { + return nil, err + } + mapping, err := mapper.RESTMapping(gk, version...) + if meta.IsNoMatchError(err) { + // if no kind match err, refresh and try once more. + mapper, err = d.Refresh() + if err != nil { + return nil, err + } + mapping, err = mapper.RESTMapping(gk, version...) + } + return mapping, err +} diff --git a/pkg/oam/discoverymapper/suit_test.go b/pkg/oam/discoverymapper/suit_test.go new file mode 100644 index 00000000..c9e5f82d --- /dev/null +++ b/pkg/oam/discoverymapper/suit_test.go @@ -0,0 +1,134 @@ +package discoverymapper + +import ( + "context" + "testing" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/envtest/printer" + // +kubebuilder:scaffold:imports +) + +// These tests use Ginkgo (BDD-style Go testing framework). Refer to +// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. + +var cfg *rest.Config +var k8sClient client.Client +var testEnv *envtest.Environment +var controllerDone chan struct{} +var routeNS corev1.Namespace +var scheme = runtime.NewScheme() + +func TestMapper(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecsWithDefaultAndCustomReporters(t, + "Test Mapper Suite", + []Reporter{printer.NewlineReporter{}}) +} + +var _ = BeforeSuite(func(done Done) { + By("Bootstrapping test environment") + testEnv = &envtest.Environment{} + var err error + cfg, err = testEnv.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(cfg).ToNot(BeNil()) + + Expect(crdv1.AddToScheme(scheme)).Should(BeNil()) + // +kubebuilder:scaffold:scheme + By("Create the k8s client") + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient).ToNot(BeNil()) + + close(done) +}, 60) + +var _ = AfterSuite(func() { + By("Tearing down the test environment") + err := testEnv.Stop() + Expect(err).ToNot(HaveOccurred()) +}) + +var _ = Describe("Mapper discovery resources", func() { + + It("discovery built-in k8s resource", func() { + dism, err := New(cfg) + Expect(err).Should(BeNil()) + mapper, err := dism.GetMapper() + Expect(err).Should(BeNil()) + mapping, err := mapper.RESTMapping(schema.GroupKind{"apps", "Deployment"}, "v1") + Expect(err).Should(BeNil()) + Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ + Group: "apps", + Version: "v1", + Resource: "deployments", + })) + }) + + It("discovery CRD", func() { + + By("Check built-in resource") + dism, err := New(cfg) + Expect(err).Should(BeNil()) + mapper, err := dism.GetMapper() + Expect(err).Should(BeNil()) + var mapping *meta.RESTMapping + mapping, err = mapper.RESTMapping(schema.GroupKind{"", "Pod"}, "v1") + Expect(err).Should(BeNil()) + Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "pods", + })) + + By("CRD should be discovered after refresh") + crd := crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foos.example.com", + Labels: map[string]string{"crd": "dependency"}, + }, + Spec: crdv1.CustomResourceDefinitionSpec{ + Group: "example.com", + Names: crdv1.CustomResourceDefinitionNames{ + Kind: "Foo", + Plural: "foos", + }, + Versions: []crdv1.CustomResourceDefinitionVersion{{ + Name: "v1", + Served: true, + Storage: true, + Schema: &crdv1.CustomResourceValidation{ + OpenAPIV3Schema: &crdv1.JSONSchemaProps{ + Type: "object", + }}, + }}, + Scope: crdv1.NamespaceScoped, + }, + } + Expect(k8sClient.Create(context.Background(), &crd)).Should(BeNil()) + + Eventually(func() error { + mapping, err = dism.RESTMapping(schema.GroupKind{"example.com", "Foo"}, "v1") + return err + }, time.Second*5, time.Millisecond*500).Should(BeNil()) + Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ + Group: "example.com", + Version: "v1", + Resource: "foos", + })) + }) +}) From 689bc39125c763c03374660243ce8c857b735473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Fri, 23 Oct 2020 17:05:56 +0800 Subject: [PATCH 4/4] refactor and fix test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 天元 --- .github/workflows/ci.yml | 3 + build | 2 +- go.mod | 2 +- .../applicationconfiguration.go | 17 +- .../applicationconfiguration_test.go | 4 +- .../applicationconfiguration/apply.go | 9 +- .../applicationconfiguration/apply_test.go | 11 +- .../dependency_test.go | 501 +++++++++++++++ .../applicationconfiguration/render.go | 6 +- .../applicationconfiguration/render_test.go | 22 +- .../applicationconfiguration/suite_test.go | 192 ++++++ .../core/scopes/healthscope/healthscope.go | 3 +- .../manualscalertrait_controller.go | 12 +- pkg/oam/discoverymapper/mapper.go | 2 + pkg/oam/discoverymapper/suit_test.go | 9 +- pkg/oam/mock/mapper.go | 78 +-- pkg/oam/util/helper.go | 29 +- pkg/oam/util/helper_test.go | 10 +- .../applicationconfiguration/handler_test.go | 5 +- .../applicationconfiguration/helper.go | 7 +- .../applicationconfiguration/helper_test.go | 5 +- .../validating_handler.go | 19 +- .../validating_handler_test.go | 2 +- test/e2e-test/appconfig_dependency_test.go | 592 ------------------ test/e2e-test/appconfig_finalizer_test.go | 2 +- test/e2e-test/component_version_test.go | 12 +- test/e2e-test/containerized_workload_test.go | 2 +- test/e2e-test/health_scope_test.go | 2 +- test/e2e-test/kubernetes_workload_test.go | 2 +- test/e2e-test/suite_test.go | 77 +-- 30 files changed, 810 insertions(+), 829 deletions(-) create mode 100644 pkg/controller/v1alpha2/applicationconfiguration/dependency_test.go create mode 100644 pkg/controller/v1alpha2/applicationconfiguration/suite_test.go delete mode 100644 test/e2e-test/appconfig_dependency_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ae4235e..e6cfceac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,6 +120,9 @@ jobs: with: go-version: ${{ env.GO_VERSION }} + - name: install Kubebuilder + uses: RyanSiu1995/kubebuilder-action@v1 + - name: Cache Go Dependencies uses: actions/cache@v2 with: diff --git a/build b/build index e8fb77d6..3c47eae1 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit e8fb77d69aefc49dd2e9ead59da21bd719cacb78 +Subproject commit 3c47eae15fc8ae3c3f2f00f11b551cb6c467610a diff --git a/go.mod b/go.mod index 8207bb6d..19624fd9 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.4.0 go.uber.org/zap v1.10.0 - golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 + golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect gopkg.in/natefinch/lumberjack.v2 v2.0.0 k8s.io/api v0.18.5 k8s.io/apiextensions-apiserver v0.18.2 diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go index a37f88dd..721e789a 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -24,12 +24,10 @@ import ( "time" "github.com/pkg/errors" - meta2 "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -43,6 +41,7 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/controller" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" ) const ( @@ -81,9 +80,9 @@ const ( // Setup adds a controller that reconciles ApplicationConfigurations. func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error { - mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + dm, err := discoverymapper.New(mgr.GetConfig()) if err != nil { - return fmt.Errorf("create discovery mapper fail %v", err) + return fmt.Errorf("create discovery dm fail %v", err) } name := "oam/" + strings.ToLower(v1alpha2.ApplicationConfigurationGroupKind) @@ -95,7 +94,7 @@ func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error { Logger: l, RevisionLimit: args.RevisionLimit, }). - Complete(NewReconciler(mgr, mapper, + Complete(NewReconciler(mgr, dm, WithLogger(l.WithValues("controller", name)), WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))))) } @@ -170,13 +169,13 @@ func WithPosthook(name string, hook ControllerHooks) ReconcilerOption { // NewReconciler returns an OAMApplicationReconciler that reconciles ApplicationConfigurations // by rendering and instantiating their Components and Traits. -func NewReconciler(m ctrl.Manager, mapper meta2.RESTMapper, o ...ReconcilerOption) *OAMApplicationReconciler { +func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, o ...ReconcilerOption) *OAMApplicationReconciler { r := &OAMApplicationReconciler{ client: m.GetClient(), scheme: m.GetScheme(), components: &components{ client: m.GetClient(), - mapper: mapper, + dm: dm, params: ParameterResolveFn(resolve), workload: ResourceRenderFn(renderWorkload), trait: ResourceRenderFn(renderTrait), @@ -184,7 +183,7 @@ func NewReconciler(m ctrl.Manager, mapper meta2.RESTMapper, o ...ReconcilerOptio workloads: &workloads{ client: resource.NewAPIPatchingApplicator(m.GetClient()), rawClient: m.GetClient(), - mapper: mapper, + dm: dm, }, gc: GarbageCollectorFn(eligible), log: logging.NewNopLogger(), @@ -269,7 +268,7 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco workloads, depStatus, err := r.components.Render(ctx, ac) if err != nil { - log.Debug("Cannot render components", "error", err, "requeue-after", time.Now().Add(shortWait)) + log.Info("Cannot render components", "error", err, "requeue-after", time.Now().Add(shortWait)) r.record.Event(ac, event.Warning(reasonCannotRenderComponents, err)) ac.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errRenderComponents))) return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, ac), errUpdateAppConfigStatus) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go index e2505658..721ee254 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go @@ -887,7 +887,7 @@ func TestDependency(t *testing.T) { t.Fatal(err) } - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() type args struct { components []v1alpha2.ApplicationConfigurationComponent @@ -1299,7 +1299,7 @@ func TestDependency(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { c := components{ - mapper: mapper, + dm: mapper, client: &test.MockClient{ MockGet: test.MockGetFn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { if obj.GetObjectKind().GroupVersionKind().Kind == "Workload" { diff --git a/pkg/controller/v1alpha2/applicationconfiguration/apply.go b/pkg/controller/v1alpha2/applicationconfiguration/apply.go index c0e17773..d119487f 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/apply.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/apply.go @@ -19,8 +19,6 @@ package applicationconfiguration import ( "context" - meta2 "k8s.io/apimachinery/pkg/api/meta" - runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/meta" @@ -32,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -78,7 +77,7 @@ func (fn WorkloadApplyFns) Finalize(ctx context.Context, ac *v1alpha2.Applicatio type workloads struct { client resource.Applicator rawClient client.Client - mapper meta2.RESTMapper + dm discoverymapper.DiscoveryMapper } func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus, w []Workload, ao ...resource.ApplyOption) error { @@ -186,7 +185,7 @@ func findDereferencedScopes(statusScopes []v1alpha2.WorkloadScope, scopes []unst func (a *workloads) applyScope(ctx context.Context, wl Workload, s unstructured.Unstructured, workloadRef runtimev1alpha1.TypedReference) error { // get ScopeDefinition - scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &s) + scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.dm, &s) if err != nil { return errors.Wrapf(err, errFmtGetScopeDefinition, s.GetAPIVersion(), s.GetKind(), s.GetName()) } @@ -240,7 +239,7 @@ func (a *workloads) applyScopeRemoval(ctx context.Context, namespace string, wr return errors.Wrapf(err, errFmtApplyScope, s.Reference.APIVersion, s.Reference.Kind, s.Reference.Name) } - scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &scopeObject) + scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.dm, &scopeObject) if err != nil { return errors.Wrapf(err, errFmtGetScopeDefinition, scopeObject.GetAPIVersion(), scopeObject.GetKind(), scopeObject.GetName()) } diff --git a/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go b/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go index 129bf9c5..90ac3a55 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/apply_test.go @@ -21,8 +21,6 @@ import ( "fmt" "testing" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" - "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/resource" @@ -36,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -320,9 +319,9 @@ func TestApplyWorkloads(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() - w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper} + w := workloads{client: tc.client, rawClient: tc.rawClient, dm: mapper} err := w.Apply(tc.args.ctx, tc.args.ws, tc.args.w) if diff := cmp.Diff(tc.want, err, test.EquateErrors()); diff != "" { @@ -467,8 +466,8 @@ func TestFinalizeWorkloadScopes(t *testing.T) { for _, tc := range cases { t.Run(tc.caseName, func(t *testing.T) { acTest := ac - mapper := mock.NewMockMapper() - w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper} + mapper := mock.NewMockDiscoveryMapper() + w := workloads{client: tc.client, rawClient: tc.rawClient, dm: mapper} err := w.Finalize(ctx, &acTest) if diff := cmp.Diff(tc.wantErr, err, test.EquateErrors()); diff != "" { diff --git a/pkg/controller/v1alpha2/applicationconfiguration/dependency_test.go b/pkg/controller/v1alpha2/applicationconfiguration/dependency_test.go new file mode 100644 index 00000000..ec509327 --- /dev/null +++ b/pkg/controller/v1alpha2/applicationconfiguration/dependency_test.go @@ -0,0 +1,501 @@ +package applicationconfiguration + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" +) + +var _ = Describe("Resource Dependency in an ApplicationConfiguration", func() { + ctx := context.Background() + namespace := "appconfig-dependency-test" + var ns corev1.Namespace + tempFoo := &unstructured.Unstructured{} + tempFoo.SetAPIVersion("example.com/v1") + tempFoo.SetKind("Foo") + tempFoo.SetNamespace(namespace) + outName := "data-output" + out := tempFoo.DeepCopy() + out.SetName(outName) + inName := "data-input" + in := tempFoo.DeepCopy() + in.SetName(inName) + componentOutName := "component-out" + componentInName := "component-in" + + BeforeEach(func() { + ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + Eventually( + func() error { + return k8sClient.Create(ctx, &ns) + }, + time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + By(" Create two components, one for data output and one for data input") + compOut := v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentOutName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{ + Workload: runtime.RawExtension{ + Object: out, + }, + }, + } + Expect(k8sClient.Create(ctx, &compOut)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + compIn := v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentInName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{ + Workload: runtime.RawExtension{ + Object: in, + }, + }, + } + Expect(k8sClient.Create(ctx, &compIn)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + }) + AfterEach(func() { + logf.Log.Info("Clean up resources") + // delete the namespace with all its resources + Expect(k8sClient.Delete(ctx, in)).Should(BeNil()) + Expect(k8sClient.Delete(ctx, out)).Should(BeNil()) + }) + + // common function for verification + verify := func(appConfigName, reason string) { + + appconfigKey := client.ObjectKey{ + Name: appConfigName, + Namespace: namespace, + } + + // Verification before satisfying dependency + By("Checking that resource which accepts data isn't created yet") + inFooKey := client.ObjectKey{ + Name: inName, + Namespace: namespace, + } + inFoo := tempFoo.DeepCopy() + By(fmt.Sprintf("Checking on resource that inputs data Key: %s", inFooKey)) + Expect(k8sClient.Get(ctx, inFooKey, inFoo)).Should(&util.NotFoundMatcher{}) + + By("Reconcile") + req := reconcile.Request{NamespacedName: appconfigKey} + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + outFooKey := client.ObjectKey{ + Name: outName, + Namespace: namespace, + } + outFoo := tempFoo.DeepCopy() + By(fmt.Sprintf("Checking that resource which provides(output) data is created, Key %s", outFoo)) + Eventually(func() error { + err := k8sClient.Get(ctx, outFooKey, outFoo) + if err != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return err + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + By("Verify the appconfig's dependency is unsatisfied, waiting for the outside controller to satisfy the requirement") + appconfig := &v1alpha2.ApplicationConfiguration{} + depStatus := v1alpha2.DependencyStatus{ + Unsatisfied: []v1alpha2.UnstaifiedDependency{{ + Reason: reason, + From: v1alpha2.DependencyFromObject{ + TypedReference: v1alpha1.TypedReference{ + APIVersion: tempFoo.GetAPIVersion(), + Name: outName, + Kind: tempFoo.GetKind(), + }, + FieldPath: "status.key", + }, + To: v1alpha2.DependencyToObject{ + TypedReference: v1alpha1.TypedReference{ + APIVersion: tempFoo.GetAPIVersion(), + Name: inName, + Kind: tempFoo.GetKind(), + }, + FieldPaths: []string{ + "spec.key", + }}}}} + logf.Log.Info("Checking on appconfig", "Key", appconfigKey) + Expect(func() v1alpha2.DependencyStatus { + k8sClient.Get(ctx, appconfigKey, appconfig) + return appconfig.Status.Dependency + }()).Should(Equal(depStatus)) + + // fill value to fieldPath + Expect(unstructured.SetNestedField(outFoo.Object, "test", "status", "key")).Should(BeNil()) + Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) + + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + // Verification after satisfying dependency + By("Checking that resource which accepts data is created now") + logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) + + Eventually(func() error { + err := k8sClient.Get(ctx, inFooKey, inFoo) + if err != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return err + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Verify the appconfig's dependency is satisfied") + appconfig = &v1alpha2.ApplicationConfiguration{} + Eventually(func() []v1alpha2.UnstaifiedDependency { + k8sClient.Get(ctx, appconfigKey, appconfig) + if appconfig.Status.Dependency.Unsatisfied != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return appconfig.Status.Dependency.Unsatisfied + }, time.Second, 300*time.Millisecond).Should(BeNil()) + } + + It("trait depends on another trait", func() { + label := map[string]string{"trait": "trait"} + // Define a workload + wl := tempFoo.DeepCopy() + wl.SetName("workload") + // Create a component + componentName := "component" + comp := v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ComponentSpec{ + Workload: runtime.RawExtension{ + Object: wl, + }, + }, + } + Expect(k8sClient.Create(ctx, &comp)).Should(BeNil()) + logf.Log.Info("Creating component", "Name", comp.Name, "Namespace", comp.Namespace) + // Create application configuration + appConfigName := "appconfig-trait-trait" + appConfig := v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: componentName, + Traits: []v1alpha2.ComponentTrait{{ + Trait: runtime.RawExtension{ + Object: out, + }, + DataOutputs: []v1alpha2.DataOutput{{ + Name: "trait-trait", + FieldPath: "status.key", + }}}, { + Trait: runtime.RawExtension{ + Object: in, + }, + DataInputs: []v1alpha2.DataInput{{ + ValueFrom: v1alpha2.DataInputValueFrom{ + DataOutputName: "trait-trait", + }, + ToFieldPaths: []string{"spec.key"}, + }}, + }}, + }}}, + } + logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) + Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) + verify(appConfigName, "status.key not found in object") + }) + + It("component depends on another component", func() { + label := map[string]string{"component": "component"} + // Create application configuration + appConfigName := "appconfig-comp-comp" + appConfig := v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: componentOutName, + DataOutputs: []v1alpha2.DataOutput{{ + Name: "comp-comp", + FieldPath: "status.key", + }}}, { + ComponentName: componentInName, + DataInputs: []v1alpha2.DataInput{{ + ValueFrom: v1alpha2.DataInputValueFrom{ + DataOutputName: "comp-comp", + }, + ToFieldPaths: []string{"spec.key"}, + }}}}}, + } + logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) + Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) + verify(appConfigName, "status.key not found in object") + }) + + It("component depends on trait", func() { + label := map[string]string{"trait": "component"} + // Create application configuration + appConfigName := "appconfig-trait-comp" + appConfig := v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: componentInName, + DataInputs: []v1alpha2.DataInput{{ + ValueFrom: v1alpha2.DataInputValueFrom{ + DataOutputName: "trait-comp", + }, + ToFieldPaths: []string{"spec.key"}, + }}, + Traits: []v1alpha2.ComponentTrait{{ + Trait: runtime.RawExtension{ + Object: out, + }, + DataOutputs: []v1alpha2.DataOutput{{ + Name: "trait-comp", + FieldPath: "status.key", + }}}}}}}, + } + logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) + Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) + verify(appConfigName, "status.key not found in object") + }) + + It("trait depends on component", func() { + label := map[string]string{"component": "trait"} + // Create application configuration + appConfigName := "appconfig-comp-trait" + appConfig := v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: componentOutName, + DataOutputs: []v1alpha2.DataOutput{{ + Name: "comp-trait", + FieldPath: "status.key", + }}, + Traits: []v1alpha2.ComponentTrait{{ + Trait: runtime.RawExtension{ + Object: in, + }, + DataInputs: []v1alpha2.DataInput{{ + ValueFrom: v1alpha2.DataInputValueFrom{ + DataOutputName: "comp-trait", + }, + ToFieldPaths: []string{"spec.key"}, + }}}}}}}, + } + logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) + Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) + verify(appConfigName, "status.key not found in object") + }) + + It("component depends on trait with updated condition", func() { + label := map[string]string{"trait": "component", "app-hash": "hash-v1"} + // Create application configuration + appConfigName := "appconfig-trait-comp-update" + appConfig := v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: componentInName, + DataInputs: []v1alpha2.DataInput{{ + ValueFrom: v1alpha2.DataInputValueFrom{ + DataOutputName: "trait-comp", + }, + ToFieldPaths: []string{"spec.key"}}}, + Traits: []v1alpha2.ComponentTrait{{ + Trait: runtime.RawExtension{ + Object: out, + }, + DataOutputs: []v1alpha2.DataOutput{{ + Name: "trait-comp", + FieldPath: "status.key", + Conditions: []v1alpha2.ConditionRequirement{{ + Operator: v1alpha2.ConditionEqual, + ValueFrom: v1alpha2.ValueFrom{FieldPath: "metadata.labels.app-hash"}, + FieldPath: "status.app-hash", + }}}}}}}}}, + } + logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) + Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) + + By("Reconcile") + appconfigKey := client.ObjectKey{ + Name: appConfigName, + Namespace: namespace, + } + req := reconcile.Request{NamespacedName: appconfigKey} + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + By("Checking that resource which accepts data isn't created yet") + inFooKey := client.ObjectKey{ + Name: inName, + Namespace: namespace, + } + inFoo := tempFoo.DeepCopy() + logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) + Expect(k8sClient.Get(ctx, inFooKey, inFoo)).Should(&util.NotFoundMatcher{}) + + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + By("Checking that resource which provides data is created") + // Originally the trait has value in `status.key`, but the hash label is old + outFooKey := client.ObjectKey{ + Name: outName, + Namespace: namespace, + } + outFoo := tempFoo.DeepCopy() + Eventually(func() error { + err := k8sClient.Get(ctx, outFooKey, outFoo) + if err != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return err + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + err := unstructured.SetNestedField(outFoo.Object, "test", "status", "key") + Expect(err).Should(BeNil()) + err = unstructured.SetNestedField(outFoo.Object, "hash-v1", "status", "app-hash") + Expect(err).Should(BeNil()) + Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) + + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + // Verification after satisfying dependency + By("Verify the appconfig's dependency is satisfied") + newAppConfig := &v1alpha2.ApplicationConfiguration{} + + Eventually(func() []v1alpha2.UnstaifiedDependency { + var tempApp = &v1alpha2.ApplicationConfiguration{} + err = k8sClient.Get(ctx, appconfigKey, tempApp) + tempApp.DeepCopyInto(newAppConfig) + if err != nil || tempApp.Status.Dependency.Unsatisfied != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return tempApp.Status.Dependency.Unsatisfied + }(), time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Checking that resource which accepts data is created now") + logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) + Expect(k8sClient.Get(ctx, inFooKey, inFoo)).Should(BeNil()) + + By("Update AppConfig with new version") + newAppConfig.Labels["app-hash"] = "hash-v2" + Expect(k8sClient.Update(ctx, newAppConfig)).Should(BeNil()) + time.Sleep(time.Second) + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + By("Verify the appconfig's dependency should be unsatisfied, because requirementCondition valueFrom not match") + depStatus := v1alpha2.DependencyStatus{ + Unsatisfied: []v1alpha2.UnstaifiedDependency{{ + Reason: "got(hash-v1) expected to be hash-v2", + From: v1alpha2.DependencyFromObject{ + TypedReference: v1alpha1.TypedReference{ + APIVersion: tempFoo.GetAPIVersion(), + Name: outName, + Kind: tempFoo.GetKind(), + }, + FieldPath: "status.key", + }, + To: v1alpha2.DependencyToObject{ + TypedReference: v1alpha1.TypedReference{ + APIVersion: tempFoo.GetAPIVersion(), + Name: inName, + Kind: tempFoo.GetKind(), + }, + FieldPaths: []string{ + "spec.key", + }}}}, + } + Expect(func() v1alpha2.DependencyStatus { + k8sClient.Get(ctx, appconfigKey, newAppConfig) + return newAppConfig.Status.Dependency + }()).Should(Equal(depStatus)) + + By("Update trait resource to meet the requirement") + Expect(k8sClient.Get(ctx, outFooKey, outFoo)).Should(BeNil()) // Get the latest before update + Expect(unstructured.SetNestedField(outFoo.Object, "test-new", "status", "key")).Should(BeNil()) + Expect(unstructured.SetNestedField(outFoo.Object, "hash-v2", "status", "app-hash")).Should(BeNil()) + Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) + + By("Reconcile") + Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil()) + + By("Verify the appconfig's dependency is satisfied") + Eventually( + func() []v1alpha2.UnstaifiedDependency { + tempAppConfig := &v1alpha2.ApplicationConfiguration{} + err := k8sClient.Get(ctx, appconfigKey, tempAppConfig) + if err != nil || tempAppConfig.Status.Dependency.Unsatisfied != nil { + // Try 3 (= 1s/300ms) times + reconciler.Reconcile(req) + } + return tempAppConfig.Status.Dependency.Unsatisfied + }(), time.Second, 300*time.Millisecond).Should(BeNil()) + + By("Checking that resource which accepts data is updated") + Expect(func() string { + k8sClient.Get(ctx, inFooKey, inFoo) + outdata, _, _ := unstructured.NestedString(inFoo.Object, "spec", "key") + return outdata + }()).Should(Equal("test-new")) + + }) +}) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 7b1c3122..b9b2f01c 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -27,7 +27,6 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -36,6 +35,7 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -84,7 +84,7 @@ var _ ComponentRenderer = &components{} type components struct { client client.Reader - mapper meta.RESTMapper + dm discoverymapper.DiscoveryMapper params ParameterResolver workload ResourceRenderer trait ResourceRenderer @@ -213,7 +213,7 @@ func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait setTraitProperties(t, traitName, ac.GetNamespace(), ref) - traitDef, err := util.FetchTraitDefinition(ctx, r.client, r.mapper, t) + traitDef, err := util.FetchTraitDefinition(ctx, r.client, r.dm, t) if err != nil { return nil, nil, errors.Wrapf(err, errFmtGetTraitDefinition, t.GetAPIVersion(), t.GetKind(), t.GetName()) } diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index e2212e4d..b3a739cd 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -21,9 +21,6 @@ import ( "encoding/json" "testing" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" @@ -503,7 +500,7 @@ func TestRenderComponents(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := &components{tc.fields.client, mock.NewMockMapper(), tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, mock.NewMockDiscoveryMapper(), tc.fields.params, tc.fields.workload, tc.fields.trait} got, _, err := r.Render(tc.args.ctx, tc.args.ac) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, diff) @@ -849,7 +846,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := &components{tc.fields.client, mock.NewMockMapper(), tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, mock.NewMockDiscoveryMapper(), 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].Object.GetName() != util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy()) { t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, "Trait name is NOT"+ @@ -888,14 +885,9 @@ func TestGetDefinitionName(t *testing.T) { } for name, ti := range tests { t.Run(name, func(t *testing.T) { - mapper := func(resource string) *mock.Mapper { - m := mock.NewMockMapper() - m.MockRESTMapping = func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - return &meta.RESTMapping{Resource: schema.GroupVersionResource{Resource: resource}}, nil - } - return m - }(ti.resource) - got, err := util.GetDefinitionName(mapper, ti.u, "") + m := mock.NewMockDiscoveryMapper() + m.MockRESTMapping = mock.NewMockRESTMapping(ti.resource) + got, err := util.GetDefinitionName(m, ti.u, "") assert.NoError(t, err) if got != ti.exp { t.Errorf("%s getCRDName want %s got %s ", ti.reason, ti.exp, got) @@ -1008,10 +1000,8 @@ func TestSetTraitProperties(t *testing.T) { expU.SetOwnerReferences([]metav1.OwnerReference{{Name: "comp1"}}) } -var scheme = runtime.NewScheme() - func TestRenderTraitName(t *testing.T) { - + var scheme = runtime.NewScheme() assert.NoError(t, clientgoscheme.AddToScheme(scheme)) assert.NoError(t, core.AddToScheme(scheme)) namespace := "ns" diff --git a/pkg/controller/v1alpha2/applicationconfiguration/suite_test.go b/pkg/controller/v1alpha2/applicationconfiguration/suite_test.go new file mode 100644 index 00000000..d30a5f4c --- /dev/null +++ b/pkg/controller/v1alpha2/applicationconfiguration/suite_test.go @@ -0,0 +1,192 @@ +package applicationconfiguration + +import ( + "context" + "path/filepath" + "testing" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/envtest/printer" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + controllerscheme "sigs.k8s.io/controller-runtime/pkg/scheme" + + "github.com/crossplane/oam-kubernetes-runtime/apis/core" + "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" + // +kubebuilder:scaffold:imports +) + +var reconciler *OAMApplicationReconciler +var mgrclose chan struct{} +var testEnv *envtest.Environment +var cfg *rest.Config +var k8sClient client.Client +var scheme = runtime.NewScheme() +var crd crdv1.CustomResourceDefinition + +func TestReconcilder(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecsWithDefaultAndCustomReporters(t, + "OAM Core Resource Controller Unit test Suite", + []Reporter{printer.NewlineReporter{}}) +} + +var _ = BeforeSuite(func(done Done) { + ctx := context.Background() + By("Bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("../../../..", "charts/oam-kubernetes-runtime/crds"), // this has all the required CRDs, + }, + } + var err error + cfg, err = testEnv.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(cfg).ToNot(BeNil()) + + logf.SetLogger(zap.New(zap.UseDevMode(true), zap.WriteTo(GinkgoWriter))) + Expect(clientgoscheme.AddToScheme(scheme)).Should(BeNil()) + Expect(core.AddToScheme(scheme)).Should(BeNil()) + Expect(crdv1.AddToScheme(scheme)).Should(BeNil()) + depExample := &unstructured.Unstructured{} + depExample.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "example.com", + Version: "v1", + Kind: "Foo", + }) + depSchemeBuilder := &controllerscheme.Builder{GroupVersion: schema.GroupVersion{Group: "example.com", Version: "v1"}} + depSchemeBuilder.Register(depExample.DeepCopyObject()) + Expect(depSchemeBuilder.AddToScheme(scheme)).Should(BeNil()) + + By("Setting up kubernetes client") + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + if err != nil { + logf.Log.Error(err, "failed to create k8sClient") + Fail("setup failed") + } + By("Finished setting up test environment") + + By("Creating Reconciler for appconfig") + mgr, err := ctrl.NewManager(cfg, ctrl.Options{Scheme: scheme, MetricsBindAddress: "0"}) + Expect(err).Should(BeNil()) + mgrclose = make(chan struct{}) + go mgr.Start(mgrclose) + + // Create a crd for appconfig dependency test + crd = crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + Labels: map[string]string{"crd": "dependency"}, + }, + Spec: crdv1.CustomResourceDefinitionSpec{ + Group: "example.com", + Names: crdv1.CustomResourceDefinitionNames{ + Kind: "Foo", + ListKind: "FooList", + Plural: "foo", + Singular: "foo", + }, + Versions: []crdv1.CustomResourceDefinitionVersion{{ + Name: "v1", + Served: true, + Storage: true, + Schema: &crdv1.CustomResourceValidation{ + OpenAPIV3Schema: &crdv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "key": {Type: "string"}, + }}, + "status": { + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "key": {Type: "string"}, + "app-hash": {Type: "string"}, + }}}}}}, + }, + Scope: crdv1.NamespaceScoped, + }, + } + Expect(k8sClient.Create(context.Background(), &crd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + By("Created a crd for appconfig dependency test") + + dm, err := discoverymapper.New(cfg) + Expect(err).Should(BeNil()) + + var mapping *meta.RESTMapping + Eventually(func() error { + mapping, err = dm.RESTMapping(schema.GroupKind{ + Group: "example.com", + Kind: "Foo", + }, "v1") + return err + }, time.Second*30, time.Millisecond*500).Should(BeNil()) + Expect(mapping.Resource.Resource).Should(Equal("foo")) + + reconciler = NewReconciler(mgr, dm, WithLogger(logging.NewLogrLogger(ctrl.Log.WithName("suit-test-appconfig")))) + + By("Creating workload definition and trait definition") + wd := v1alpha2.WorkloadDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + }, + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foo.example.com", + }, + }, + } + td := v1alpha2.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + }, + Spec: v1alpha2.TraitDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foo.example.com", + }, + }, + } + // For some reason, WorkloadDefinition is created as a Cluster scope object + Expect(k8sClient.Create(ctx, &wd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + // For some reason, TraitDefinition is created as a Cluster scope object + Expect(k8sClient.Create(ctx, &td)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + close(done) +}, 300) + +var _ = AfterSuite(func() { + + crd = crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + Labels: map[string]string{"crd": "dependency"}, + }, + } + Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) + By("Deleted the custom resource definition") + + By("Tearing down the test environment") + err := testEnv.Stop() + Expect(err).ToNot(HaveOccurred()) + close(mgrclose) +}) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 32dbda1b..4ba983eb 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -23,6 +23,7 @@ import ( "reflect" "time" + "github.com/pkg/errors" apps "k8s.io/api/apps/v1" core "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,8 +35,6 @@ import ( corev1alpha2 "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" - "github.com/pkg/errors" - runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" ) diff --git a/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go b/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go index e8ba7858..d3b6f7dc 100644 --- a/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go +++ b/pkg/controller/v1alpha2/core/traits/manualscalertrait/manualscalertrait_controller.go @@ -20,9 +20,6 @@ import ( "fmt" "strings" - "k8s.io/apimachinery/pkg/api/meta" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/logging" @@ -42,6 +39,7 @@ import ( oamv1alpha2 "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/controller" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -54,14 +52,14 @@ const ( // Setup adds a controller that reconciles ContainerizedWorkload. func Setup(mgr ctrl.Manager, args controller.Args, log logging.Logger) error { - mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + dm, err := discoverymapper.New(mgr.GetConfig()) if err != nil { return err } reconciler := Reconciler{ Client: mgr.GetClient(), DiscoveryClient: *discovery.NewDiscoveryClientForConfigOrDie(mgr.GetConfig()), - mapper: mapper, + dm: dm, log: ctrl.Log.WithName("ManualScalarTrait"), record: event.NewAPIRecorder(mgr.GetEventRecorderFor("ManualScalarTrait")), Scheme: mgr.GetScheme(), @@ -73,7 +71,7 @@ func Setup(mgr ctrl.Manager, args controller.Args, log logging.Logger) error { type Reconciler struct { client.Client discovery.DiscoveryClient - mapper meta.RESTMapper + dm discoverymapper.DiscoveryMapper log logr.Logger record event.Recorder Scheme *runtime.Scheme @@ -114,7 +112,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // Fetch the child resources list from the corresponding workload - resources, err := util.FetchWorkloadChildResources(ctx, mLog, r, r.mapper, workload) + resources, err := util.FetchWorkloadChildResources(ctx, mLog, r, r.dm, workload) if err != nil { mLog.Error(err, "Error while fetching the workload child resources", "workload", workload.UnstructuredContent()) r.record.Event(eventObj, event.Warning(util.ErrFetchChildResources, err)) diff --git a/pkg/oam/discoverymapper/mapper.go b/pkg/oam/discoverymapper/mapper.go index abe2de3f..d2840eb8 100644 --- a/pkg/oam/discoverymapper/mapper.go +++ b/pkg/oam/discoverymapper/mapper.go @@ -8,6 +8,7 @@ import ( "k8s.io/client-go/restmapper" ) +// DiscoveryMapper is a interface for refresh and discovery resources from GVK. type DiscoveryMapper interface { GetMapper() (meta.RESTMapper, error) Refresh() (meta.RESTMapper, error) @@ -52,6 +53,7 @@ func (d *DefaultDiscoveryMapper) Refresh() (meta.RESTMapper, error) { return d.mapper, nil } +// RESTMapping will mapping resources from GVK, if not found, it will refresh from APIServer and try once again func (d *DefaultDiscoveryMapper) RESTMapping(gk schema.GroupKind, version ...string) (*meta.RESTMapping, error) { mapper, err := d.GetMapper() if err != nil { diff --git a/pkg/oam/discoverymapper/suit_test.go b/pkg/oam/discoverymapper/suit_test.go index c9e5f82d..30c46e79 100644 --- a/pkg/oam/discoverymapper/suit_test.go +++ b/pkg/oam/discoverymapper/suit_test.go @@ -8,7 +8,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -27,8 +26,6 @@ import ( var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment -var controllerDone chan struct{} -var routeNS corev1.Namespace var scheme = runtime.NewScheme() func TestMapper(t *testing.T) { @@ -70,7 +67,7 @@ var _ = Describe("Mapper discovery resources", func() { Expect(err).Should(BeNil()) mapper, err := dism.GetMapper() Expect(err).Should(BeNil()) - mapping, err := mapper.RESTMapping(schema.GroupKind{"apps", "Deployment"}, "v1") + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "Deployment"}, "v1") Expect(err).Should(BeNil()) Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ Group: "apps", @@ -87,7 +84,7 @@ var _ = Describe("Mapper discovery resources", func() { mapper, err := dism.GetMapper() Expect(err).Should(BeNil()) var mapping *meta.RESTMapping - mapping, err = mapper.RESTMapping(schema.GroupKind{"", "Pod"}, "v1") + mapping, err = mapper.RESTMapping(schema.GroupKind{Group: "", Kind: "Pod"}, "v1") Expect(err).Should(BeNil()) Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ Group: "", @@ -122,7 +119,7 @@ var _ = Describe("Mapper discovery resources", func() { Expect(k8sClient.Create(context.Background(), &crd)).Should(BeNil()) Eventually(func() error { - mapping, err = dism.RESTMapping(schema.GroupKind{"example.com", "Foo"}, "v1") + mapping, err = dism.RESTMapping(schema.GroupKind{Group: "example.com", Kind: "Foo"}, "v1") return err }, time.Second*5, time.Millisecond*500).Should(BeNil()) Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{ diff --git a/pkg/oam/mock/mapper.go b/pkg/oam/mock/mapper.go index a4392152..9b334301 100644 --- a/pkg/oam/mock/mapper.go +++ b/pkg/oam/mock/mapper.go @@ -1,89 +1,55 @@ package mock import ( + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" ) -var _ meta.RESTMapper = &Mapper{} - -// nolint -type KindFor func(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) - -// nolint -type KindsFor func(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) +var _ discoverymapper.DiscoveryMapper = &DiscoveryMapper{} // nolint -type ResourceFor func(input schema.GroupVersionResource) (schema.GroupVersionResource, error) +type GetMapper func() (meta.RESTMapper, error) // nolint -type ResourcesFor func(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) +type Refresh func() (meta.RESTMapper, error) // nolint type RESTMapping func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) -// nolint -type RESTMappings func(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) - -// nolint -type ResourceSingularizer func(resource string) (singular string, err error) - -// NewMockMapper for mock -func NewMockMapper() *Mapper { - return &Mapper{ +// NewMockDiscoveryMapper for mock +func NewMockDiscoveryMapper() *DiscoveryMapper { + return &DiscoveryMapper{ MockRESTMapping: NewMockRESTMapping(""), } } -// NewMockRESTMapping for mock +// NewMockRESTMapping for unit test only func NewMockRESTMapping(resource string) RESTMapping { return func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - return &meta.RESTMapping{Resource: schema.GroupVersionResource{Resource: resource}}, nil + return &meta.RESTMapping{Resource: schema.GroupVersionResource{Resource: resource, Version: versions[0], Group: gk.Group}}, nil } } -// Mapper for mock -type Mapper struct { - MockKindFor KindFor - MockKindsFor KindsFor - MockResourceFor ResourceFor - MockResourcesFor ResourcesFor - MockRESTMapping RESTMapping - MockRESTMappings RESTMappings - MockResourceSingularizer ResourceSingularizer +// DiscoveryMapper for unit test only, use GetMapper and refresh will panic +type DiscoveryMapper struct { + MockGetMapper GetMapper + MockRefresh Refresh + MockRESTMapping RESTMapping } -// KindFor for mock -func (m *Mapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { - return m.MockKindFor(resource) +// GetMapper for mock +func (m *DiscoveryMapper) GetMapper() (meta.RESTMapper, error) { + return m.MockGetMapper() } -// KindsFor for mock -func (m *Mapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { - return m.MockKindsFor(resource) -} - -// ResourceFor for mock -func (m *Mapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { - return m.MockResourceFor(input) -} - -// ResourcesFor for mock -func (m *Mapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { - return m.MockResourcesFor(input) +// Refresh for mock +func (m *DiscoveryMapper) Refresh() (meta.RESTMapper, error) { + return m.MockRefresh() } // RESTMapping for mock -func (m *Mapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { +func (m *DiscoveryMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { return m.MockRESTMapping(gk, versions...) } - -// RESTMappings for mock -func (m *Mapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { - return m.MockRESTMappings(gk, versions...) -} - -// ResourceSingularizer for mock -func (m *Mapper) ResourceSingularizer(resource string) (singular string, err error) { - return m.MockResourceSingularizer(resource) -} diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 5cbcfa8a..5d485c5a 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -9,25 +9,22 @@ import ( "reflect" "time" - "k8s.io/apimachinery/pkg/runtime/schema" - - "k8s.io/apimachinery/pkg/api/meta" - + cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/davecgh/go-spew/spew" "github.com/go-logr/logr" "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "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" - cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" - "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" ) var ( @@ -115,11 +112,11 @@ func FetchWorkload(ctx context.Context, c client.Client, mLog logr.Logger, oamTr } // FetchScopeDefinition fetch corresponding scopeDefinition given a scope -func FetchScopeDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, +func FetchScopeDefinition(ctx context.Context, r client.Reader, dm discoverymapper.DiscoveryMapper, scope *unstructured.Unstructured) (*v1alpha2.ScopeDefinition, error) { // The name of the scopeDefinition CR is the CRD name of the scope // TODO(wonderflow): we haven't support scope definition label type yet. - spName, err := GetDefinitionName(mapper, scope, "") + spName, err := GetDefinitionName(dm, scope, "") if err != nil { return nil, err } @@ -134,10 +131,10 @@ func FetchScopeDefinition(ctx context.Context, r client.Reader, mapper meta.REST } // FetchTraitDefinition fetch corresponding traitDefinition given a trait -func FetchTraitDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, +func FetchTraitDefinition(ctx context.Context, r client.Reader, dm discoverymapper.DiscoveryMapper, trait *unstructured.Unstructured) (*v1alpha2.TraitDefinition, error) { // The name of the traitDefinition CR is the CRD name of the trait - trName, err := GetDefinitionName(mapper, trait, oam.TraitTypeLabel) + trName, err := GetDefinitionName(dm, trait, oam.TraitTypeLabel) if err != nil { return nil, err } @@ -152,10 +149,10 @@ func FetchTraitDefinition(ctx context.Context, r client.Reader, mapper meta.REST } // FetchWorkloadDefinition fetch corresponding workloadDefinition given a workload -func FetchWorkloadDefinition(ctx context.Context, r client.Reader, mapper meta.RESTMapper, +func FetchWorkloadDefinition(ctx context.Context, r client.Reader, dm discoverymapper.DiscoveryMapper, workload *unstructured.Unstructured) (*v1alpha2.WorkloadDefinition, error) { // The name of the workloadDefinition CR is the CRD name of the component - wldName, err := GetDefinitionName(mapper, workload, oam.WorkloadTypeLabel) + wldName, err := GetDefinitionName(dm, workload, oam.WorkloadTypeLabel) if err != nil { return nil, err } @@ -171,9 +168,9 @@ func FetchWorkloadDefinition(ctx context.Context, r client.Reader, mapper meta.R // FetchWorkloadChildResources fetch corresponding child resources given a workload func FetchWorkloadChildResources(ctx context.Context, mLog logr.Logger, r client.Reader, - mapper meta.RESTMapper, workload *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { + dm discoverymapper.DiscoveryMapper, workload *unstructured.Unstructured) ([]*unstructured.Unstructured, error) { // Fetch the corresponding workloadDefinition CR - workloadDefinition, err := FetchWorkloadDefinition(ctx, r, mapper, workload) + workloadDefinition, err := FetchWorkloadDefinition(ctx, r, dm, workload) if err != nil { return nil, err } @@ -241,7 +238,7 @@ func PassLabelAndAnnotation(parentObj oam.Object, childObj labelAnnotationObject // the format of the definition of a resource is . // Now the definition name of a resource could also be defined as `definition.oam.dev/name` in `metadata.annotations` // typeLabel specified which Definition it is, if specified, will directly get definition from label. -func GetDefinitionName(mapper meta.RESTMapper, u *unstructured.Unstructured, typeLabel string) (string, error) { +func GetDefinitionName(dm discoverymapper.DiscoveryMapper, u *unstructured.Unstructured, typeLabel string) (string, error) { if typeLabel != "" { if labels := u.GetLabels(); labels != nil { if definitionName, ok := labels[typeLabel]; ok { @@ -253,7 +250,7 @@ func GetDefinitionName(mapper meta.RESTMapper, u *unstructured.Unstructured, typ if err != nil { return "", err } - mapping, err := mapper.RESTMapping(schema.GroupKind{Group: groupVersion.Group, Kind: u.GetKind()}, groupVersion.Version) + mapping, err := dm.RESTMapping(schema.GroupKind{Group: groupVersion.Group, Kind: u.GetKind()}, groupVersion.Version) if err != nil { return "", err } diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index 2c93fdd1..b0eddf9f 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -341,7 +341,7 @@ func TestScopeRelatedUtils(t *testing.T) { tclient := test.MockClient{ MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - got, err := util.FetchScopeDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredScope) + got, err := util.FetchScopeDefinition(ctx, &tclient, mock.NewMockDiscoveryMapper(), unstructuredScope) t.Log(fmt.Sprint("Running test: ", name)) assert.Equal(t, tc.want.err, err) assert.Equal(t, tc.want.spd, got) @@ -434,7 +434,7 @@ func TestTraitHelper(t *testing.T) { tclient := test.MockClient{ MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - got, err := util.FetchTraitDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredTrait) + got, err := util.FetchTraitDefinition(ctx, &tclient, mock.NewMockDiscoveryMapper(), unstructuredTrait) t.Log(fmt.Sprint("Running test: ", name)) assert.Equal(t, tc.want.err, err) assert.Equal(t, tc.want.td, got) @@ -521,7 +521,7 @@ func TestUtils(t *testing.T) { tclient := test.MockClient{ MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), } - got, err := util.FetchWorkloadDefinition(ctx, &tclient, mock.NewMockMapper(), unstructuredWorkload) + got, err := util.FetchWorkloadDefinition(ctx, &tclient, mock.NewMockDiscoveryMapper(), unstructuredWorkload) t.Log(fmt.Sprint("Running test: ", name)) assert.Equal(t, tc.want.err, err) @@ -705,7 +705,7 @@ func TestChildResources(t *testing.T) { MockGet: test.NewMockGetFn(nil, tc.fields.getFunc), MockList: test.NewMockListFn(nil, tc.fields.listFunc), } - got, err := util.FetchWorkloadChildResources(ctx, log, &tclient, mock.NewMockMapper(), unstructuredWorkload) + got, err := util.FetchWorkloadChildResources(ctx, log, &tclient, mock.NewMockDiscoveryMapper(), unstructuredWorkload) t.Log(fmt.Sprint("Running test: ", name)) assert.Equal(t, tc.want.err, err) assert.Equal(t, tc.want.crks, got) @@ -763,7 +763,7 @@ func TestUnstructured(t *testing.T) { }, } for name, ti := range tests { - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() mapper.MockRESTMapping = mock.NewMockRESTMapping(ti.resource) got, err := util.GetDefinitionName(mapper, ti.u, ti.typeLabel) assert.NoError(t, err) diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go index 258fe24d..069f5a04 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -22,6 +20,7 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -191,7 +190,7 @@ var _ = Describe("ApplicationConfiguration Admission controller Test", func() { }) It("Test validating handler", func() { - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() var handler admission.Handler = &ValidatingHandler{Mapper: mapper} decoderInjector := handler.(admission.DecoderInjector) decoderInjector.InjectDecoder(decoder) diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go index 193ba385..5aa60909 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go @@ -5,13 +5,12 @@ import ( "encoding/json" "strings" - "k8s.io/apimachinery/pkg/api/meta" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -21,7 +20,7 @@ const ( ) // checkComponentVersionEnabled check whethter a component is versioning mechanism enabled -func checkComponentVersionEnabled(ctx context.Context, client client.Reader, mapper meta.RESTMapper, +func checkComponentVersionEnabled(ctx context.Context, client client.Reader, dm discoverymapper.DiscoveryMapper, acc *v1alpha2.ApplicationConfigurationComponent) (bool, error) { if acc.RevisionName != "" { return true, nil @@ -31,7 +30,7 @@ func checkComponentVersionEnabled(ctx context.Context, client client.Reader, map if err := json.Unmarshal(ct.Trait.Raw, ut); err != nil { return false, errors.Wrap(err, errUnmarshalTrait) } - td, err := util.FetchTraitDefinition(ctx, client, mapper, ut) + td, err := util.FetchTraitDefinition(ctx, client, dm, ut) if err != nil { return false, errors.Wrapf(err, errFmtGetTraitDefinition, ut.GetAPIVersion(), ut.GetKind(), ut.GetName()) } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go index 98b67dc4..3234c119 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go @@ -5,8 +5,6 @@ import ( "fmt" "testing" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" - "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/stretchr/testify/assert" @@ -17,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" ) func TestCheckComponentVersionEnabled(t *testing.T) { @@ -100,7 +99,7 @@ func TestCheckComponentVersionEnabled(t *testing.T) { result: false, }, } - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() for _, tv := range tests { func(t *testing.T) { diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go index b7845f30..cbb13daa 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go @@ -6,19 +6,14 @@ import ( "fmt" "net/http" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - - "k8s.io/apimachinery/pkg/api/meta" - - "k8s.io/apimachinery/pkg/util/validation/field" - - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" - "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/klog" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -43,7 +38,7 @@ var appConfigResource = v1alpha2.SchemeGroupVersion.WithResource("applicationcon // ValidatingHandler handles CloneSet type ValidatingHandler struct { Client client.Client - Mapper meta.RESTMapper + Mapper discoverymapper.DiscoveryMapper // Decoder decodes objects Decoder *admission.Decoder @@ -131,11 +126,11 @@ func checkRevisionName(appConfig *v1alpha2.ApplicationConfiguration) (bool, stri } // checkWorkloadNameForVersioning check whether versioning-enabled component workload name is empty -func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, mapper meta.RESTMapper, +func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, dm discoverymapper.DiscoveryMapper, appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { for _, v := range appConfig.Spec.Components { acc := v - vEnabled, err := checkComponentVersionEnabled(ctx, client, mapper, &acc) + vEnabled, err := checkComponentVersionEnabled(ctx, client, dm, &acc) if err != nil { return false, fmt.Sprintf(errFmtCheckWorkloadName, err.Error()) } @@ -184,7 +179,7 @@ func (h *ValidatingHandler) InjectDecoder(d *admission.Decoder) error { // RegisterValidatingHandler will register application configuration validation to webhook func RegisterValidatingHandler(mgr manager.Manager) error { server := mgr.GetWebhookServer() - mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig()) + mapper, err := discoverymapper.New(mgr.GetConfig()) if err != nil { return err } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go index 930d38ad..8bd09dfd 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go @@ -152,7 +152,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { Name: tName, }}) - mapper := mock.NewMockMapper() + mapper := mock.NewMockDiscoveryMapper() tests := []struct { caseName string diff --git a/test/e2e-test/appconfig_dependency_test.go b/test/e2e-test/appconfig_dependency_test.go deleted file mode 100644 index 15d177a5..00000000 --- a/test/e2e-test/appconfig_dependency_test.go +++ /dev/null @@ -1,592 +0,0 @@ -package controllers_test - -import ( - "context" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" - - "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" -) - -var _ = Describe("Resource Dependency in an ApplicationConfiguration", func() { - ctx := context.Background() - namespace := "appconfig-dependency-test" - var ns corev1.Namespace - var wd v1alpha2.WorkloadDefinition - var td v1alpha2.TraitDefinition - tempFoo := &unstructured.Unstructured{} - tempFoo.SetAPIVersion("example.com/v1") - tempFoo.SetKind("Foo") - tempFoo.SetNamespace(namespace) - outName := "data-output" - out := tempFoo.DeepCopy() - out.SetName(outName) - inName := "data-input" - in := tempFoo.DeepCopy() - in.SetName(inName) - componentOutName := "component-out" - componentInName := "component-in" - BeforeEach(func() { - ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespace, - }, - } - logf.Log.Info("Start to run a test, clean up previous resources") - // delete the namespace with all its resources - Eventually( - // gomega has a bug that can't take nil as the actual input, so has to make it a func - func() error { - return k8sClient.Delete(ctx, &ns, client.PropagationPolicy(metav1.DeletePropagationForeground)) - }, - time.Second*30, time.Millisecond*500).Should(SatisfyAny(BeNil(), &util.NotFoundMatcher{})) - - logf.Log.Info("make sure all the resources are removed") - objectKey := client.ObjectKey{ - Name: namespace, - } - res := &corev1.Namespace{} - Eventually( - // gomega has a bug that can't take nil as the actual input, so has to make it a func - func() error { - return k8sClient.Get(ctx, objectKey, res) - }, - time.Second*30, time.Millisecond*500).Should(&util.NotFoundMatcher{}) - // recreate the namespace for testing - Eventually( - func() error { - return k8sClient.Create(ctx, &ns) - }, - time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) - wd = v1alpha2.WorkloadDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo.example.com", - }, - Spec: v1alpha2.WorkloadDefinitionSpec{ - Reference: v1alpha2.DefinitionReference{ - Name: "foo.example.com", - }, - }, - } - td = v1alpha2.TraitDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foos.example.com", - }, - Spec: v1alpha2.TraitDefinitionSpec{ - Reference: v1alpha2.DefinitionReference{ - Name: "foos.example.com", - }, - }, - } - logf.Log.Info("Creating workload definition") - // For some reason, WorkloadDefinition is created as a Cluster scope object - Expect(k8sClient.Create(ctx, &wd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) - logf.Log.Info("Creating trait definition") - // For some reason, TraitDefinition is created as a Cluster scope object - Expect(k8sClient.Create(ctx, &td)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) - // Create a component for data outputs - compOut := v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentOutName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Object: out, - }, - }, - } - Expect(k8sClient.Create(ctx, &compOut)).Should(BeNil()) - logf.Log.Info("Creating component that outputs data", "Name", compOut.Name, "Namespace", compOut.Namespace) - // Create a component for data inputs - compIn := v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentInName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Object: in, - }, - }, - } - Expect(k8sClient.Create(ctx, &compIn)).Should(BeNil()) - logf.Log.Info("Creating component that inputs data", "Name", compIn.Name, "Namespace", compIn.Namespace) - }) - AfterEach(func() { - logf.Log.Info("Clean up resources") - // Delete the WorkloadDefinition and TraitDefinition - Expect(k8sClient.Delete(ctx, &wd)).Should(BeNil()) - Expect(k8sClient.Delete(ctx, &td)).Should(BeNil()) - // delete the namespace with all its resources - Expect(k8sClient.Delete(ctx, &ns, client.PropagationPolicy(metav1.DeletePropagationForeground))).Should(BeNil()) - }) - - // common function for verification - verify := func(appConfigName, reason string) { - // Verification before satisfying dependency - By("Checking that resource which accepts data isn't created yet") - inFooKey := client.ObjectKey{ - Name: inName, - Namespace: namespace, - } - inFoo := tempFoo.DeepCopy() - logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) - Eventually( - func() error { - return k8sClient.Get(ctx, inFooKey, inFoo) - }, - time.Second*60, time.Second*2).Should(&util.NotFoundMatcher{}) - By("Checking that resource which provides data is created") - outFooKey := client.ObjectKey{ - Name: outName, - Namespace: namespace, - } - outFoo := tempFoo.DeepCopy() - logf.Log.Info("Checking on resource that outputs data", "Key", outFoo) - Eventually( - func() error { - return k8sClient.Get(ctx, outFooKey, outFoo) - }, - time.Second*60, time.Second*2).Should(BeNil()) - By("Verify the appconfig's dependency is unsatisfied") - appconfigKey := client.ObjectKey{ - Name: appConfigName, - Namespace: namespace, - } - appconfig := &v1alpha2.ApplicationConfiguration{} - depStatus := v1alpha2.DependencyStatus{ - Unsatisfied: []v1alpha2.UnstaifiedDependency{ - { - Reason: reason, - From: v1alpha2.DependencyFromObject{ - TypedReference: v1alpha1.TypedReference{ - APIVersion: tempFoo.GetAPIVersion(), - Name: outName, - Kind: tempFoo.GetKind(), - }, - FieldPath: "status.key", - }, - To: v1alpha2.DependencyToObject{ - TypedReference: v1alpha1.TypedReference{ - APIVersion: tempFoo.GetAPIVersion(), - Name: inName, - Kind: tempFoo.GetKind(), - }, - FieldPaths: []string{ - "spec.key", - }, - }, - }, - }, - } - logf.Log.Info("Checking on appconfig", "Key", appconfigKey) - Eventually( - func() v1alpha2.DependencyStatus { - k8sClient.Get(ctx, appconfigKey, appconfig) - return appconfig.Status.Dependency - }, - time.Second*120, time.Second*2).Should(Equal(depStatus)) - // fill value to fieldPath - err := unstructured.SetNestedField(outFoo.Object, "test", "status", "key") - Expect(err).Should(BeNil()) - Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) - // Verification after satisfying dependency - By("Checking that resource which accepts data is created now") - logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) - Eventually( - func() error { - return k8sClient.Get(ctx, inFooKey, inFoo) - }, - time.Second*120, time.Second*2).Should(BeNil()) - By("Verify the appconfig's dependency is satisfied") - appconfig = &v1alpha2.ApplicationConfiguration{} - logf.Log.Info("Checking on appconfig", "Key", appconfigKey) - Eventually( - func() []v1alpha2.UnstaifiedDependency { - k8sClient.Get(ctx, appconfigKey, appconfig) - return appconfig.Status.Dependency.Unsatisfied - }, - time.Second*300, time.Second*2).Should(BeNil()) - } - - It("trait depends on another trait", func() { - label := map[string]string{"trait": "trait"} - // Define a workload - wl := tempFoo.DeepCopy() - wl.SetName("workload") - // Create a component - componentName := "component" - comp := v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Object: wl, - }, - }, - } - Expect(k8sClient.Create(ctx, &comp)).Should(BeNil()) - logf.Log.Info("Creating component", "Name", comp.Name, "Namespace", comp.Namespace) - // Create application configuration - appConfigName := "appconfig-trait-trait" - appConfig := v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: appConfigName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentName, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Object: out, - }, - DataOutputs: []v1alpha2.DataOutput{ - { - Name: "trait-trait", - FieldPath: "status.key", - }, - }, - }, - { - Trait: runtime.RawExtension{ - Object: in, - }, - DataInputs: []v1alpha2.DataInput{ - { - ValueFrom: v1alpha2.DataInputValueFrom{ - DataOutputName: "trait-trait", - }, - ToFieldPaths: []string{"spec.key"}, - }, - }, - }, - }, - }, - }, - }, - } - logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) - Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) - verify(appConfigName, "status.key not found in object") - }) - - It("component depends on another component", func() { - label := map[string]string{"component": "component"} - // Create application configuration - appConfigName := "appconfig-comp-comp" - appConfig := v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: appConfigName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentOutName, - DataOutputs: []v1alpha2.DataOutput{ - { - Name: "comp-comp", - FieldPath: "status.key", - }, - }, - }, - { - ComponentName: componentInName, - DataInputs: []v1alpha2.DataInput{ - { - ValueFrom: v1alpha2.DataInputValueFrom{ - DataOutputName: "comp-comp", - }, - ToFieldPaths: []string{"spec.key"}, - }, - }, - }, - }, - }, - } - logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) - Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) - verify(appConfigName, "status.key not found in object") - }) - - It("component depends on trait", func() { - label := map[string]string{"trait": "component"} - // Create application configuration - appConfigName := "appconfig-trait-comp" - appConfig := v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: appConfigName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentInName, - DataInputs: []v1alpha2.DataInput{ - { - ValueFrom: v1alpha2.DataInputValueFrom{ - DataOutputName: "trait-comp", - }, - ToFieldPaths: []string{"spec.key"}, - }, - }, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Object: out, - }, - DataOutputs: []v1alpha2.DataOutput{ - { - Name: "trait-comp", - FieldPath: "status.key", - }, - }, - }, - }, - }, - }, - }, - } - logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) - Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) - verify(appConfigName, "status.key not found in object") - }) - - It("trait depends on component", func() { - label := map[string]string{"component": "trait"} - // Create application configuration - appConfigName := "appconfig-comp-trait" - appConfig := v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: appConfigName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentOutName, - DataOutputs: []v1alpha2.DataOutput{ - { - Name: "comp-trait", - FieldPath: "status.key", - }, - }, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Object: in, - }, - DataInputs: []v1alpha2.DataInput{ - { - ValueFrom: v1alpha2.DataInputValueFrom{ - DataOutputName: "comp-trait", - }, - ToFieldPaths: []string{"spec.key"}, - }, - }, - }, - }, - }, - }, - }, - } - logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) - Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) - verify(appConfigName, "status.key not found in object") - }) - - It("component depends on trait with updated condition", func() { - label := map[string]string{"trait": "component", "app-hash": "hash-v1"} - // Create application configuration - appConfigName := "appconfig-trait-comp" - appConfig := v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: appConfigName, - Namespace: namespace, - Labels: label, - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentInName, - DataInputs: []v1alpha2.DataInput{ - { - ValueFrom: v1alpha2.DataInputValueFrom{ - DataOutputName: "trait-comp", - }, - ToFieldPaths: []string{"spec.key"}, - }, - }, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Object: out, - }, - DataOutputs: []v1alpha2.DataOutput{ - { - Name: "trait-comp", - FieldPath: "status.key", - Conditions: []v1alpha2.ConditionRequirement{ - { - Operator: v1alpha2.ConditionEqual, - ValueFrom: v1alpha2.ValueFrom{FieldPath: "metadata.labels.app-hash"}, - FieldPath: "status.app-hash", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) - Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) - By("Checking that resource which accepts data isn't created yet") - inFooKey := client.ObjectKey{ - Name: inName, - Namespace: namespace, - } - inFoo := tempFoo.DeepCopy() - logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) - Eventually( - func() error { - return k8sClient.Get(ctx, inFooKey, inFoo) - }, - time.Second*60, time.Second*2).Should(&util.NotFoundMatcher{}) - - By("Checking that resource which provides data is created") - // Originally the trait has value in `status.key`, but the hash label is old - outFooKey := client.ObjectKey{ - Name: outName, - Namespace: namespace, - } - outFoo := tempFoo.DeepCopy() - Eventually( - func() error { - return k8sClient.Get(ctx, outFooKey, outFoo) - }, - time.Second*60, time.Second*2).Should(BeNil()) - err := unstructured.SetNestedField(outFoo.Object, "test", "status", "key") - Expect(err).Should(BeNil()) - err = unstructured.SetNestedField(outFoo.Object, "hash-v1", "status", "app-hash") - Expect(err).Should(BeNil()) - Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) - - appconfigKey := client.ObjectKey{ - Name: appConfigName, - Namespace: namespace, - } - newAppConfig := &v1alpha2.ApplicationConfiguration{} - - // Verification after satisfying dependency - By("Verify the appconfig's dependency is satisfied") - newAppConfig = &v1alpha2.ApplicationConfiguration{} - logf.Log.Info("Checking on appconfig", "Key", appconfigKey) - Eventually( - func() []v1alpha2.UnstaifiedDependency { - var tempApp = &v1alpha2.ApplicationConfiguration{} - k8sClient.Get(ctx, appconfigKey, tempApp) - tempApp.DeepCopyInto(newAppConfig) - return tempApp.Status.Dependency.Unsatisfied - }, - time.Second*80, time.Second*2).Should(BeNil()) - By("Checking that resource which accepts data is created now") - logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) - Eventually( - func() error { - return k8sClient.Get(ctx, inFooKey, inFoo) - }, - time.Second*80, time.Second*2).Should(BeNil()) - - newAppConfig.Labels["app-hash"] = "hash-v2" - Expect(k8sClient.Update(ctx, newAppConfig)).Should(BeNil()) - - By("Verify the appconfig's dependency should be unsatisfied, because requirementCondition valueFrom not match") - - depStatus := v1alpha2.DependencyStatus{ - Unsatisfied: []v1alpha2.UnstaifiedDependency{ - { - Reason: "got(hash-v1) expected to be hash-v2", - From: v1alpha2.DependencyFromObject{ - TypedReference: v1alpha1.TypedReference{ - APIVersion: tempFoo.GetAPIVersion(), - Name: outName, - Kind: tempFoo.GetKind(), - }, - FieldPath: "status.key", - }, - To: v1alpha2.DependencyToObject{ - TypedReference: v1alpha1.TypedReference{ - APIVersion: tempFoo.GetAPIVersion(), - Name: inName, - Kind: tempFoo.GetKind(), - }, - FieldPaths: []string{ - "spec.key", - }, - }, - }, - }, - } - logf.Log.Info("Checking on appconfig", "Key", appconfigKey) - Eventually( - func() v1alpha2.DependencyStatus { - k8sClient.Get(ctx, appconfigKey, newAppConfig) - return newAppConfig.Status.Dependency - }, - time.Second*60, time.Second*2).Should(Equal(depStatus)) - - // Update trait object - k8sClient.Get(ctx, outFooKey, outFoo) // Get the latest before update - err = unstructured.SetNestedField(outFoo.Object, "test-new", "status", "key") - Expect(err).Should(BeNil()) - err = unstructured.SetNestedField(outFoo.Object, "hash-v2", "status", "app-hash") - Expect(err).Should(BeNil()) - Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed()) - - // Verification after satisfying dependency - By("Checking that resource which accepts data is updated now") - logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey) - Eventually( - func() string { - k8sClient.Get(ctx, inFooKey, inFoo) - outdata, _, _ := unstructured.NestedString(inFoo.Object, "spec", "key") - return outdata - }, - time.Second*80, time.Second*2).Should(Equal("test-new")) - By("Verify the appconfig's dependency is satisfied") - logf.Log.Info("Checking on appconfig", "Key", appconfigKey) - Eventually( - func() []v1alpha2.UnstaifiedDependency { - tempAppConfig := &v1alpha2.ApplicationConfiguration{} - k8sClient.Get(ctx, appconfigKey, tempAppConfig) - return tempAppConfig.Status.Dependency.Unsatisfied - }, - time.Second*80, time.Second*2).Should(BeNil()) - }) -}) diff --git a/test/e2e-test/appconfig_finalizer_test.go b/test/e2e-test/appconfig_finalizer_test.go index d5f6ad2b..e62fdf40 100644 --- a/test/e2e-test/appconfig_finalizer_test.go +++ b/test/e2e-test/appconfig_finalizer_test.go @@ -101,7 +101,7 @@ var _ = Describe("Finalizer for HealthScope in ApplicationConfiguration", func() func() error { return k8sClient.Get(ctx, objectKey, res) }, - time.Second*60, time.Millisecond*500).Should(&util.NotFoundMatcher{}) + time.Second*120, time.Millisecond*500).Should(&util.NotFoundMatcher{}) Eventually( func() error { return k8sClient.Create(ctx, &ns) diff --git a/test/e2e-test/component_version_test.go b/test/e2e-test/component_version_test.go index c0d852e4..39510e2d 100644 --- a/test/e2e-test/component_version_test.go +++ b/test/e2e-test/component_version_test.go @@ -134,7 +134,7 @@ var _ = Describe("Versioning mechanism of components", func() { func() error { return k8sClient.Get(ctx, objectKey, res) }, - time.Second*60, time.Millisecond*500).Should(&util.NotFoundMatcher{}) + time.Second*120, time.Millisecond*500).Should(&util.NotFoundMatcher{}) Eventually( func() error { return k8sClient.Create(ctx, &ns) @@ -334,7 +334,7 @@ var _ = Describe("Versioning mechanism of components", func() { k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &comp1) return comp1.Status.LatestRevision }, - time.Second*30, time.Millisecond*500).ShouldNot(BeNil()) + time.Second*120, time.Millisecond*500).ShouldNot(BeNil()) revisionNameV1 := comp1.Status.LatestRevision.Name @@ -365,7 +365,7 @@ var _ = Describe("Versioning mechanism of components", func() { } return nil }, - time.Second*30, time.Millisecond*500).ShouldNot(BeNil()) + time.Second*120, time.Millisecond*500).ShouldNot(BeNil()) revisionNameV2 := comp2.Status.LatestRevision.Name @@ -377,7 +377,7 @@ var _ = Describe("Versioning mechanism of components", func() { w2.SetKind("Bar") return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: revisionNameV2}, &w2) }, - time.Second*15, time.Millisecond*500).Should(BeNil()) + time.Second*60, time.Millisecond*500).Should(BeNil()) k2, _, _ := unstructured.NestedString(w2.Object, "spec", "key") Expect(k2).Should(BeEquivalentTo("v2"), fmt.Sprintf("%v", w2.Object)) @@ -436,7 +436,7 @@ var _ = Describe("Versioning mechanism of components", func() { w1.SetKind("Bar") return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &w1) }, - time.Second*15, time.Millisecond*500).Should(BeNil()) + time.Second*60, time.Millisecond*500).Should(BeNil()) k1, _, _ := unstructured.NestedString(w1.Object, "spec", "key") Expect(k1).Should(BeEquivalentTo("v1"), fmt.Sprintf("%v", w1.Object)) @@ -461,7 +461,7 @@ var _ = Describe("Versioning mechanism of components", func() { k2, _, _ := unstructured.NestedString(w2.Object, "spec", "key") return k2 }, - time.Second*15, time.Millisecond*500).Should(BeEquivalentTo("v2")) + time.Second*120, time.Millisecond*500).Should(BeEquivalentTo("v2")) By("Check AppConfig status") Eventually( diff --git a/test/e2e-test/containerized_workload_test.go b/test/e2e-test/containerized_workload_test.go index 8f4c2b39..ea49e56f 100644 --- a/test/e2e-test/containerized_workload_test.go +++ b/test/e2e-test/containerized_workload_test.go @@ -60,7 +60,7 @@ var _ = Describe("ContainerizedWorkload", func() { func() error { return k8sClient.Get(ctx, objectKey, res) }, - time.Second*30, time.Millisecond*500).Should(&util.NotFoundMatcher{}) + time.Second*120, time.Millisecond*500).Should(&util.NotFoundMatcher{}) // recreate it Eventually( func() error { diff --git a/test/e2e-test/health_scope_test.go b/test/e2e-test/health_scope_test.go index 8906f832..143cbda4 100644 --- a/test/e2e-test/health_scope_test.go +++ b/test/e2e-test/health_scope_test.go @@ -49,7 +49,7 @@ var _ = Describe("HealthScope", func() { func() error { return k8sClient.Get(ctx, objectKey, res) }, - time.Second*30, time.Millisecond*500).Should(&util.NotFoundMatcher{}) + time.Second*120, time.Millisecond*500).Should(&util.NotFoundMatcher{}) // recreate it Eventually( func() error { diff --git a/test/e2e-test/kubernetes_workload_test.go b/test/e2e-test/kubernetes_workload_test.go index 7201ee86..9395e7f3 100644 --- a/test/e2e-test/kubernetes_workload_test.go +++ b/test/e2e-test/kubernetes_workload_test.go @@ -43,7 +43,7 @@ var _ = Describe("Test kubernetes native workloads", func() { func() error { return k8sClient.Get(ctx, objectKey, res) }, - time.Second*30, time.Millisecond*500).Should(&util.NotFoundMatcher{}) + time.Second*120, time.Millisecond*500).Should(&util.NotFoundMatcher{}) // recreate it Eventually( func() error { diff --git a/test/e2e-test/suite_test.go b/test/e2e-test/suite_test.go index 715793cc..d7d9be5b 100644 --- a/test/e2e-test/suite_test.go +++ b/test/e2e-test/suite_test.go @@ -197,52 +197,6 @@ var _ = BeforeSuite(func(done Done) { } Expect(k8sClient.Create(context.Background(), &adminRoleBinding)).Should(BeNil()) By("Created cluster role binding for the test service account") - // Create a crd for appconfig dependency test - crd = crdv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo.example.com", - Labels: map[string]string{"crd": "dependency"}, - }, - Spec: crdv1.CustomResourceDefinitionSpec{ - Group: "example.com", - Names: crdv1.CustomResourceDefinitionNames{ - Kind: "Foo", - ListKind: "FooList", - Plural: "foo", - Singular: "foo", - }, - Versions: []crdv1.CustomResourceDefinitionVersion{ - { - Name: "v1", - Served: true, - Storage: true, - Schema: &crdv1.CustomResourceValidation{ - OpenAPIV3Schema: &crdv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]crdv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]crdv1.JSONSchemaProps{ - "key": {Type: "string"}, - }, - }, - "status": { - Type: "object", - Properties: map[string]crdv1.JSONSchemaProps{ - "key": {Type: "string"}, - "app-hash": {Type: "string"}, - }, - }, - }, - }, - }, - }, - }, - Scope: crdv1.NamespaceScoped, - }, - } - Expect(k8sClient.Create(context.Background(), &crd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) - By("Created a crd for appconfig dependency test") crd = crdv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -301,22 +255,6 @@ var _ = AfterSuite(func() { } Expect(k8sClient.Delete(context.Background(), &adminRoleBinding)).Should(BeNil()) By("Deleted the cluster role binding") - manualscalertrait = v1alpha2.TraitDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "manualscalertraits.core.oam.dev", - Labels: map[string]string{"trait": "manualscalertrait"}, - }, - } - Expect(k8sClient.Delete(context.Background(), &manualscalertrait)).Should(BeNil()) - Expect(k8sClient.Delete(context.Background(), &extendedmanualscalertrait)).Should(BeNil()) - By("Deleted the manual scalertrait definition") - crd = crdv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo.example.com", - Labels: map[string]string{"crd": "dependency"}, - } - } - Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) crd = crdv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -327,18 +265,19 @@ var _ = AfterSuite(func() { Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) By("Deleted the custom resource definition") - td := v1alpha2.TraitDefinition{ + crd = crdv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ - Name: "bars.example.com", + Name: "workloaddefinitions.core.oam.dev", }, } - k8sClient.Delete(context.Background(), &td) + Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) + By("Deleted the workloaddefinitions CRD") - wd := v1alpha2.WorkloadDefinition{ + crd = crdv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ - Name: "bars.example.com", + Name: "traitdefinitions.core.oam.dev", }, } - k8sClient.Delete(context.Background(), &wd) - + Expect(k8sClient.Delete(context.Background(), &crd)).Should(BeNil()) + By("Deleted the workloaddefinitions CRD") })