Cirrus: Simplify log collection commands

[cevich]

Signed-off-by: Chris Evich [email protected]

[mheon]

Changes LGTM

[cevich]

note: I plan on doing a pass across the entire .cirrus.yml, but need to verify some cirrus-ci YAML parsing behavior. IIRC, there was also some strange/unexpected interactions between always/on_failure instructions. Need to instrument some test failures to confirm.

[edsantiago]

Ah - yes, I see those other instances now. Thanks.

[cevich]

Ugg, yep, cirrus (I think) YAML parser does really odd/nonstandard things with quotes.

cat: '||': No such file or directory
cat: echo: No such file or directory
cat: "'Failed": No such file or directory
cat: to: No such file or directory
cat: collect: No such file or directory
cat: ginkgo: No such file or directory
cat: node: No such file or directory
cat: "logs'": No such file or directory

[edsantiago]

In the removed code, the commands were enclosed in single-quotes:

   failed_df_script: '${DFCMD}'

In the added code, no quotes. Should you try adding them back?

[cevich]

Naa, taking a different approach. It's the quote-handling that's the problem. I've wasted time on this topic before, it's not worth the aggravation...

[cevich]

@edsantiago does this approach seem reasonable? IMHO it rings a bit of overkill, but perhaps is good for future growth and maintenance. The absence of ginkgo node-logs in some tasks is expected (I think) - depends on what the makefile does.

[edsantiago]

@cevich yes, this approach makes sense and looks good. But why not take the next step, and just call the script logcollector or something, and make it do everything in just one pass? Every single instance I see has four invocations, all of which are duplication, not to mention the complexity of the script itself; why not just:

    always:
        log_collector_script: '${WHATEVER}'

[cevich]

Ack on name change...I always struggle with these.

Reason for duplicate calls is because it splits the output up in the WebUI and in the backend storage. Also because Cirrus truncates the output, we want to keep it as short as possible to avoid additional clicks to access it.

[edsantiago]

Mostly LGTM. One bug (unused code), one suggestion.

[edsantiago]

I hate to suggest making this five duplicated stanzas... but I find myself just now desperate for the output of ./bin/podman info on the rootless test; in particular, the .store settings. Is there some non-complicated way of adding that?

[cevich]

I find myself just now desperate for the output of

All rooless testing happens from the special_testing_rootless_task now, so adding a new script item like $SCRIPT_BASE/logcollector.sh podman there would do it. That also fixes the issue of stanza duplication, one of them would be slightly different 😁

five duplicated stanzas

Actually, now that you mention it, I wonder if Cirrus's customized parser can handle YAML anchors and aliases? I'll give that a try, but leave special_testing_rootless_task alone in anticipation of your addition.

[cevich]

@edsantiago anchors/aliases appear to work! I even left a comment on why rootless testing doesn't use it 😄

[edsantiago]

Excellent! Thank you for looking into it!

[cevich]

Thanks @edsantiago for helping me remember about YAML anchor/alias. I'm happy to see Cirrus-CI's custom parser supports them. If it weren't my last day before vacation, I'd be tempted to open a PR and apply that trick all over 😁

[cevich]

(this PR is ready to go)

[rhatdan]

/approve
You need to remove WIP if you want it to merge.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

/lgtm