Use full attach path, rather than a symlink

[haircommander]

conmon 2.0.24 (not yet released) will add the functionality to specify --full-attach, meaning the path to the attach socket will be accessed directly, rather than creating a symlink to a socketsDir. This not only drops a couple of unneeded operations, but also fixes issues that come up when a user runs podman with a sufficiently long UID (containers/conmon#225)

This PR does many things along those lines:

• use the specified helper function AttachSocketPath, rather than building the path from scratch
• drop the use of socketDir entirely, and add the --full-attach option to the conmon args, so neither podman nor conmon operate with the socketDir anymore
• drops ExecContainerCleanup method from the oci interface, as the only work it was doing was cleaning up the exec specific socket dir
• returns findConmon to podman (from c/common). I don't think moving it was the correct strategy, as it makes bumping the minimum required conmon bothersome (rather than coordinating two repos, we would have to coordinate three).
• bumps the minimum required conmon to 2.0.24 to take advantage of all of this

A couple of notes:

• I am open to not moving findConmon, but we do need some way to specify the minimum conmon version for podman. it can't live in c/common. The other strategy would be to change the FindConmon() method in c/common to take major, minor and patch versions, so the calling engine can configure it themselves. However, I find this approach is better for now, because no other engines actually use FindConmon to my knowledge.
• I am not excited about throwing all of the findConmon code in libpod/runtime. it muddles up an otherwise pretty straight forward file. If there's other suggestions for where to put it, please let me know.
• this obviously relies on 2.0.24 to exist, which it does not yet. You can look at the PR that caused all of this work here.

[haircommander]

properly fixes containers/conmon#225

[haircommander]

/hold

added a commit that should bypass the smoke test (I'm not sure how we are to test this, should I check where the old socket dir was?)

[haircommander]

@edsantiago @cevich is there any way to force the sys tests to run without int passing on all platforms?

[edsantiago]

Not that I know of, other than directly mucking with .cirrus.yml

[haircommander]

Not that I know of, other than directly mucking with .cirrus.yml

that seems clear now that you mention it. I am giving it a shot

[haircommander]

@edsantiago huzzah (?) it does fail on ubuntu :)

[edsantiago]

@haircommander I see a failure on sys podman ubuntu-2004, not 2010; and the failure doesn't look like a relevant one:

# # podman run --name mybigdatacontainer --attach stderr --log-driver k8s-file quay.io/libpod/testimage:20200929 dd if=/dev/zero count=700000 bs=1
# #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# #|     FAIL: dd: number of records in
# #| expected: '700000+0 records in'
# #|   actual: ''
# #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

That is: it never actually made it to the byte-count-check part of the test. Grrr.

[cevich]

@haircommander I think maybe you're just trying to temporarily get to the system tests sooner, for faster cycling/debugging purposes?

The easiest way to do this is going through and adding/changing skip: $CI == $CI to the tasks you don't want. Though I don't recommend doing this to the main build_task for (hopefully) obvious reasons 😄 It should work with nearly every other task in .cirrus.yml.

@edsantiago FYI ^^^

[haircommander]

I think the last piece missing is containers/conmon#241

[rhatdan]

/approve
LGTM
@containers/podman-maintainers PTAL
Thanks @haircommander

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, 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

[TomSweeneyRedHat]

LGTM
but would like a head nod from @mheon

[haircommander]

Just FYI there's a wip commit I have to drop before we merge this.. A related side note: what's the idiomatic way to skip adding a test these days?

On Saturday, April 17, 2021, Tom Sweeney ***@***.***> wrote: LGTM but would like a head nod from @mheon — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.<

https://ci4.googleusercontent.com/proxy/j2mpGoWS_CbXAYDs5mRgII_UA21vP512JtaG9qGkU0rvrP3iavfmEX2X5kUhIJ8RZRv9V4ci3X9bgTanx-cq_SKfT8YEJGk-dsMvKGTMdgseBCEUMg4CWihmprfAcnj2edqAHevVTgZGKFg7oRIyobx0sgtGpAHgjskvyP5tafkskfVBaVbrqcc2kYb-LJh9dDERh_34fBY-uYDLyUBm4yWiB-qDhmzDndDGzvTAYw=s0-d-e1-ft#https://github.com/notifications/beacon/ACFJQJZDZYWGUYZ6SSYMBK3TJGWOPA5CNFSM4WC6YV3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGD6F6OA.gif>

[edsantiago]

what's the idiomatic way to skip adding a test

If you're seeing a CI failure because of missing tests, the test failure itself should give you pretty clear instructions on what to do. If the instructions aren't clear, that's a bug, and PRs (or suggestions) welcome.

[mheon]

/lgtm

[rhatdan]

/hold cancel