From 6bf85f6f6ec9dc6dc98170b5cacc93cc33ad5fa9 Mon Sep 17 00:00:00 2001 From: roywang Date: Mon, 8 Mar 2021 21:15:56 +0900 Subject: [PATCH] use klog/v2 as logger in util/applicator Signed-off-by: roywang --- .../applicationconfiguration.go | 2 +- pkg/utils/apply/apply.go | 31 +++++++++---------- pkg/utils/apply/apply_suite_test.go | 3 +- pkg/utils/apply/apply_test.go | 8 ++--- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/applicationconfiguration.go index 180fa5e92..e5155ed5f 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -189,7 +189,7 @@ func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, log loggi trait: ResourceRenderFn(renderTrait), }, workloads: &workloads{ - applicator: apply.NewAPIApplicator(m.GetClient(), log), + applicator: apply.NewAPIApplicator(m.GetClient()), rawClient: m.GetClient(), dm: dm, }, diff --git a/pkg/utils/apply/apply.go b/pkg/utils/apply/apply.go index 5ab8f0b3d..789bc5cd8 100644 --- a/pkg/utils/apply/apply.go +++ b/pkg/utils/apply/apply.go @@ -3,15 +3,16 @@ package apply import ( "context" - "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/api/errors" 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/types" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/oam-dev/kubevela/pkg/controller/common" "github.com/oam-dev/kubevela/pkg/oam" ) @@ -32,23 +33,22 @@ type ApplyOption func(ctx context.Context, existing, desired runtime.Object) err // NewAPIApplicator creates an Applicator that applies state to an // object or creates the object if not exist. -func NewAPIApplicator(c client.Client, log logging.Logger) *APIApplicator { +func NewAPIApplicator(c client.Client) *APIApplicator { return &APIApplicator{ creator: creatorFn(createOrGetExisting), patcher: patcherFn(threeWayMergePatch), c: c, - log: log, } } type creator interface { - createOrGetExisting(context.Context, logging.Logger, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error) + createOrGetExisting(context.Context, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error) } -type creatorFn func(context.Context, logging.Logger, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error) +type creatorFn func(context.Context, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error) -func (fn creatorFn) createOrGetExisting(ctx context.Context, log logging.Logger, c client.Client, o runtime.Object, ao ...ApplyOption) (runtime.Object, error) { - return fn(ctx, log, c, o, ao...) +func (fn creatorFn) createOrGetExisting(ctx context.Context, c client.Client, o runtime.Object, ao ...ApplyOption) (runtime.Object, error) { + return fn(ctx, c, o, ao...) } type patcher interface { @@ -65,23 +65,22 @@ func (fn patcherFn) patch(c, m runtime.Object) (client.Patch, error) { type APIApplicator struct { creator patcher - c client.Client - log logging.Logger + c client.Client } // loggingApply will record a log with desired object applied -func loggingApply(log logging.Logger, msg string, desired runtime.Object) { +func loggingApply(msg string, desired runtime.Object) { d, ok := desired.(metav1.Object) if !ok { - log.Debug(msg, "resource", desired.GetObjectKind().GroupVersionKind().String()) + klog.V(common.LogDebug).InfoS(msg, "resource", desired.GetObjectKind().GroupVersionKind().String()) return } - log.Debug(msg, "name", d.GetName(), "resource", desired.GetObjectKind().GroupVersionKind().String()) + klog.V(common.LogDebug).InfoS(msg, "name", d.GetName(), "resource", desired.GetObjectKind().GroupVersionKind().String()) } // Apply applies new state to an object or create it if not exist func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao ...ApplyOption) error { - existing, err := a.createOrGetExisting(ctx, a.log, a.c, desired, ao...) + existing, err := a.createOrGetExisting(ctx, a.c, desired, ao...) if err != nil { return err } @@ -93,7 +92,7 @@ func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao .. if err := executeApplyOptions(ctx, existing, desired, ao); err != nil { return err } - loggingApply(a.log, "patching object", desired) + loggingApply("patching object", desired) patch, err := a.patcher.patch(existing, desired) if err != nil { return errors.Wrap(err, "cannot calculate patch by computing a three way diff") @@ -103,7 +102,7 @@ func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao .. // createOrGetExisting will create the object if it does not exist // or get and return the existing object -func createOrGetExisting(ctx context.Context, log logging.Logger, c client.Client, desired runtime.Object, ao ...ApplyOption) (runtime.Object, error) { +func createOrGetExisting(ctx context.Context, c client.Client, desired runtime.Object, ao ...ApplyOption) (runtime.Object, error) { m, ok := desired.(oam.Object) if !ok { return nil, errors.New("cannot access object metadata") @@ -117,7 +116,7 @@ func createOrGetExisting(ctx context.Context, log logging.Logger, c client.Clien if err := addLastAppliedConfigAnnotation(desired); err != nil { return nil, err } - loggingApply(log, "creating object", desired) + loggingApply("creating object", desired) return nil, errors.Wrap(c.Create(ctx, desired), "cannot create object") } diff --git a/pkg/utils/apply/apply_suite_test.go b/pkg/utils/apply/apply_suite_test.go index 4daf8d283..af85df3a6 100644 --- a/pkg/utils/apply/apply_suite_test.go +++ b/pkg/utils/apply/apply_suite_test.go @@ -11,7 +11,6 @@ import ( oamcore "github.com/oam-dev/kubevela/apis/core.oam.dev" oamstd "github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1" - "github.com/crossplane/crossplane-runtime/pkg/logging" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -57,7 +56,7 @@ var _ = BeforeSuite(func(done Done) { rawClient, err = client.New(cfg, client.Options{Scheme: testScheme}) Expect(err).ShouldNot(HaveOccurred()) Expect(rawClient).ShouldNot(BeNil()) - k8sApplicator = NewAPIApplicator(rawClient, logging.NewNopLogger()) + k8sApplicator = NewAPIApplicator(rawClient) By("Create test namespace") applyNS = corev1.Namespace{ diff --git a/pkg/utils/apply/apply_test.go b/pkg/utils/apply/apply_test.go index 570fe3406..cf545c712 100644 --- a/pkg/utils/apply/apply_test.go +++ b/pkg/utils/apply/apply_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" @@ -119,14 +118,13 @@ func TestAPIApplicator(t *testing.T) { for caseName, tc := range cases { t.Run(caseName, func(t *testing.T) { a := &APIApplicator{ - creator: creatorFn(func(_ context.Context, _ logging.Logger, _ client.Client, _ runtime.Object, _ ...ApplyOption) (runtime.Object, error) { + creator: creatorFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...ApplyOption) (runtime.Object, error) { return tc.args.existing, tc.args.creatorErr }), patcher: patcherFn(func(c, m runtime.Object) (client.Patch, error) { return nil, tc.args.patcherErr }), - c: tc.c, - log: logging.NewNopLogger(), + c: tc.c, } result := a.Apply(ctx, tc.args.desired, tc.args.ao...) if diff := cmp.Diff(tc.want, result, test.EquateErrors()); diff != "" { @@ -287,7 +285,7 @@ func TestCreator(t *testing.T) { for caseName, tc := range cases { t.Run(caseName, func(t *testing.T) { - result, err := createOrGetExisting(ctx, logging.NewNopLogger(), tc.c, tc.args.desired, tc.args.ao...) + result, err := createOrGetExisting(ctx, tc.c, tc.args.desired, tc.args.ao...) if diff := cmp.Diff(tc.want.existing, result); diff != "" { t.Errorf("\n%s\ncreateOrGetExisting(...): -want , +got \n%s\n", tc.reason, diff) }