From 0113624352eba7ab45c4f78e6bdd9913cdf94c85 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Fri, 11 Oct 2024 12:48:32 -0500 Subject: [PATCH] chore(support-bundle): respect using load-cluster-specs=false (#1634) * fix: Allow using load-cluster-specs=false Signed-off-by: Evans Mungai * Some more simplification Signed-off-by: Evans Mungai * Ensure error in loading specs is printed in CLI Signed-off-by: Evans Mungai * Run linter Signed-off-by: Evans Mungai * Fix failing tests Signed-off-by: Evans Mungai * Remove unnecessary test case rename Signed-off-by: Evans Mungai * Fix error wrapping Signed-off-by: Evans Mungai * Check if load-cluster-specs was provided in cli Signed-off-by: Evans Mungai * Better wording in comments Signed-off-by: Evans Mungai --------- Signed-off-by: Evans Mungai --- cmd/troubleshoot/cli/root.go | 14 +++++++++++++- cmd/troubleshoot/cli/run.go | 26 +++++++------------------- internal/specs/specs.go | 2 ++ pkg/loader/loader.go | 28 ++++++++++++++-------------- pkg/types/types.go | 7 ++++++- 5 files changed, 42 insertions(+), 35 deletions(-) diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go index 78b3cc2b..f2cd0390 100644 --- a/cmd/troubleshoot/cli/root.go +++ b/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") diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 89cfef52..335f3320 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/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 diff --git a/internal/specs/specs.go b/internal/specs/specs.go index 2f442e35..cfb0060f 100644 --- a/internal/specs/specs.go +++ b/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") diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go index e139835d..872b7827 100644 --- a/pkg/loader/loader.go +++ b/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 } diff --git a/pkg/types/types.go b/pkg/types/types.go index d710362a..47fa1b5c 100644 --- a/pkg/types/types.go +++ b/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 {