troubleshoot icon indicating copy to clipboard operation
troubleshoot copied to clipboard

Set a bundle path when using preflight

Open CpuID opened this issue 2 years ago • 8 comments

Describe the rationale for the suggested feature.

We should configure a bundle path for preflight usage, even if we don't store the final outcomes. This would allow using a lot of collector/analyzer combinations that expect a result written using SaveResult and then read in via the analyzer.

eg. using the run collector with the textAnalyze analyzer in preflights.

Describe the feature

What we do now in preflight - https://github.com/replicatedhq/troubleshoot/blob/main/pkg/preflight/collect.go#L108 (empty "" for bundle path)

What we do in support bundle - https://github.com/replicatedhq/troubleshoot/blob/main/pkg/supportbundle/collect.go#L34

Make this behaviour consistent across both.

Describe alternatives you've considered

N/A

Additional context

N/A

CpuID avatar Aug 22 '23 02:08 CpuID

Considerations: does this introduce any minimum disk space requirements for preflight usage? If results are being stored?

CpuID avatar Aug 22 '23 02:08 CpuID

The preflight and support-bundle are using different way of processing collectResults.

In preflight, we are using allCollectedData to pass the result to analyze from memory. Here is the code explain https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/run.go#L96

r, err := collectHost(ctx, specs.HostPreflightSpec, progressCh)
collectResults = append(collectResults, *r)
...
for _, res := range collectResults {
	analyzeResults = append(analyzeResults, res.Analyze()...)
}

You can see our collector pass the data directly to res.Analyze()

In the Analyzer()

https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/analyze.go#L22C9-L22C9

It is using c.AllCollectedData

And in doAnalyzer()

https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/analyze.go#L60C3-L60C3

That can confirm that getCollectedFileContents is reading from memory.

To explain of sharing AllCollectedData, you can check collectHost, we are using CollectHostWithContext https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/collect.go#L143

The collectResult is HostCollectResult https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/collect.go#L69C6-L69C23

decoupled from https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/preflight/collect.go#L51 since they have Analyze() and IsRBACAllowed() method

DexterYan avatar Aug 22 '23 06:08 DexterYan

However, in our support-bundle

we are using AnalyzeSupportBundle(ctx, spec, bundlePath) https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/supportbundle/supportbundle.go#L154C68-L154C68

And in AnalyzeSupportBundle, we have AnalyzeLocal to process the collected data https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/supportbundle/supportbundle.go#L259C34-L259C46

AnalyzeLocal is sharing the same of Analyze(), however, the difference is how we pass getCollectedFileContents https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/analyze/download.go#L43C1-L44C1 In support bundle, it is using fcp.getFileContents, fcp.getChildFileContents

DexterYan avatar Aug 22 '23 06:08 DexterYan

I think we may need to use the same way as preflight of reading from memory, since we return collectedData as runHostOutput in https://github.com/replicatedhq/troubleshoot/blob/6972789bea2f4350a6753723618dec3796c9b676/pkg/collect/host_run.go#L73-L78

DexterYan avatar Aug 22 '23 06:08 DexterYan

This would allow using a lot of collector/analyzer combinations that expect a result written using SaveResult and then read in via the analyzer.

I'm not saying we shouldn't add a bundlePath for Preflight for whatever reason, but have we confirmed that the above is true? Are there analyzers which can't be used in kind: Preflight or kind: HostPreflight because it expects to read results from disk instead of from memory? If so, I'd argue the issue is either:

  1. Analyzer not reading from memory when it should be
  2. Collect result for a particular collector not always being stored in memory.

diamonwiggins avatar Aug 23 '23 19:08 diamonwiggins

I looked at the spec in question (below)

apiVersion: troubleshoot.sh/v1beta2
kind: HostPreflight
spec:
  collectors:
    - run:
        name: iptables
        collectorName: iptables-forward-chain
        command: iptables
        args:
          - '-L'
          - FORWARD
  analyzers:
    - textAnalyze:
        checkName: forward-chain-reject-check
        fileName: iptables/iptables-forward-chain.log
        regex: ^REJECT\s+all\s+.*\s+anywhere\s+anywhere
        outcomes:
          - pass:
              message: No default REJECT rule found in iptables FORWARD chain
              when: "false"
          - fail:
              message: |
                Default REJECT rule found in iptables FORWARD chain.
                Please remove it and re-run the installer.
              when: "true"

and noticed that the path specified in the analyser is wrong. It ought to be fileName: host-collectors/run-host/iptables-forward-chain.txt. As per the collector's documentation, collected files will be stored in host-collectors/run-host/ directory. It however wrongly states that the filename would be [collector-name].json. That bit of the doc needs updating.

Recommended doc update

  • Correct the file name of collected output
  • Add missing host-collectors/run-host/<collector-name>-info.json file which is also collected
  • Add an analyser example

banjoh avatar Aug 24 '23 11:08 banjoh

As a debugging tip, run the preflight binary with -v=2 to print out the collected paths of collected data

[evans] $ preflight spec.yaml -v=2
I0824 12:24:34.837202   35937 loader.go:204] loaded troubleshoot specs successfully
 * [etc-hosts] Running collector...
I0824 12:24:34.840306   35937 result.go:105] Added "host-collectors/run-host/etc-hosts-info.json" to bundle output
I0824 12:24:34.840323   35937 result.go:105] Added "host-collectors/run-host/etc-hosts.txt" to bundle output

============ Collectors summary =============
Succeeded (S), eXcluded (X), Failed (F)
=============================================
etc-hosts (S) : 3ms

============ Redactors summary =============
No redactors executed

============= Analyzers summary =============
Succeeded (S), eXcluded (X), Failed (F)
=============================================
check-etc-hosts (S) : 0ms

Duration: 1,987ms

banjoh avatar Aug 24 '23 11:08 banjoh

Docs update PR: https://github.com/replicatedhq/troubleshoot.sh/pull/521

banjoh avatar Aug 24 '23 12:08 banjoh