diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go index 86d5c617..d6acf488 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -30,7 +30,6 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - clientappv1 "k8s.io/client-go/kubernetes/typed/apps/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -79,9 +78,8 @@ func Setup(mgr ctrl.Manager, l logging.Logger) error { Named(name). For(&v1alpha2.ApplicationConfiguration{}). Watches(&source.Kind{Type: &v1alpha2.Component{}}, &ComponentHandler{ - Client: mgr.GetClient(), - Logger: l, - AppsClient: clientappv1.NewForConfigOrDie(mgr.GetConfig()), + Client: mgr.GetClient(), + Logger: l, }). Complete(NewReconciler(mgr, WithLogger(l.WithValues("controller", name)), @@ -163,11 +161,10 @@ func NewReconciler(m ctrl.Manager, o ...ReconcilerOption) *OAMApplicationReconci client: m.GetClient(), scheme: m.GetScheme(), components: &components{ - client: m.GetClient(), - appsClient: clientappv1.NewForConfigOrDie(m.GetConfig()), - params: ParameterResolveFn(resolve), - workload: ResourceRenderFn(renderWorkload), - trait: ResourceRenderFn(renderTrait), + client: m.GetClient(), + params: ParameterResolveFn(resolve), + workload: ResourceRenderFn(renderWorkload), + trait: ResourceRenderFn(renderTrait), }, workloads: &workloads{ client: resource.NewAPIPatchingApplicator(m.GetClient()), diff --git a/pkg/controller/v1alpha2/applicationconfiguration/component.go b/pkg/controller/v1alpha2/applicationconfiguration/component.go index c631e2a2..e53b9906 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/component.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/component.go @@ -14,7 +14,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - clientappv1 "k8s.io/client-go/kubernetes/typed/apps/v1" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -25,9 +24,8 @@ import ( // ComponentHandler will watch component change and generate Revision automatically. type ComponentHandler struct { - Client client.Client - AppsClient clientappv1.AppsV1Interface - Logger logging.Logger + Client client.Client + Logger logging.Logger } // Create implements EventHandler @@ -100,11 +98,18 @@ func (c *ComponentHandler) IsRevisionDiff(mt metav1.Object, curComp *v1alpha2.Co if curComp.Status.LatestRevision == nil { return true, 0 } - oldRev, err := c.AppsClient.ControllerRevisions(mt.GetNamespace()).Get(context.Background(), curComp.Status.LatestRevision.Name, metav1.GetOptions{}) - if err != nil { + + // client in controller-runtime will use infoermer cache + // use client will be more efficient + oldRev := &appsv1.ControllerRevision{} + if err := c.Client.Get(context.TODO(), client.ObjectKey{Namespace: mt.GetNamespace(), Name: curComp.Status.LatestRevision.Name}, oldRev); err != nil { c.Logger.Info(fmt.Sprintf("get old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision.Name, err), "componentName", mt.GetName()) return true, curComp.Status.LatestRevision.Revision } + if oldRev.Name == "" { + c.Logger.Info(fmt.Sprintf("Not found controllerRevision %s", curComp.Status.LatestRevision.Name), "componentName", mt.GetName()) + return true, curComp.Status.LatestRevision.Revision + } oldComp, err := UnpackRevisionData(oldRev) if err != nil { c.Logger.Info(fmt.Sprintf("Unmarshal old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision.Name, err), "componentName", mt.GetName()) @@ -168,7 +173,7 @@ func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtim Revision: nextRevision, Data: runtime.RawExtension{Object: curComp}, } - _, err := c.AppsClient.ControllerRevisions(mt.GetNamespace()).Create(context.Background(), &revision, metav1.CreateOptions{}) + err := c.Client.Create(context.TODO(), &revision) if err != nil { c.Logger.Info(fmt.Sprintf("error create controllerRevision %v", err), "componentName", mt.GetName()) return false diff --git a/pkg/controller/v1alpha2/applicationconfiguration/component_test.go b/pkg/controller/v1alpha2/applicationconfiguration/component_test.go index c2229e98..4082ce14 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/component_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/component_test.go @@ -16,7 +16,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllertest" @@ -28,39 +27,72 @@ import ( func TestComponentHandler(t *testing.T) { q := controllertest.Queue{Interface: workqueue.New()} - fakeAppClient := fake.NewSimpleClientset().AppsV1() var curComp = &v1alpha2.Component{} + var createdRevisions = []appsv1.ControllerRevision{} var instance = ComponentHandler{ Client: &test.MockClient{ MockList: test.NewMockListFn(nil, func(obj runtime.Object) error { - lists := v1alpha2.ApplicationConfigurationList{ - Items: []v1alpha2.ApplicationConfiguration{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "app1", - }, - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{{ - ComponentName: "comp1", - }}, + switch robj := obj.(type) { + case *v1alpha2.ApplicationConfigurationList: + lists := v1alpha2.ApplicationConfigurationList{ + Items: []v1alpha2.ApplicationConfiguration{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "app1", + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{{ + ComponentName: "comp1", + }}, + }, }, }, - }, + } + lists.DeepCopyInto(robj) + return nil + case *appsv1.ControllerRevisionList: + lists := appsv1.ControllerRevisionList{ + Items: createdRevisions, + } + lists.DeepCopyInto(robj) } - lBytes, _ := json.Marshal(lists) - json.Unmarshal(lBytes, obj) return nil }), MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(obj runtime.Object) error { - cur, ok := obj.(*v1alpha2.Component) + switch robj := obj.(type) { + case *v1alpha2.Component: + robj.DeepCopyInto(curComp) + case *appsv1.ControllerRevision: + for _, revision := range createdRevisions { + if revision.Name == robj.Name { + robj.DeepCopyInto(&revision) + } + } + } + return nil + }), + MockCreate: test.NewMockCreateFn(nil, func(obj runtime.Object) error { + cur, ok := obj.(*appsv1.ControllerRevision) if ok { - cur.DeepCopyInto(curComp) + createdRevisions = append(createdRevisions, *cur) + } + return nil + }), + MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error { + switch robj := obj.(type) { + case *appsv1.ControllerRevision: + if len(createdRevisions) == 0 { + return nil + } + // test frame can't get the key, and just return the newest revision + createdRevisions[len(createdRevisions)-1].DeepCopyInto(robj) + case *v1alpha2.Component: + robj.DeepCopyInto(curComp) } return nil }), }, - AppsClient: fakeAppClient, - Logger: logging.NewLogrLogger(ctrl.Log.WithName("test")), + Logger: logging.NewLogrLogger(ctrl.Log.WithName("test")), } comp := &v1alpha2.Component{ ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "comp1"}, @@ -80,7 +112,8 @@ func TestComponentHandler(t *testing.T) { req := item.(reconcile.Request) // AppConfig event triggered, and compare revision created assert.Equal(t, req.Name, "app1") - revisions, err := fakeAppClient.ControllerRevisions("biz").List(context.Background(), metav1.ListOptions{}) + revisions := &appsv1.ControllerRevisionList{} + err := instance.Client.List(context.TODO(), revisions) assert.NoError(t, err) assert.Equal(t, 1, len(revisions.Items)) assert.Equal(t, true, strings.HasPrefix(revisions.Items[0].Name, "comp1-")) @@ -114,7 +147,8 @@ func TestComponentHandler(t *testing.T) { req = item.(reconcile.Request) // AppConfig event triggered, and compare revision created assert.Equal(t, req.Name, "app1") - revisions, err = fakeAppClient.ControllerRevisions("biz").List(context.Background(), metav1.ListOptions{}) + revisions = &appsv1.ControllerRevisionList{} + err = instance.Client.List(context.TODO(), revisions) assert.NoError(t, err) // Component changed, we have two revision now. assert.Equal(t, 2, len(revisions.Items)) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index b66ac09c..d54089fb 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -25,12 +25,12 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" + appsv1 "k8s.io/api/apps/v1" 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/types" "k8s.io/apimachinery/pkg/util/intstr" - clientappv1 "k8s.io/client-go/kubernetes/typed/apps/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" @@ -81,11 +81,10 @@ func (fn ComponentRenderFn) Render(ctx context.Context, ac *v1alpha2.Application } type components struct { - client client.Reader - appsClient clientappv1.AppsV1Interface - params ParameterResolver - workload ResourceRenderer - trait ResourceRenderer + client client.Reader + params ParameterResolver + workload ResourceRenderer + trait ResourceRenderer } func (r *components) Render(ctx context.Context, ac *v1alpha2.ApplicationConfiguration) ([]Workload, *v1alpha2.DependencyStatus, error) { @@ -251,10 +250,13 @@ func (r *components) getComponent(ctx context.Context, acc v1alpha2.ApplicationC c := &v1alpha2.Component{} var revisionName string if acc.RevisionName != "" { - revision, err := r.appsClient.ControllerRevisions(namespace).Get(ctx, acc.RevisionName, metav1.GetOptions{}) - if err != nil { + revision := &appsv1.ControllerRevision{} + if err := r.client.Get(context.TODO(), client.ObjectKey{Namespace: namespace, Name: acc.RevisionName}, revision); err != nil { return nil, "", errors.Wrapf(err, errFmtGetComponentRevision, acc.RevisionName) } + if revision.Name == "" { + return nil, "", fmt.Errorf("Not found revision %s", acc.RevisionName) + } c, err := UnpackRevisionData(revision) if err != nil { return nil, "", errors.Wrapf(err, errFmtControllerRevisionData, acc.RevisionName) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 3b6a74de..92d6f7ad 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -35,9 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes/fake" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - clientappv1 "k8s.io/client-go/kubernetes/typed/apps/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core" @@ -89,28 +87,13 @@ func TestRenderComponents(t *testing.T) { }, }, } - fakeAppClient := fake.NewSimpleClientset().AppsV1() - fakeAppClient.ControllerRevisions(namespace).Create(context.Background(), &v1.ControllerRevision{ - ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, - Data: runtime.RawExtension{Object: &v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, - Status: v1alpha2.ComponentStatus{}, - }}, - Revision: 1, - }, metav1.CreateOptions{}) - ref := metav1.NewControllerRef(ac, v1alpha2.ApplicationConfigurationGroupVersionKind) type fields struct { - client client.Reader - appclient clientappv1.AppsV1Interface - params ParameterResolver - workload ResourceRenderer - trait ResourceRenderer + client client.Reader + params ParameterResolver + workload ResourceRenderer + trait ResourceRenderer } type args struct { ctx context.Context @@ -231,8 +214,26 @@ func TestRenderComponents(t *testing.T) { "Success-With-RevisionName": { reason: "Workload should successfully be rendered with fixed componentRevision", fields: fields{ - client: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, - appclient: fakeAppClient, + client: &test.MockClient{MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error { + robj, ok := obj.(*v1.ControllerRevision) + if ok { + rev := &v1.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, + Data: runtime.RawExtension{Object: &v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, + Status: v1alpha2.ComponentStatus{}, + }}, + Revision: 1, + } + rev.DeepCopyInto(robj) + return nil + } + return nil + })}, params: ParameterResolveFn(func(_ []v1alpha2.ComponentParameter, _ []v1alpha2.ComponentParameterValue) ([]Parameter, error) { return nil, nil }), @@ -330,7 +331,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.appclient, tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, 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) @@ -676,7 +677,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := &components{tc.fields.client, nil, tc.fields.params, tc.fields.workload, tc.fields.trait} + r := &components{tc.fields.client, 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"+ @@ -825,37 +826,18 @@ func TestSetTraitProperties(t *testing.T) { func TestGetComponent(t *testing.T) { type Fields struct { - client client.Reader - appclient clientappv1.AppsV1Interface - params ParameterResolver - workload ResourceRenderer - trait ResourceRenderer + client client.Reader + params ParameterResolver + workload ResourceRenderer + trait ResourceRenderer } namespace := "ns" componentName := "newcomponent" revisionName := "newcomponent-aa1111" revisionName2 := "newcomponent-bb1111" - fakeAppClient := fake.NewSimpleClientset().AppsV1() - fakeAppClient.ControllerRevisions(namespace).Create(context.Background(), &v1.ControllerRevision{ - ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, - Data: runtime.RawExtension{Object: &v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: componentName, - Namespace: namespace, - }, - Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, - Status: v1alpha2.ComponentStatus{}, - }}, - Revision: 1, - }, metav1.CreateOptions{}) var fields = Fields{ client: &test.MockClient{MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error { - objc, ok := obj.(*v1alpha2.Component) - if !ok { - return nil - } - c := &v1alpha2.Component{ ObjectMeta: metav1.ObjectMeta{ Name: componentName, @@ -870,10 +852,33 @@ func TestGetComponent(t *testing.T) { LatestRevision: &v1alpha2.Revision{Name: revisionName2, Revision: 2}, }, } - c.DeepCopyInto(objc) + + rev := &v1.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace}, + Data: runtime.RawExtension{Object: &v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}}, + Status: v1alpha2.ComponentStatus{}, + }}, + Revision: 1, + } + + objc, ok := obj.(*v1alpha2.Component) + if ok { + c.DeepCopyInto(objc) + return nil + } + objr, ok := obj.(*v1.ControllerRevision) + if ok { + rev.DeepCopyInto(objr) + return nil + } + return nil })}, - appclient: fakeAppClient, params: ParameterResolveFn(func(_ []v1alpha2.ComponentParameter, _ []v1alpha2.ComponentParameterValue) ([]Parameter, error) { return nil, nil }), @@ -882,7 +887,7 @@ func TestGetComponent(t *testing.T) { return w, nil }), } - r := &components{fields.client, fields.appclient, fields.params, fields.workload, fields.trait} + r := &components{fields.client, fields.params, fields.workload, fields.trait} c, revision, err := r.getComponent(context.Background(), v1alpha2.ApplicationConfigurationComponent{RevisionName: revisionName}, namespace) assert.NoError(t, err) assert.Equal(t, revisionName, revision)