Cirrus: More tests to verify cache_images

[cevich]

Depends on #3078 and #3239

CIRRUS: TEST IMAGES

Signed-off-by: Chris Evich cevich@redhat.com

[cevich]

note to me: The new image-check script should fail until #3192 merges

[cevich]

/hold

[cevich]

Note: Need to resolve mount-options issue first before merging this.

@edsantiago mind taking a peek while we wait? I'm not sure if I should unify the original check function you wrote, and the new one here (they're slightly different). Also, I'm sure you'll spot some "better ideas" for some of my bash-chicken-scratch 😄

[edsantiago]

I'm pretty swamped today so I only took a quick look at the topmost commit. test_okay is a little more complicated than I think it needs to be, but the test suite is comforting, thank you. Mostly lgtm. I did not review the other changes and cannot today.

[edsantiago]

I think you mean tail, not head

[edsantiago]

Actually 3 or 4

[edsantiago]

This doesn't work if the test is invoked as ./contrib/etc, or ./lib.sh.t from the same directory, etc. (And, the test name is completely weird).

[rh-atomic-bot]

☔ The latest upstream changes (presumably #3236) made this pull request unmergeable. Please resolve the merge conflicts.

[cevich]

Thanks Ed, yeah I know all about being swamped.

I'll see about simplifying test_okay (and read your other comments) later in the week.

[rhatdan]

@cevich Whats the latest on this PR?

[cevich]

Actually, @edsantiago would you mind taking over this PR? I'm going to be out on vacation next week, and having this additional validation merged will come in handy should images need to be updated/changed.

The most super-important thing here is the disk-space check, that was a really difficult problem to solve[*] and if it ever sneaks back in, introduces a nasty, non-obvious testing flake.

[*]Actual fix is embedded in the base-image, however it can be overridden anytime (by static or dynamic cloud metadata).

[edsantiago]

@cevich I was only able to get to this today, Thursday of a busy week, and it's taken me a long time to wrap my head around it. I think some of the code (mountopts) is no longer necessary. And, well, I just can't find a way to accept test_okay. It's a huge amount of complexity that could, IMO, be better handled by using the builtin test and better-worded diagnostics. When you get back, would you consider minimizing this a little, just making it the bare essentials?

[cevich]

It's a huge amount of complexity that could, IMO, be better handled by using the builtin test and better-worded diagnostics.

Yep I believe that, was hoping you might wield thy +12 sword of complexity-cleavage to achieve the same end-goal.

It's okay though, there are other tests that could/should be added anyway. Like confirming ooe.sh is installed/executable, and that the google systemd services are enabled on boot:

google-accounts-daemon.service             enabled        
google-clock-skew-daemon.service           enabled        
google-instance-setup.service              enabled        
google-network-daemon.service              enabled        
google-shutdown-scripts.service            enabled        
google-startup-scripts.service             enabled        

[cevich]

@edsantiago there, PTAL, I think you'll like this better.

[edsantiago]

LGTM, and much cleaner, thanks.

For the sake of sanity, could you change line 15 in the .t file?

-        echo "ok $testnum $3 = $MSG"
+        echo "ok $testnum $(echo $3) = $(echo $MSG)"

[edsantiago]

/lgtm

[cevich]

/hold cancel

[rhatdan]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

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