cli: remove global logger; inject logger via dependency injection

Resolves #585

Remove the package-level `Logger` variable from `cli/cmd` and replace
all usages with contextual child loggers injected through `Opts` structs
and service constructors.

- cli/cmd/root.go: remove `var Logger *log.ZapLogger` and its init assignment
- cli/cmd/capture/capture.go: add `logger` field to `Opts` struct, init in NewCommand()
- cli/cmd/capture/create.go: replace `retinacmd.Logger` with `opts.logger`
- cli/cmd/capture/delete.go: replace `retinacmd.Logger` with `opts.logger`
- cli/cmd/capture/download.go: add `logger` to `DownloadService`, replace usages

Signed-off-by: mail2sudheerobbu-oss <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,7 +23,6 @@ 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"
@@ -109,47 +108,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 +157,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)
 }
@@ -299,17 +298,17 @@ 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}
 	}
 
 	if opts.namespaceSelectors != "" || opts.podSelectors != "" || opts.podNames != "" {
 		// if node selector is using the default value (aka hasn't been set by user), set it to nil to prevent clash with namespace and pod selector
 		if opts.nodeSelectors == DefaultNodeSelectors {
-			retinacmd.Logger.Info("Overriding default node selectors value and setting it to nil. Using namespace, pod selectors, or pod names. " +
+			opts.logger.Info("Overriding default node selectors value and setting it to nil. Using namespace, pod selectors, or pod names. " +
 				"To use node selector, please remove namespace and pod selectors.")
 			opts.nodeSelectors = ""
 		}
@@ -365,17 +364,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
 	}
 
@@ -384,7 +383,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
 	}
 
@@ -446,7 +445,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
@@ -459,7 +458,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
 }
@@ -478,7 +477,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()
@@ -490,7 +489,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
 			}
@@ -502,7 +501,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, ",")),
@@ -528,7 +527,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,7 +9,6 @@ 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/pkg/errors"
@@ -70,7 +69,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 +81,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
 		},

cli/cmd/capture/download.go

