Kube Play - set ReportWriter when building an image

[ygalblum]

Add test for a specific crash

Does this PR introduce a user-facing change?

No

None

Resolves: #20432

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ygalblum

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 [ygalblum]

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

[ygalblum]

The CI is failing because the play build tests expect a clean output while following this change, the output includes lines about the pulling of the image.
I think there are two ways to address this. Either adjust the test, or revert this change and instead check the ReportWriter before trying to write into the it in containers-common (to avoid the crash).
@vrothberg WDYT?

[vrothberg]

I am in favor of changing the tests. The build output is important to have IMO.

@rhatdan PTAL

[rhatdan]

Yes lets fix the test. Opts output is to stderr so can we segment based on this. Haven't looked but in BATS I know this is difficult. if not impossible.

[ygalblum]

Thanks. I was thinking about setting --quiet in the failing tests which should allow the tests to pass. However, I know realized that that if the user sets --quiet, the --writer passed on will be nil causing the crash this change tries to avoid.
So, I'll open a PR in containers-common to add a check on the writer. Once merged and picked up by Podman, we can address it here.

[umohnani8]

LGTM

[edsantiago]

Nasty red. Suggestion:

@@ -1480,2 +1480,3 @@ EOF
 
+    local untagged_image=quay.io/libpod/busybox
     local image_name=test_image
@@ -1489,3 +1490,3 @@ EOF
     cat >$container_file_path << EOF
-FROM busybox
+FROM $untagged_image
 EOF
@@ -1533,3 +1534,3 @@ EOF
     service_cleanup $QUADLET_SERVICE_NAME inactive
-    run_podman rmi $(pause_image)
+    run_podman rmi $untagged_image $image_name $(pause_image)
 }

(secondary, low-priority suggestion: rename variable image_name to built_image)

[ygalblum]

Done (both), thanks

[edsantiago]

Well, partly: $untagged_image is undefined, so there's still red. You can either take the other half of my suggestion above, or s/$untagged_image/busybox/. My reason for suggesting $untagged_image and setting it to a repo+image (untagged) is to prevent docker.io rate limiting on hosts like Fedora CI where they might not have our registries.conf overrides. I would like to prevent Fedora Gating flakes.

Obviously I tested quay.io/libpod/busybox on my end, using your PR as well as main (where the test fails with the expected crash) but if you go that route please confirm.

[ygalblum]

I've taken in all your comments including using quay.io/libpod/busybox. I also make sure that the image is not available on the local machine before running the test. I ran the test locally and it did not prompt about any stray images. So, hopefully, it's OK now.

BTW initially, I wanted to use a local registry instead of downloading a new image. However, building an image based on a private registry using kube play is not fully supported (See #20890 for details)

Anyhow, thanks a lot for your help (here and in general)

[rhatdan]

/lgtm

[lsm5]

@Luap99 @mheon @ashley-cui are we ok backporting this to v4.8 for v4.8.2?

[Luap99]

Looks like a bug fix so no objection from my side.

[lsm5]

/cherry-pick v4.8

[openshift-cherrypick-robot]

@lsm5: #20889 failed to apply on top of branch "v4.8":

Applying: Kube Play - set ReportWriter when building an image
Using index info to reconstruct a base tree...
M	test/system/252-quadlet.bats
Falling back to patching base and 3-way merge...
Auto-merging test/system/252-quadlet.bats
CONFLICT (content): Merge conflict in test/system/252-quadlet.bats
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Kube Play - set ReportWriter when building an image
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick v4.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.