buildah bud tests: better handling of remote

[edsantiago]

When I first enabled buildah-bud tests under podman-remote (#9887),
I got one aspect all wrong: I added a podman-remote() helper function
to match the podman() one. Turns out it's never actually called,
even when $PODMAN_BINARY=podman-remote, because functions/aliases
don't work that way.

The way it works is, those few cases in which bud.bats runs
podman are not magically remapped to podman-remote, they use
the podman() function. That's where we need to check if
we're using podman-remote, and that's where we need to
remove the registry-and-rootdir options.

With this fix, we can reenable two previously-skipped bud tests.

Signed-off-by: Ed Santiago [email protected]

None

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

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

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

[mheon]

LGTM

[rhatdan]

/lgtm
Couldn't we just fix Buildah to use the podman commands that are now in delta?

[edsantiago]

I'm not sure I understand your question. If I interpret correctly, you're asking if we can change the buildah tests themselves? This is one of the places that calls podman:

https://github.com/containers/buildah/blob/3cfa17a69564a7a3968a5b4671f6c203c9ec1eda/tests/bud.bats#L5238-L5243

(the other is almost identlcal). How would you propose changing those tests?

[edsantiago]

...and why does github sometimes show a nice inline excerpt of the lines in question, and other times just a plain link?

[rhatdan]

I am saying why not apply

 function podman() {
-    command ${PODMAN_BINARY:-podman} ${PODMAN_REGISTRY_OPTS} ${ROOTDIR_OPTS} "$@"
+    local cmd=${PODMAN_BINARY:-podman}
+    local opts="${PODMAN_REGISTRY_OPTS} ${ROOTDIR_OPTS}"
+    if [[ $cmd =~ remote ]]; then
+        opts=
+    fi
+    command $cmd $opts "$@"
 }

+function podman-remote() {
+    command ${PODMAN_BINARY:-podman-remote} ${ROOTDIR_OPTS} "$@"
+}
+

To builders/test/helpers.bash

[edsantiago]

Oh! Separation. My thinking at the time was, I don't want buildah knowing anything about podman-remote or about the test-buildah-bud-under-podman environment. Revisiting my decision just now... I still think it stands. I know it's ugly to have these diffs in the podman repo, but I find it uglier to have that code in buildah where it will never be needed. I'm willing to hear arguments in favor, though. @nalind @flouthoc WDYT?

[edsantiago]

ping, @nalind @flouthoc, ^^

The question is: should the podman-remote diffs be incorporated into buildah/tests/helpers.bash instead of carried in podman? My inclination is no, because buildah shouldn't be responsible for hackery done on the podman side. Also no because unpredictable future changes in podman may require different diffs. But I'd value your opinions.

[flouthoc]

My personal vote will be no for this, since in ideal world buildah should not be even aware about podman except the public api changes. We already use this philosophy in code I think tests should do the same, shouldn't it ? But this is just my opinion and if we have a strong reason to do otherwise then I'm cool with it. @nalind WDYT ?