buildah: increase rootless test coverage with rootless + unshare

[flouthoc]

Tests for rootless added here #3804 skipped some of the cases.

Almost all of the skipped test cases can be covered under an environment of rootless + unshare.
Following PR attempts to solve that by modifying test infrastructure and adding a cirrus task dedicated to rootless+unshare testing.

[edsantiago]

I think there's some confusion here. What I meant to do with my patch is eliminate the need to run the whole entire thing unshared.

With my patch, you can remove a lot of the skips: the tests will pass under plain simple rootless. unshare is only used for buildah mount/unmount, so it's a better test of actual rootless.

Running the entire test suite under unshare does not seem terribly useful to me

[flouthoc]

@edsantiago Yes. Actually this is intentional. My motive is to perform isolated testing for rootless and rootless+unshare in different environments cause they both are testing different things.

With this we can make sure that user namespacing is working correctly.

[edsantiago]

I have no real idea what that means, but given what I see, you do not need my patch for anything: my patch was intended to make more tests pass. You have not removed any skips, you have only added more skips. That is the opposite of my intention.

Please either remove skips, or remove my patch.

[flouthoc]

I have no real idea what that means, but given what I see, you do not need my patch for anything: my patch was intended to make more tests pass.

@edsantiago I think this we can still do this. Scratch my previous comment and let me try the original patch which you posted again since it only works for mount / unmount i think it does what we are trying to achieve.

[flouthoc]

@edsantiago Thanks 😄 . I looked at it again and re-used your patch as it is and this enables us to remove skip from almost all of the rootless tests some are still skipped but they are more of a root user test so we should be good.

@rhatdan @cevich @nalind @edsantiago PTAL

[edsantiago]

Thank you @flouthoc, that is exactly what I intended.

My patch was hasty and did not include comments; could you please add some for benefit of future maintainers?

[edsantiago]

Oh, one more. Since there's no longer the need for multiple confusing rootless_this_and_that, could you just rename everything to skip_if_rootless?

[flouthoc]

@edsantiago Thanks added requested comments.

Oh, one more. Since there's no longer the need for multiple confusing rootless_this_and_that, could you just rename everything to skip_if_rootless?

I agree I think we can merge all existing function into one. I don't know why did we add $BUILDAH_ISOLATION" = "rootless" check and skip_if_rootless_and_cgroupv1 we might not even need all of this now. But I just want to go through commit history once before removing these. But since this look unrelated to this PR I'll do that in a new PR after reading commit history.

[cevich]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

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

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 [cevich,flouthoc]

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

[cevich]

Oh, and super-big thanks for doing this, it really helped everyone out a lot, especially me 😁

[edsantiago]

Language suggestions. Also, I have a medium-strong preference for eighty-column formatting especially in comments. I don't think we have a style guide, so I will not make my preference a blocker.

I see what you mean re: skip_if_rootless but we seem to have a very very bad situation that needs to be fixed: the existing skip_if_rootless bears no relation whatsoever to the is_rootless() function, and that is going to cause a lot of confusion one day (including today). That would be a very good topic for a followup PR.

[flouthoc]

Restarted flakes.

[rhatdan]

/lgtm