@@ -20,11 +20,11 @@ import (
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/storage"
-	retinacmd "github.com/microsoft/retina/cli/cmd"
 	captureConstants "github.com/microsoft/retina/pkg/capture/constants"
 	captureFile "github.com/microsoft/retina/pkg/capture/file"
 	captureUtils "github.com/microsoft/retina/pkg/capture/utils"
 	captureLabels "github.com/microsoft/retina/pkg/label"
+	"github.com/microsoft/retina/pkg/log"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"go.uber.org/zap"
@@ -104,6 +104,7 @@ type DownloadService struct {
 	kubeClient kubernetes.Interface
 	config     *rest.Config
 	namespace  string
+	logger *log.ZapLogger
 }
 
 // Key represents a unique capture identifier
@@ -115,14 +116,15 @@ type Key struct {
 // NewDownloadService creates a new download service with shared dependencies
 func NewDownloadService(kubeClient kubernetes.Interface, config *rest.Config, namespace string) *DownloadService {
 	return &DownloadService{
+		logger: log.Logger().Named("retina-capture-download"),
 		kubeClient: kubeClient,
 		config:     config,
 		namespace:  namespace,
 	}
 }
 
-func getDownloadCmd(node *corev1.Node, hostPath, fileName string) (*DownloadCmd, error) {
-	nodeOS, err := getNodeOS(node)
+func (ds *DownloadService) getDownloadCmd(node *corev1.Node, hostPath, fileName string) (*DownloadCmd, error) {
+	nodeOS, err := ds.getNodeOS(node)
 	if err != nil {
 		return nil, err
 	}
@@ -136,7 +138,7 @@ func getDownloadCmd(node *corev1.Node, hostPath, fileName string) (*DownloadCmd,
 		srcFilePath := "C:\\host" + strings.ReplaceAll(hostPath, "/", "\\") + "\\" + fileName + ".tar.gz"
 		mountPath := "C:\\host" + strings.ReplaceAll(hostPath, "/", "\\")
 		return &DownloadCmd{
-			ContainerImage:   getWindowsContainerImage(node),
+			ContainerImage:   ds.getWindowsContainerImage(node),
 			SrcFilePath:      srcFilePath,
 			MountPath:        mountPath,
 			KeepAliveCommand: []string{"cmd", "/c", "echo Download pod ready & ping -n 3601 127.0.0.1 > nul"},
@@ -159,24 +161,24 @@ func getDownloadCmd(node *corev1.Node, hostPath, fileName string) (*DownloadCmd,
 	}
 }
 
-func getNodeOS(node *corev1.Node) (NodeOS, error) {
+func (ds *DownloadService) getNodeOS(node *corev1.Node) (NodeOS, error) {
 	nodeOS := strings.ToLower(node.Status.NodeInfo.OperatingSystem)
 
 	if strings.Contains(nodeOS, "windows") {
-		retinacmd.Logger.Info("Detected node OS: Windows", zap.String("node", node.Name), zap.String("os", node.Status.NodeInfo.OperatingSystem))
+		ds.logger.Info("Detected node OS: Windows", zap.String("node", node.Name), zap.String("os", node.Status.NodeInfo.OperatingSystem))
 		return Windows, nil
 	}
 
 	if strings.Contains(nodeOS, "linux") {
-		retinacmd.Logger.Info("Detected node OS: Linux", zap.String("node", node.Name), zap.String("os", node.Status.NodeInfo.OperatingSystem))
+		ds.logger.Info("Detected node OS: Linux", zap.String("node", node.Name), zap.String("os", node.Status.NodeInfo.OperatingSystem))
 		return Linux, nil
 	}
 
 	return nil, fmt.Errorf("unsupported operating system: %s: %w", node.Status.NodeInfo.OperatingSystem, ErrUnsupportedNodeOS)
 }
 
 // Detects the Windows LTSC version and returns the appropriate nanoserver image
-func getWindowsContainerImage(node *corev1.Node) string {
+func (ds *DownloadService) getWindowsContainerImage(node *corev1.Node) string {
 	osImage := strings.ToLower(node.Status.NodeInfo.OSImage)
 
 	var suffix string
@@ -188,14 +190,14 @@ func getWindowsContainerImage(node *corev1.Node) string {
 	case strings.Contains(osImage, "2016"):
 		suffix = "ltsc2016"
 	default:
-		retinacmd.Logger.Warn("Could not determine Windows LTSC version, defaulting to ltsc2022",
+		ds.logger.Warn("Could not determine Windows LTSC version, defaulting to ltsc2022",
 			zap.String("node", node.Name),
 			zap.String("osImage", osImage))
 		suffix = "ltsc2022"
 	}
 
 	containerImage := "mcr.microsoft.com/windows/nanoserver:" + suffix
-	retinacmd.Logger.Info("Selected Windows container image", zap.String("image", containerImage))
+	ds.logger.Info("Selected Windows container image", zap.String("image", containerImage))
 
 	return containerImage
 }
@@ -290,7 +292,7 @@ func (ds *DownloadService) DownloadFileContent(ctx context.Context, nodeName, ho
 		return nil, errors.Join(ErrGetNodeInfo, err)
 	}
 
-	downloadCmd, err := getDownloadCmd(node, hostPath, fileName)
+	downloadCmd, err := ds.getDownloadCmd(node, hostPath, fileName)
 	if err != nil {
 		return nil, err
 	}
@@ -305,7 +307,7 @@ func (ds *DownloadService) DownloadFileContent(ctx context.Context, nodeName, ho
 	defer func() {
 		cleanupErr := ds.kubeClient.CoreV1().Pods(ds.namespace).Delete(ctx, downloadPod.Name, metav1.DeleteOptions{})
 		if cleanupErr != nil {
-			retinacmd.Logger.Warn("Failed to clean up debug pod", zap.String("name", downloadPod.Name), zap.Error(cleanupErr))
+			ds.logger.Warn("Failed to clean up debug pod", zap.String("name", downloadPod.Name), zap.Error(cleanupErr))
 		}
 	}()
 
@@ -481,15 +483,16 @@ func (ds *DownloadService) createDownloadExec(ctx context.Context, pod *corev1.P
 }
 
 func downloadFromBlob() error {
+	l := log.Logger().Named("retina-capture-download")
 	u, err := url.Parse(blobURL)
 	if err != nil {
-		retinacmd.Logger.Error("err: ", zap.Error(err))
+		l.Error("err: ", zap.Error(err))
 		return fmt.Errorf("failed to parse SAS URL %s: %w", blobURL, err)
 	}
 
 	b, err := storage.NewAccountSASClientFromEndpointToken(u.String(), u.Query().Encode())
 	if err != nil {
-		retinacmd.Logger.Error("err: ", zap.Error(err))
+		l.Error("err: ", zap.Error(err))
 		return fmt.Errorf("failed to create storage account client: %w", err)
 	}
 
@@ -501,12 +504,12 @@ func downloadFromBlob() error {
 	params := storage.ListBlobsParameters{Prefix: *opts.Name}
 	blobList, err := blobService.GetContainerReference(containerName).ListBlobs(params)
 	if err != nil {
-		retinacmd.Logger.Error("err: ", zap.Error(err))
+		l.Error("err: ", zap.Error(err))
 		return fmt.Errorf("failed to list blobstore: %w", err)
 	}
 
 	if len(blobList.Blobs) == 0 {
-		retinacmd.Logger.Error("err: ", zap.Error(err))
+		l.Error("err: ", zap.Error(err))
 		return fmt.Errorf("%w: %s", ErrNoBlobsFound, *opts.Name)
 	}
 
@@ -520,21 +523,21 @@ func downloadFromBlob() error {
 		blobRef := blobService.GetContainerReference(containerName).GetBlobReference(blob.Name)
 		readCloser, err := blobRef.Get(&storage.GetBlobOptions{})
 		if err != nil {
-			retinacmd.Logger.Error("err: ", zap.Error(err))
+			l.Error("err: ", zap.Error(err))
 			return fmt.Errorf("failed to read from blobstore: %w", err)
 		}
 
 		blobData, err := io.ReadAll(readCloser)
 		readCloser.Close()
 		if err != nil {
-			retinacmd.Logger.Error("err: ", zap.Error(err))
+			l.Error("err: ", zap.Error(err))
 			return fmt.Errorf("failed to obtain blob from blobstore: %w", err)
 		}
 
 		outputFile := filepath.Join(outputPath, blob.Name)
 		err = os.WriteFile(outputFile, blobData, 0o600)
 		if err != nil {
-			retinacmd.Logger.Error("err: ", zap.Error(err))
+			l.Error("err: ", zap.Error(err))
 			return fmt.Errorf("failed to write file: %w", err)
 		}