Chore: Refactor

* Pass DataCollector struct as a reference from main
* Improve logic around job result/status

Author: mrajagopal

cmd/nginx-supportpkg.go

@@ -31,31 +31,22 @@ import (
 
 func Execute() {
 
-	var namespaces []string
 	var product string
-	var excludeDBData bool
-	var excludeTimeSeriesData bool
 	var jobList []jobs.Job
+	collector := data_collector.DataCollector{}
 
 	var rootCmd = &cobra.Command{
 		Use:   "nginx-supportpkg",
 		Short: "nginx-supportpkg - a tool to create Ingress Controller diagnostics package",
 		Long:  `nginx-supportpkg - a tool to create Ingress Controller diagnostics package`,
 		Run: func(cmd *cobra.Command, args []string) {
 
-			collector, err := data_collector.NewDataCollector(namespaces...)
+			err := data_collector.NewDataCollector(&collector)
 			if err != nil {
 				fmt.Println(fmt.Errorf("unable to start data collector: %s", err))
 				os.Exit(1)
 			}
 
-			if excludeDBData {
-				collector.ExcludeDBData = true
-			}
-			if excludeTimeSeriesData {
-				collector.ExcludeTimeSeriesData = true
-			}
-
 			collector.Logger.Printf("Starting kubectl-nginx-supportpkg - version: %s - build: %s", version.Version, version.Build)
 			collector.Logger.Printf("Input args are %v", os.Args)
 
@@ -77,14 +68,14 @@ func Execute() {
 				failedJobs := 0
 				for _, job := range jobList {
 					fmt.Printf("Running job %s...", job.Name)
-					err, Skipped := job.Collect(collector)
+					err, Skipped := job.Collect(&collector)
 					if Skipped {
 						fmt.Print(" SKIPPED\n")
 					} else if err != nil {
-						fmt.Printf(" Error: %s\n", err)
+						fmt.Printf(" FAILED: %s\n", err)
 						failedJobs++
 					} else {
-						fmt.Print(" OK\n")
+						fmt.Print(" COMPLETED\n")
 					}
 				}
 
@@ -107,7 +98,7 @@ func Execute() {
 		},
 	}
 
-	rootCmd.Flags().StringSliceVarP(&namespaces, "namespace", "n", []string{}, "list of namespaces to collect information from")
+	rootCmd.Flags().StringSliceVarP(&collector.Namespaces, "namespace", "n", []string{}, "list of namespaces to collect information from")
 	if err := rootCmd.MarkFlagRequired("namespace"); err != nil {
 		fmt.Println(err)
 		os.Exit(1)
@@ -119,8 +110,8 @@ func Execute() {
 		os.Exit(1)
 	}
 
-	rootCmd.Flags().BoolVarP(&excludeDBData, "exclude-db-data", "d", false, "exclude DB data collection")
-	rootCmd.Flags().BoolVarP(&excludeTimeSeriesData, "exclude-time-series-data", "t", false, "exclude time series data collection")
+	rootCmd.Flags().BoolVarP(&collector.ExcludeDBData, "exclude-db-data", "d", false, "exclude DB data collection")
+	rootCmd.Flags().BoolVarP(&collector.ExcludeTimeSeriesData, "exclude-time-series-data", "t", false, "exclude time series data collection")
 
 	versionStr := "nginx-supportpkg - version: " + version.Version + " - build: " + version.Build + "\n"
 	rootCmd.SetVersionTemplate(versionStr)

pkg/data_collector/data_collector.go

@@ -60,16 +60,16 @@ type DataCollector struct {
 	ExcludeTimeSeriesData bool
 }
 
-func NewDataCollector(namespaces ...string) (*DataCollector, error) {
+func NewDataCollector(collector *DataCollector) error {
 
 	tmpDir, err := os.MkdirTemp("", "-pkg-diag")
 	if err != nil {
-		return nil, fmt.Errorf("unable to create temp directory: %s", err)
+		return fmt.Errorf("unable to create temp directory: %s", err)
 	}
 
 	logFile, err := os.OpenFile(filepath.Join(tmpDir, "supportpkg.log"), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
 	if err != nil {
-		return nil, fmt.Errorf("unable to create log file: %s", err)
+		return fmt.Errorf("unable to create log file: %s", err)
 	}
 
 	// Find config
@@ -80,30 +80,27 @@ func NewDataCollector(namespaces ...string) (*DataCollector, error) {
 	config, err := clientcmd.BuildConfigFromFlags("", kubeConfig)
 
 	if err != nil {
-		return nil, fmt.Errorf("unable to connect to k8s using file %s: %s", kubeConfig, err)
-	}
-
-	dc := DataCollector{
-		BaseDir:          tmpDir,
-		Namespaces:       namespaces,
-		LogFile:          logFile,
-		Logger:           log.New(logFile, "", log.LstdFlags|log.LUTC|log.Lmicroseconds|log.Lshortfile),
-		K8sHelmClientSet: make(map[string]helmClient.Client),
+		return fmt.Errorf("unable to connect to k8s using file %s: %s", kubeConfig, err)
 	}
+	// Set up the DataCollector options
+	collector.BaseDir = tmpDir
+	collector.LogFile = logFile
+	collector.Logger = log.New(logFile, "", log.LstdFlags|log.LUTC|log.Lmicroseconds|log.Lshortfile)
+	collector.K8sHelmClientSet = make(map[string]helmClient.Client)
 
 	//Initialize clients
-	dc.K8sRestConfig = config
-	dc.K8sCoreClientSet, _ = kubernetes.NewForConfig(config)
-	dc.K8sCrdClientSet, _ = crdClient.NewForConfig(config)
-	dc.K8sMetricsClientSet, _ = metricsClient.NewForConfig(config)
-	for _, namespace := range dc.Namespaces {
-		dc.K8sHelmClientSet[namespace], _ = helmClient.NewClientFromRestConf(&helmClient.RestConfClientOptions{
+	collector.K8sRestConfig = config
+	collector.K8sCoreClientSet, _ = kubernetes.NewForConfig(config)
+	collector.K8sCrdClientSet, _ = crdClient.NewForConfig(config)
+	collector.K8sMetricsClientSet, _ = metricsClient.NewForConfig(config)
+	for _, namespace := range collector.Namespaces {
+		collector.K8sHelmClientSet[namespace], _ = helmClient.NewClientFromRestConf(&helmClient.RestConfClientOptions{
 			Options:    &helmClient.Options{Namespace: namespace},
 			RestConfig: config,
 		})
 	}
 
-	return &dc, nil
+	return nil
 }
 
 func (c *DataCollector) WrapUp(product string) (string, error) {

pkg/jobs/job.go

@@ -52,13 +52,16 @@ func (j Job) Collect(dc *data_collector.DataCollector) (error, bool) {
 	select {
 	case <-ctx.Done():
 		dc.Logger.Printf("\tJob %s has timed out: %s\n---\n", j.Name, ctx.Err())
-		err := fmt.Errorf("Context cancelled: %v", ctx.Err())
-		return err, false
+		return fmt.Errorf("Context cancelled: %v", ctx.Err()), false
 
 	case jobResults := <-ch:
+		if jobResults.Skipped {
+			dc.Logger.Printf("\tJob %s has been skipped\n---\n", j.Name)
+			return nil, true
+		}
 		if jobResults.Error != nil {
 			dc.Logger.Printf("\tJob %s has failed: %s\n", j.Name, jobResults.Error)
-			return jobResults.Error, jobResults.Skipped
+			return jobResults.Error, false
 		}
 
 		for fileName, fileValue := range jobResults.Files {