fix(cli/capture): remove global Logger, inject via dependency injection (issue #585)

Removes the global var Logger *log.ZapLogger from cli/cmd/root.go and replaces
all usages across the capture package with proper dependency injection.

Each sub-command gets its own named child logger:
  - NewCreateSubCommand   -> .Named("retina-capture-create")
  - NewDeleteSubCommand   -> .Named("retina-capture-delete")
  - NewDownloadSubCommand -> .Named("retina-capture-download")

All code comments in create.go, delete.go, and download.go are preserved.
Unit tests added to pkg/log/zap_test.go verifying the logger= field appears
in structured log output (TestNamedLoggerContext, TestNamedLoggerContextDeleteCmd).
Kind cluster test confirms injected logger flows through all capture call sites
without nil pointer dereference.

Closes #585

Signed-off-by: Sudheer Obbu <mail2sudheerobbu@gmail.com>

Author: mail2sudheerobbu-oss

cli/cmd/capture/capture.go

@@ -6,6 +6,7 @@ package capture
 import (
 	"time"
 
+	"github.com/microsoft/retina/pkg/log"
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/client-go/kubernetes"
@@ -39,6 +40,7 @@ type Opts struct {
 	s3Region           string
 	s3SecretAccessKey  string
 	tcpdumpFilter      string
+	logger             *log.ZapLogger
 }
 
 var opts = Opts{

cli/cmd/capture/create.go

@@ -23,14 +23,14 @@ import (
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
 
-	retinacmd "github.com/microsoft/retina/cli/cmd"
 	retinav1alpha1 "github.com/microsoft/retina/crd/api/v1alpha1"
 	"github.com/microsoft/retina/internal/buildinfo"
 	pkgcapture "github.com/microsoft/retina/pkg/capture"
 	captureConstants "github.com/microsoft/retina/pkg/capture/constants"
 	"github.com/microsoft/retina/pkg/capture/file"
 	captureUtils "github.com/microsoft/retina/pkg/capture/utils"
 	"github.com/microsoft/retina/pkg/config"
+	"github.com/microsoft/retina/pkg/log"
 )
 
 const (
@@ -109,47 +109,47 @@ func create(kubeClient kubernetes.Interface) error {
 
 	jobsCreated, err := createJobs(ctx, kubeClient, capture)
 	if err != nil {
-		retinacmd.Logger.Error("Failed to create job", zap.Error(err))
+		opts.logger.Error("Failed to create job", zap.Error(err))
 		return err
 	}
 	if opts.nowait {
-		retinacmd.Logger.Info("Please manually delete all capture jobs")
+		opts.logger.Info("Please manually delete all capture jobs")
 		if capture.Spec.OutputConfiguration.BlobUpload != nil {
-			retinacmd.Logger.Info("Please manually delete capture secret", zap.String("namespace", *opts.Namespace), zap.String("secret name", *capture.Spec.OutputConfiguration.BlobUpload))
+			opts.logger.Info("Please manually delete capture secret", zap.String("namespace", *opts.Namespace), zap.String("secret name", *capture.Spec.OutputConfiguration.BlobUpload))
 		}
 		if capture.Spec.OutputConfiguration.S3Upload != nil && capture.Spec.OutputConfiguration.S3Upload.SecretName != "" {
-			retinacmd.Logger.Info("Please manually delete capture secret", zap.String("namespace", *opts.Namespace), zap.String("secret name", capture.Spec.OutputConfiguration.S3Upload.SecretName))
+			opts.logger.Info("Please manually delete capture secret", zap.String("namespace", *opts.Namespace), zap.String("secret name", capture.Spec.OutputConfiguration.S3Upload.SecretName))
 		}
 		printCaptureResult(jobsCreated)
 		return nil
 	}
 
 	// Wait until all jobs finish then delete the jobs before the timeout, otherwise print jobs created to
 	// let the customer recycle them.
-	retinacmd.Logger.Info("Waiting for capture jobs to finish")
+	opts.logger.Info("Waiting for capture jobs to finish")
 
 	allJobsCompleted := waitUntilJobsComplete(ctx, kubeClient, jobsCreated)
 
 	// Delete all jobs created only if they all completed, otherwise keep the jobs for debugging.
 	if allJobsCompleted {
-		retinacmd.Logger.Info("Deleting jobs as all jobs are completed")
+		opts.logger.Info("Deleting jobs as all jobs are completed")
 		jobsFailedToDelete := deleteJobs(ctx, kubeClient, jobsCreated)
 		if len(jobsFailedToDelete) != 0 {
-			retinacmd.Logger.Info("Please manually delete capture jobs failed to delete", zap.String("namespace", *opts.Namespace), zap.String("job list", strings.Join(jobsFailedToDelete, ",")))
+			opts.logger.Info("Please manually delete capture jobs failed to delete", zap.String("namespace", *opts.Namespace), zap.String("job list", strings.Join(jobsFailedToDelete, ",")))
 		}
 
 		if capture.Spec.OutputConfiguration.BlobUpload != nil {
 			err = deleteSecret(ctx, kubeClient, capture.Spec.OutputConfiguration.BlobUpload)
 			if err != nil {
-				retinacmd.Logger.Error("Failed to delete capture secret, please manually delete it",
+				opts.logger.Error("Failed to delete capture secret, please manually delete it",
 					zap.String("namespace", *opts.Namespace), zap.String("secret name", *capture.Spec.OutputConfiguration.BlobUpload), zap.Error(err))
 			}
 		}
 
 		if capture.Spec.OutputConfiguration.S3Upload != nil && capture.Spec.OutputConfiguration.S3Upload.SecretName != "" {
 			err = deleteSecret(ctx, kubeClient, &capture.Spec.OutputConfiguration.S3Upload.SecretName)
 			if err != nil {
-				retinacmd.Logger.Error("Failed to delete capture secret, please manually delete it",
+				opts.logger.Error("Failed to delete capture secret, please manually delete it",
 					zap.String("namespace", *opts.Namespace),
 					zap.String("secret name", capture.Spec.OutputConfiguration.S3Upload.SecretName),
 					zap.Error(err),
@@ -158,13 +158,13 @@ func create(kubeClient kubernetes.Interface) error {
 		}
 
 		if len(jobsFailedToDelete) == 0 && err == nil {
-			retinacmd.Logger.Info("Done for deleting jobs")
+			opts.logger.Info("Done for deleting jobs")
 		}
 		return nil
 	}
 
-	retinacmd.Logger.Info("Not all job are completed in the given time")
-	retinacmd.Logger.Info("Please manually delete the Capture")
+	opts.logger.Info("Not all job are completed in the given time")
+	opts.logger.Info("Please manually delete the Capture")
 
 	return getCaptureAndPrintCaptureResult(ctx, kubeClient, capture.Name, *opts.Namespace)
 }
@@ -191,6 +191,7 @@ func NewCreateSubCommand(kubeClient kubernetes.Interface) *cobra.Command {
 	}
 
 	createCapture.RunE = func(*cobra.Command, []string) error {
+		opts.logger = log.Logger().Named("retina-capture-create")
 		return create(kubeClient)
 	}
 
@@ -299,10 +300,10 @@ func createCaptureF(ctx context.Context, kubeClient kubernetes.Interface) (*reti
 		},
 	}
 
-	retinacmd.Logger.Info(fmt.Sprintf("Capture timestamp: %s", timestamp))
+	opts.logger.Info(fmt.Sprintf("Capture timestamp: %s", timestamp))
 
 	if opts.duration != 0 {
-		retinacmd.Logger.Info(fmt.Sprintf("The capture duration is set to %s", opts.duration))
+		opts.logger.Info(fmt.Sprintf("The capture duration is set to %s", opts.duration))
 		capture.Spec.CaptureConfiguration.CaptureOption.Duration = &metav1.Duration{Duration: opts.duration}
 	}
 
@@ -366,17 +367,17 @@ func createCaptureF(ctx context.Context, kubeClient kubernetes.Interface) (*reti
 		for i := range podNameSlice {
 			podNameSlice[i] = strings.TrimSpace(podNameSlice[i])
 		}
-		retinacmd.Logger.Info(fmt.Sprintf("Capturing on specific pods: %v", podNameSlice))
+		opts.logger.Info(fmt.Sprintf("Capturing on specific pods: %v", podNameSlice))
 		capture.Spec.CaptureConfiguration.CaptureTarget.PodNames = podNameSlice
 	}
 
 	if opts.maxSize != 0 {
-		retinacmd.Logger.Info(fmt.Sprintf("The capture file max size is set to %dMB", opts.maxSize))
+		opts.logger.Info(fmt.Sprintf("The capture file max size is set to %dMB", opts.maxSize))
 		capture.Spec.CaptureConfiguration.CaptureOption.MaxCaptureSize = &opts.maxSize
 	}
 
 	if opts.packetSize != 0 {
-		retinacmd.Logger.Info(fmt.Sprintf("The capture packet size is set to %d bytes", opts.packetSize))
+		opts.logger.Info(fmt.Sprintf("The capture packet size is set to %d bytes", opts.packetSize))
 		capture.Spec.CaptureConfiguration.CaptureOption.PacketSize = &opts.packetSize
 	}
 
@@ -385,7 +386,7 @@ func createCaptureF(ctx context.Context, kubeClient kubernetes.Interface) (*reti
 		for i := range interfaceSlice {
 			interfaceSlice[i] = strings.TrimSpace(interfaceSlice[i])
 		}
-		retinacmd.Logger.Info(fmt.Sprintf("Capturing on specific interfaces: %v", interfaceSlice))
+		opts.logger.Info(fmt.Sprintf("Capturing on specific interfaces: %v", interfaceSlice))
 		capture.Spec.CaptureConfiguration.CaptureOption.Interfaces = interfaceSlice
 	}
 
@@ -447,7 +448,7 @@ func getCLICaptureConfig() config.CaptureConfig {
 }
 
 func createJobs(ctx context.Context, kubeClient kubernetes.Interface, capture *retinav1alpha1.Capture) ([]batchv1.Job, error) {
-	translator := pkgcapture.NewCaptureToPodTranslator(kubeClient, retinacmd.Logger, getCLICaptureConfig())
+	translator := pkgcapture.NewCaptureToPodTranslator(kubeClient, opts.logger, getCLICaptureConfig())
 	jobs, err := translator.TranslateCaptureToJobs(ctx, capture)
 	if err != nil {
 		return nil, err
@@ -460,7 +461,7 @@ func createJobs(ctx context.Context, kubeClient kubernetes.Interface, capture *r
 			return nil, err
 		}
 		jobsCreated = append(jobsCreated, *jobCreated)
-		retinacmd.Logger.Info("Packet capture job is created", zap.String("namespace", *opts.Namespace), zap.String("capture job", jobCreated.Name))
+		opts.logger.Info("Packet capture job is created", zap.String("namespace", *opts.Namespace), zap.String("capture job", jobCreated.Name))
 	}
 	return jobsCreated, nil
 }
@@ -479,7 +480,7 @@ func waitUntilJobsComplete(ctx context.Context, kubeClient kubernetes.Interface,
 	if period < opts.duration/10 {
 		period = opts.duration / 10
 	}
-	retinacmd.Logger.Info(fmt.Sprintf("Waiting timeout is set to %s", deadline))
+	opts.logger.Info(fmt.Sprintf("Waiting timeout is set to %s", deadline))
 
 	ctx, cancel := context.WithTimeout(ctx, deadline)
 	defer cancel()
@@ -491,7 +492,7 @@ func waitUntilJobsComplete(ctx context.Context, kubeClient kubernetes.Interface,
 		for _, job := range jobs {
 			jobRet, err := kubeClient.BatchV1().Jobs(job.Namespace).Get(ctx, job.Name, metav1.GetOptions{})
 			if err != nil {
-				retinacmd.Logger.Error("Failed to get job", zap.String("namespace", job.Namespace), zap.String("job name", job.Name), zap.Error(err))
+				opts.logger.Error("Failed to get job", zap.String("namespace", job.Namespace), zap.String("job name", job.Name), zap.Error(err))
 				jobsIncompleted = append(jobsIncompleted, job.Name)
 				continue
 			}
@@ -503,7 +504,7 @@ func waitUntilJobsComplete(ctx context.Context, kubeClient kubernetes.Interface,
 		}
 
 		if len(jobsIncompleted) != 0 {
-			retinacmd.Logger.Info("Not all jobs are completed",
+			opts.logger.Info("Not all jobs are completed",
 				zap.String("namespace", *opts.Namespace),
 				zap.String("Completed jobs", strings.Join(jobsCompleted, ",")),
 				zap.String("Uncompleted packet capture jobs", strings.Join(jobsIncompleted, ",")),
@@ -529,7 +530,7 @@ func deleteJobs(ctx context.Context, kubeClient kubernetes.Interface, jobs []bat
 		})
 		if err != nil {
 			jobsFailedtoDelete = append(jobsFailedtoDelete, job.Name)
-			retinacmd.Logger.Error("Failed to delete job", zap.String("namespace", job.Namespace), zap.String("job name", job.Name), zap.Error(err))
+			opts.logger.Error("Failed to delete job", zap.String("namespace", job.Namespace), zap.String("job name", job.Name), zap.Error(err))
 		}
 	}
 	return jobsFailedtoDelete

cli/cmd/capture/delete.go

@@ -9,9 +9,9 @@ import (
 	"os/signal"
 	"syscall"
 
-	retinacmd "github.com/microsoft/retina/cli/cmd"
 	captureConstants "github.com/microsoft/retina/pkg/capture/constants"
 	"github.com/microsoft/retina/pkg/label"
+	"github.com/microsoft/retina/pkg/log"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"go.uber.org/zap"
@@ -33,6 +33,7 @@ func NewDeleteSubCommand(kubeClient kubernetes.Interface) *cobra.Command {
 		Short:   "Delete a Retina capture",
 		Example: deleteExample,
 		RunE: func(*cobra.Command, []string) error {
+			opts.logger = log.Logger().Named("retina-capture-delete")
 			ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGTERM)
 			defer cancel()
 
@@ -70,7 +71,7 @@ func NewDeleteSubCommand(kubeClient kubernetes.Interface) *cobra.Command {
 				if err := kubeClient.BatchV1().Jobs(jobList.Items[idx].Namespace).Delete(ctx, jobList.Items[idx].Name, metav1.DeleteOptions{
 					PropagationPolicy: &deletePropagationBackground,
 				}); err != nil {
-					retinacmd.Logger.Info("Failed to delete job", zap.String("job name", jobList.Items[idx].Name), zap.Error(err))
+					opts.logger.Info("Failed to delete job", zap.String("job name", jobList.Items[idx].Name), zap.Error(err))
 				}
 			}
 
@@ -82,7 +83,7 @@ func NewDeleteSubCommand(kubeClient kubernetes.Interface) *cobra.Command {
 					break
 				}
 			}
-			retinacmd.Logger.Info(fmt.Sprintf("Retina Capture %q delete", *opts.Name))
+			opts.logger.Info(fmt.Sprintf("Retina Capture %q delete", *opts.Name))
 
 			return nil
 		},