Run collector and text analyzer don't work well together

[MikaelSmith]

Naming of collector output is unexpected, and doesn't match up well with what text analyzer expects. In particular https://troubleshoot.sh/reference/analyzers/regex/ is very wrong.

What I've found digging through the source is that the Run collector creates a file key for its output with fmt.Sprintf("%s/%s.log", run.Name, run.CollectorName) (this is the result of names working through creating a pod, calling getPodLogs, etc), while the text analyzer expects a name of filepath.Join(textAnalyzer.CollectorName, textAnalyzer.FileName).

The only way I got this to work was with

collectors:
- run:
    name: foo
    collectorName: foo
    image: postgres:11.7-alpine
    command: ["echo"]
    args: ["hello"]
analyzers:
- textAnalyze:
    collectorName: foo
    fileName: foo.log
    regex: hello
    outcomes:
    - pass:
        message: "It worked!"
    - fail:
        message: "It didn't!"

[laverya]

I've updated the regex example:

apiVersion: troubleshoot.replicated.com/v1beta1
kind: Preflight
metadata:
  name: require-hosted-k8s
spec:
  collectors:
    - run:
        collectorName: google-busybox
        image: busybox:1
        name: ping
        namespace: default
        command: ["ping"]
        args: ["-w", "25", "-c", "25", "-i", "0.3", "google.com"]
        imagePullPolicy: IfNotPresent

  analyzers:
    - textAnalyze:
        checkName: Ping Google
        fileName: ping/google-busybox.log
        regexGroups: '(?P<Transmitted>\d+) packets? transmitted, (?P<Received>\d+) packets? received, (?P<Loss>\d+)(\.\d+)?% packet loss'
        outcomes:
          - pass:
              when: "Loss < 5"
              message: google.com resolves correctly
          - warn:
              when: "Loss < 20"
              message: google.com resolves correctly, but with loss between 5% and 20%
          - fail:
              message: High packet loss

I'm convinced we're doing something very wrong with collectorName. Luckily, it's an optional param, and it's easier to just specify the entire filepath at once.

[dexhorthy]

@MikaelSmith did this approach work for you? Good to close?

[MikaelSmith]

Well, I still think it's poor default behavior, but I believe the example does work.

[manavellamnimble]

@laverya @dexhorthy I've been thinking of ways to make run collector and text analyzer more "compatible". I think the following modifications must be made:

• Run collector should save the files under runcollector.CollectorName/runcollector.Name .log (other collectors use the same format, currently it is saved under runcollector.Name/runcollector.CollectorName .log). Collector Name can default to 'runCollector' for example.

• The name parameter should be called logFileName or fileName, as it would be more appropriate according to its functionality, I think, and would avoid confusions with the CollectorName field.

Text analyzer looks for a file under textAnalyzer.CollectorName/FileName, which after the modifications could be completed without confusion. If you are okey with this I will start making the changes in the code and the docs (which I think have some errors, e.g. I think that the name field is described as CollectorName field).

[manavellamnimble]

@laverya @dexhorthy I dug further into this, and found that in the Run collector, CollectorName is actually used as PodName, and Name is use as the root folder of the log file. I will change it to make CollectorName the root folder, and I will change name to podName in the specs and make it be the pod's name. That will leave the file in CollectorName/pod.Name. log CollectorName/pod.Name/container.log, which I think is more reasonable and fits the format used by other collectors.

[manavellamnimble]

New example would look like this:

apiVersion: troubleshoot.sh/v1beta2
kind: Preflight
metadata:
  name: example
spec:
  collectors:
    - run:
        collectorName: "run-ping"
        image: busybox:1
        podName: ping
        namespace: default
        command: ["ping"]
        args: ["-w", "10", "-c", "10", "-i", "0.3", "www.google.com"]
  analyzers:
    - textAnalyze:
        checkName: "run-ping"
        fileName: "ping.log"
        regexGroup: '(?P<Transmitted>\d+) packets? transmitted, (?P<Received>\d+) packets? received, (?P<Loss>\d+)(\.\d+)?% packet loss'
        outcomes:
          - pass:
              when: "Loss < 5"
              message: Solid connection to google.com
          - fail:
              message: High packet loss

[MikaelSmith]

One thing I'd really like is that there's a way to define the collector and analyzer that works for both before and after your changes. Otherwise it's going to be painful to support folks on two different versions of KOTS.

[manavellamnimble]

@MikaelSmith Hi! Are you referring to the change of the name field to podName? or to the order in the filePath of the run collector logs? In the first case, I can make the programm to parse both name and podName to the same variable, and that would not be a problem. In the second case, I think the best way would be to leave it as @laverya suggested, using the full path in the text analyzer, and maybe make the docs clearer.

[MikaelSmith]

I haven't thought through your proposed changes enough to have a specific problem. I was stating a possible requirement that would help smooth upgrades.

[manavellamnimble]

@MikaelSmith Great, thank you! I will think about it

[MikaelSmith]

Maybe this requires a change in support bundle and troubleshoot api version? I guess that would handle the migration issue.