Resolve comments

Signed-off-by: Jian Qiu <[email protected]>

Author: qiujian16

pkg/work/helper/helpers.go

@@ -190,8 +190,8 @@ func DeleteAppliedResources(
 			Namespace(resource.Namespace).
 			Get(ctx, resource.Name, metav1.GetOptions{})
 		if errors.IsNotFound(err) {
-			logger.Info("Resource is removed Successfully",
-				"gvr", gvr, "resourceNamespace", resource.Namespace, "resourceName", resource.Name)
+			logger.Info("Resource is removed successfully",
+				"gvr", gvr.String(), "resourceNamespace", resource.Namespace, "resourceName", resource.Name)
 			continue
 		}
 
@@ -258,7 +258,7 @@ func DeleteAppliedResources(
 
 		resourcesPendingFinalization = append(resourcesPendingFinalization, resource)
 		logger.Info("Deleted resource",
-			"gvr", gvr, "resourceNamespace", resource.Namespace, "resourceName", resource.Name, "reason", reason)
+			"gvr", gvr.String(), "resourceNamespace", resource.Namespace, "resourceName", resource.Name, "reason", reason)
 	}
 
 	return resourcesPendingFinalization, errs
@@ -437,7 +437,7 @@ func ApplyOwnerReferences(ctx context.Context, dynamicClient dynamic.Interface,
 	}
 
 	logger.V(2).Info("Patching resource",
-		"gvr", gvr, "resourceNamespace", accessor.GetNamespace(), "resourceName", accessor.GetName(), "patch", string(patchData))
+		"gvr", gvr.String(), "resourceNamespace", accessor.GetNamespace(), "resourceName", accessor.GetName(), "patch", string(patchData))
 	_, err = dynamicClient.Resource(gvr).Namespace(accessor.GetNamespace()).Patch(ctx, accessor.GetName(), types.MergePatchType, patchData, metav1.PatchOptions{})
 	return err
 }

pkg/work/spoke/apply/create_only_apply.go

@@ -44,7 +44,7 @@ func (c *CreateOnlyApply) Apply(ctx context.Context,
 			ctx, resourcemerge.WithCleanLabelsAndAnnotations(required).(*unstructured.Unstructured), metav1.CreateOptions{})
 		if err != nil {
 			logger.Info("Resource created because of missing",
-				"kind", required.GetKind(), "resourceNamespace", required.GetNamespace(), "resourceName", required.GetName())
+				"gvr", gvr.String(), "resourceNamespace", required.GetNamespace(), "resourceName", required.GetName())
 		}
 	}
 

pkg/work/spoke/apply/read_only_apply.go

@@ -20,13 +20,13 @@ func NewReadOnlyApply() *ReadOnlyApply {
 }
 
 func (c *ReadOnlyApply) Apply(ctx context.Context,
-	_ schema.GroupVersionResource,
+	gvr schema.GroupVersionResource,
 	required *unstructured.Unstructured,
 	_ metav1.OwnerReference,
 	_ *workapiv1.ManifestConfigOption,
 	_ events.Recorder) (runtime.Object, error) {
 	logger := klog.FromContext(ctx)
 	logger.Info("Noop because its read-only",
-		"kind", required.GetKind(), "resourceNamespace", required.GetNamespace(), "resourceName", required.GetName())
+		"gvr", gvr.String(), "resourceNamespace", required.GetNamespace(), "resourceName", required.GetName())
 	return required, nil
 }

pkg/work/spoke/auth/basic/auth.go

@@ -153,7 +153,7 @@ func (v *SarValidator) CheckEscalation(ctx context.Context, sa *workapiv1.Manife
 	})
 	if apierrors.IsForbidden(err) {
 		logger.Info("not allowed to apply the resource",
-			"gvr", gvr, "resourceNamespace", namespace, "resourceName", name, "error", err)
+			"gvr", gvr.String(), "resourceNamespace", namespace, "resourceName", name, "error", err)
 		return &NotAllowedError{
 			Err: fmt.Errorf("not allowed to apply the resource %s %s, %s %s, error: permission escalation",
 				gvr.Group, gvr.Resource, namespace, name),

pkg/work/spoke/controllers/finalizercontroller/manifestwork_finalize_controller.go

@@ -70,7 +70,7 @@ func (m *ManifestWorkFinalizeController) sync(ctx context.Context, controllerCon
 	appliedManifestWorkName := fmt.Sprintf("%s-%s", m.hubHash, manifestWorkName)
 
 	logger := klog.FromContext(ctx).WithName(appliedManifestWorkFinalizer).
-		WithValues(manifestWorkFinalizer, appliedManifestWorkName, "manifestWorkName", manifestWorkName)
+		WithValues("appliedManifestWorkName", appliedManifestWorkName, "manifestWorkName", manifestWorkName)
 	ctx = klog.NewContext(ctx, logger)
 
 	logger.V(5).Info("Reconciling ManifestWork")

pkg/work/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller.go

@@ -91,7 +91,7 @@ func (m *unmanagedAppliedWorkController) sync(ctx context.Context, controllerCon
 	appliedManifestWorkName := controllerContext.QueueKey()
 
 	logger := klog.FromContext(ctx).WithName(appliedManifestWorkFinalizer).
-		WithValues(unManagedAppliedManifestWork, appliedManifestWorkName)
+		WithValues("appliedManifestWorkName", appliedManifestWorkName)
 	ctx = klog.NewContext(ctx, logger)
 
 	logger.V(5).Info("Reconciling AppliedManifestWork")

pkg/work/spoke/controllers/manifestcontroller/manifestwork_controller.go

@@ -126,7 +126,7 @@ func NewManifestWorkController(
 func (m *ManifestWorkController) sync(ctx context.Context, controllerContext factory.SyncContext) error {
 	manifestWorkName := controllerContext.QueueKey()
 	logger := klog.FromContext(ctx).WithName(controllerName).WithValues("manifestWorkName", manifestWorkName)
-	logger.V(4).Info("Reconciling ManifestWork")
+	logger.V(5).Info("Reconciling ManifestWork")
 	ctx = klog.NewContext(ctx, logger)
 
 	oldManifestWork, err := m.manifestWorkLister.Get(manifestWorkName)

pkg/work/spoke/spokeagent.go

@@ -2,6 +2,7 @@ package spoke
 
 import (
 	"context"
+	"os"
 	"time"
 
 	"github.com/openshift/library-go/pkg/controller/controllercmd"
@@ -63,6 +64,10 @@ func NewWorkAgentConfig(commonOpts *options.AgentOptions, opts *WorkloadAgentOpt
 func (o *WorkAgentConfig) RunWorkloadAgent(ctx context.Context, controllerContext *controllercmd.ControllerContext) error {
 	// setting up contextual logger
 	logger := klog.NewKlogr()
+	podName := os.Getenv("POD_NAME")
+	if podName != "" {
+		logger = logger.WithValues("podName", podName)
+	}
 	ctx = klog.NewContext(ctx, logger)
 
 	// load spoke client config and create spoke clients,