chore(support-bundle): respect using load-cluster-specs=false (#1634)

* fix: Allow using load-cluster-specs=false

Signed-off-by: Evans Mungai <[email protected]>

* Some more simplification

Signed-off-by: Evans Mungai <[email protected]>

* Ensure error in loading specs is printed in CLI

Signed-off-by: Evans Mungai <[email protected]>

* Run linter

Signed-off-by: Evans Mungai <[email protected]>

* Fix failing tests

Signed-off-by: Evans Mungai <[email protected]>

* Remove unnecessary test case rename

Signed-off-by: Evans Mungai <[email protected]>

* Fix error wrapping

Signed-off-by: Evans Mungai <[email protected]>

* Check if load-cluster-specs was provided in cli

Signed-off-by: Evans Mungai <[email protected]>

* Better wording in comments

Signed-off-by: Evans Mungai <[email protected]>

---------

Signed-off-by: Evans Mungai <[email protected]>

Author: banjoh

cmd/troubleshoot/cli/root.go

@@ -44,6 +44,18 @@ If no arguments are provided, specs are automatically loaded from the cluster by
 		RunE: func(cmd *cobra.Command, args []string) error {
 			v := viper.GetViper()
 
+			// If there are no locations to load specs from passed in the cli args, we should
+			// load them from the cluster by setting "load-cluster-specs=true". If the caller
+			// provided "--load-cluster-specs" cli option, we should respect it.
+			if len(args) == 0 {
+				// Check if --load-cluster-specs was set by the cli caller to avoid overriding it
+				flg := cmd.Flags().Lookup("load-cluster-specs")
+				if flg != nil && !flg.Changed {
+					// Load specs from the cluster if no spec(s) is(are) provided in the cli args
+					v.Set("load-cluster-specs", true)
+				}
+			}
+
 			closer, err := traces.ConfigureTracing("support-bundle")
 			if err != nil {
 				// Do not fail running support-bundle if tracing fails
@@ -77,7 +89,7 @@ If no arguments are provided, specs are automatically loaded from the cluster by
 	cmd.Flags().Bool("interactive", true, "enable/disable interactive mode")
 	cmd.Flags().Bool("collect-without-permissions", true, "always generate a support bundle, even if it some require additional permissions")
 	cmd.Flags().StringSliceP("selector", "l", []string{"troubleshoot.sh/kind=support-bundle"}, "selector to filter on for loading additional support bundle specs found in secrets within the cluster")
-	cmd.Flags().Bool("load-cluster-specs", false, "enable/disable loading additional troubleshoot specs found within the cluster. This is the default behavior if no spec is provided as an argument")
+	cmd.Flags().Bool("load-cluster-specs", false, "enable/disable loading additional troubleshoot specs found within the cluster. Do not load by default unless no specs are provided in the cli args")
 	cmd.Flags().String("since-time", "", "force pod logs collectors to return logs after a specific date (RFC3339)")
 	cmd.Flags().String("since", "", "force pod logs collectors to return logs newer than a relative duration like 5s, 2m, or 3h.")
 	cmd.Flags().StringP("output", "o", "", "specify the output file path for the support bundle")

cmd/troubleshoot/cli/run.go

@@ -292,24 +292,9 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 		err       error
 	)
 
-	if len(args) < 1 {
-		fmt.Println("\r\033[36mNo specs provided, attempting to load from cluster...\033[m")
-		kinds, err = specs.LoadFromCluster(ctx, client, vp.GetStringSlice("selector"), vp.GetString("namespace"))
-		if err != nil {
-			return nil, nil, errors.Wrap(err, "failed to load specs from cluster, and no specs were provided as arguments")
-		}
-		if len(redactors) > 0 {
-			additionalKinds, err := specs.LoadFromCLIArgs(ctx, client, allArgs, vp)
-			if err != nil {
-				return nil, nil, errors.Wrap(err, "failed to load redactors from CLI args")
-			}
-			kinds.RedactorsV1Beta2 = append(kinds.RedactorsV1Beta2, additionalKinds.RedactorsV1Beta2...)
-		}
-	} else {
-		kinds, err = specs.LoadFromCLIArgs(ctx, client, allArgs, vp)
-		if err != nil {
-			return nil, nil, errors.Wrap(err, "failed to load specs from CLI args")
-		}
+	kinds, err = specs.LoadFromCLIArgs(ctx, client, allArgs, vp)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to load specs from CLI args")
 	}
 
 	// Load additional specs from support bundle URIs
@@ -326,7 +311,10 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 	if len(kinds.CollectorsV1Beta2) == 0 &&
 		len(kinds.HostCollectorsV1Beta2) == 0 &&
 		len(kinds.SupportBundlesV1Beta2) == 0 {
-		return nil, nil, types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.Wrap(err, "no collectors specified to run. Use --debug and/or -v=2 to see more information"))
+		return nil, nil, types.NewExitCodeError(
+			constants.EXIT_CODE_CATCH_ALL,
+			errors.New("no collectors specified to run. Use --debug and/or -v=2 to see more information"),
+		)
 	}
 
 	// Merge specs

internal/specs/specs.go

@@ -268,6 +268,8 @@ func downloadFromHttpURL(ctx context.Context, url string, headers map[string]str
 // to list & read secrets and configmaps from all namespaces, we will fallback to trying each
 // namespace individually, and eventually default to the configured kubeconfig namespace.
 func LoadFromCluster(ctx context.Context, client kubernetes.Interface, selectors []string, ns string) (*loader.TroubleshootKinds, error) {
+	klog.V(1).Infof("Load troubleshoot specs from the cluster using selectors: %v", selectors)
+
 	if reflect.DeepEqual(selectors, []string{"troubleshoot.sh/kind=support-bundle"}) {
 		// Its the default selector so we append troubleshoot.io/kind=support-bundle to it due to backwards compatibility
 		selectors = append(selectors, "troubleshoot.io/kind=support-bundle")

pkg/loader/loader.go

@@ -83,14 +83,18 @@ type TroubleshootKinds struct {
 }
 
 func (kinds *TroubleshootKinds) IsEmpty() bool {
-	return len(kinds.AnalyzersV1Beta2) == 0 &&
-		len(kinds.CollectorsV1Beta2) == 0 &&
-		len(kinds.HostCollectorsV1Beta2) == 0 &&
-		len(kinds.HostPreflightsV1Beta2) == 0 &&
-		len(kinds.PreflightsV1Beta2) == 0 &&
-		len(kinds.RedactorsV1Beta2) == 0 &&
-		len(kinds.RemoteCollectorsV1Beta2) == 0 &&
-		len(kinds.SupportBundlesV1Beta2) == 0
+	return kinds.Len() == 0
+}
+
+func (kinds *TroubleshootKinds) Len() int {
+	return len(kinds.AnalyzersV1Beta2) +
+		len(kinds.CollectorsV1Beta2) +
+		len(kinds.HostCollectorsV1Beta2) +
+		len(kinds.HostPreflightsV1Beta2) +
+		len(kinds.PreflightsV1Beta2) +
+		len(kinds.RedactorsV1Beta2) +
+		len(kinds.RemoteCollectorsV1Beta2) +
+		len(kinds.SupportBundlesV1Beta2)
 }
 
 func (kinds *TroubleshootKinds) Add(other *TroubleshootKinds) {
@@ -200,7 +204,7 @@ func (l *specLoader) loadFromStrings(rawSpecs ...string) (*TroubleshootKinds, er
 			// If it's not a configmap or secret, just append it to the splitdocs
 			splitdocs = append(splitdocs, rawDoc)
 		} else {
-			klog.V(1).Infof("Skip loading %q kind", parsed.Kind)
+			klog.V(2).Infof("Skip loading %q kind", parsed.Kind)
 		}
 	}
 
@@ -254,11 +258,7 @@ func (l *specLoader) loadFromSplitDocs(splitdocs []string) (*TroubleshootKinds,
 		}
 	}
 
-	if kinds.IsEmpty() {
-		klog.V(1).Info("No troubleshoot specs were loaded")
-	} else {
-		klog.V(1).Info("Loaded troubleshoot specs successfully")
-	}
+	klog.V(2).Infof("Loaded %d troubleshoot specs successfully", kinds.Len())
 
 	return kinds, nil
 }

pkg/types/types.go

@@ -20,6 +20,7 @@ type ExitError interface {
 type ExitCodeError struct {
 	Msg  string
 	Code int
+	Err  error
 }
 
 type ExitCodeWarning struct {
@@ -30,6 +31,10 @@ func (e *ExitCodeError) Error() string {
 	return e.Msg
 }
 
+func (e *ExitCodeError) Unwrap() error {
+	return e.Err
+}
+
 func (e *ExitCodeError) ExitStatus() int {
 	return e.Code
 }
@@ -39,7 +44,7 @@ func NewExitCodeError(exitCode int, theErr error) *ExitCodeError {
 	if theErr != nil {
 		useErr = theErr.Error()
 	}
-	return &ExitCodeError{Msg: useErr, Code: exitCode}
+	return &ExitCodeError{Msg: useErr, Code: exitCode, Err: theErr}
 }
 
 func NewExitCodeWarning(theErrMsg string) *ExitCodeWarning {