infra container: replace pause with catatonit #12218

vrothberg · 2021-11-08T13:31:47Z

Podman has been using catatonit for a number of years already.
Thanks to @giuseppe, catatonit is now able to run as a pause
process which allows us to replace the pause binary entirely.

[NO NEW TESTS NEEDED] since no functionality has been added.

Signed-off-by: Valentin Rothberg [email protected]

Luap99

LGTM
We have to highlight this in the release notes to make sure catatonit is a hard dependency in all distros.

giuseppe

LGTM

openshift-ci · 2021-11-08T13:41:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, vrothberg

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~~ [giuseppe,vrothberg]

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

vrothberg · 2021-11-08T14:09:05Z

Probably need to update the VMs and images to include catatonit 0.1.7. @cevich WDYT?

rhatdan · 2021-11-08T15:33:23Z

I updated fedora to use newer catatonit.

cevich · 2021-11-08T16:01:42Z

Roger, I'll add it to my list.

vrothberg · 2021-11-08T16:05:01Z

Thanks, @cevich. I think we can also rerun hack/install_catatonit.sh on the images

cevich · 2021-11-08T16:14:27Z

I think we can also rerun hack/install_catatonit.sh on the images

Yep, that's exactly what we do. So I'll just bump it up in podman, and the images will cache it for us as I build them.

Ref: containers#12218 Signed-off-by: Chris Evich <[email protected]>

vrothberg · 2021-11-09T12:31:52Z

@cevich do we need to change the images or shall we force-install catatonit?

pkg/specgen/generate/pod_create.go

cevich · 2021-11-09T20:11:10Z

do we need to change the images or shall we force-install catatonit?

Nope, no new images needed - they only cache catatonit based on podman main-branch. So forcing it (as my PR did) is sufficient. It'll just run a few seconds slower until new VM images pick up the change and cache it for us.

vrothberg · 2021-11-10T08:59:08Z

do we need to change the images or shall we force-install catatonit?

Nope, no new images needed - they only cache catatonit based on podman main-branch. So forcing it (as my PR did) is sufficient. It'll just run a few seconds slower until new VM images pick up the change and cache it for us.

The script will only install catatonit if it's not already present on the host. Bumping the version will not automatically reinstall it if executed. Or am I missing something?

cevich · 2021-11-10T15:49:30Z

The script will only install catatonit if it's not already present on the host.

Oh crud, good catch! So we do need the new images then (I just started a build this morning). I believe I have all the testing-problems worked out for F35, but new ones could creep in at any moment 😞 So if that workflow ends up being "too slow" for this PR, I can work on adding a '--force' option to the installer script - since that will be more reliable than trying to do version-detection/conditional install.

vrothberg · 2021-11-10T15:51:37Z

@cevich, thanks for confirming. I can do the --force here and once we have new images, we can drop that.

cevich · 2021-11-10T15:55:02Z

Another possibility is #12256 (freshly built yesterday) which could go in before the F35 update (assuming tests pass).

vrothberg · 2021-11-10T16:15:05Z

Added another commit to force-install catatonit ✔️

cevich · 2021-11-10T16:37:57Z

.cirrus.yml

@@ -93,6 +93,9 @@ ext_svc_check_task:
              git reset --hard $CIRRUS_CHANGE_IN_REPO
          fi
          make install.tools
+          # FIXME: this is just a temporary workaround to force-install
+          # catatonit 0.17.0.  Please remove once the images are updated.
+          ./hack/install_catatonit.sh --force


edsantiago · 2021-11-10T16:51:16Z

@vrothberg test failures look real: I restarted both of them once (flake check) but both failed again, same way.

TomSweeneyRedHat · 2021-11-11T01:20:15Z

LGTM
assuming the tests get happy

vrothberg · 2021-11-12T11:13:42Z

It doesn't seem to install catatonit yet:

[+0002s] ./hack/install_catatonit.sh
[+0002s] skipping ... catatonit is already installed

vrothberg · 2021-11-12T11:19:41Z

Trying an alternative via contrib/cirrus/setup_environment.sh

vrothberg · 2021-11-12T11:52:06Z

APIv2 fail is legit. Fixed it already but I want to wait for e2e and system tests before pushing.

cevich · 2021-11-12T15:01:15Z

...confirmed, 0.1.7 is in the current VM images.

rhatdan · 2021-11-12T15:07:17Z

The spec file should be updated to require it.

vrothberg · 2021-11-12T15:12:22Z

The spec file should be updated to require it.

We have a chicken-egg problem or is catatonit 0.17.0 already available in F3{3,4,5}?

vrothberg · 2021-11-12T15:21:46Z

The spec file should be updated to require it.

Ah, already done in this PR. I will drop the force-install of catatonit now and hope VMs are good.

rhatdan · 2021-11-12T18:33:32Z

BTW F33 support should be dropped now.

siretart · 2021-11-12T21:40:57Z

Thanks for the heads up, turns out we already have it in Debian 😀

…

On Fri, Nov 12, 2021, 09:57 Valentin Rothberg ***@***.***> wrote: LGTM We have to highlight this in the release notes to make sure catatonit is a hard dependency in all distros. Heads up for packages: catatonit is now a hard-requirement for running Pods. @lsm5 <https://github.com/lsm5> @jnovy <https://github.com/jnovy> @siretart <https://github.com/siretart> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12218 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOKTJAHDBDSDO63HQXSIBTULUTOBANCNFSM5HSVVDXQ> .

vrothberg · 2021-11-13T10:12:20Z

@cevich, it doesn't seem to be in the Ubuntu 2110 images.

This reverts commit 9d2b8d2 since catatonit's new pause functionality can replace the `pause` binary entirely. Signed-off-by: Valentin Rothberg <[email protected]>

@giuseppe

Podman has been using catatonit for a number of years already. Thanks to @giuseppe, catatonit is now able to run as a pause process which allows us to replace the pause binary entirely. Signed-off-by: Valentin Rothberg <[email protected]>

A temporary workaround until the CI images are updated. Signed-off-by: Valentin Rothberg <[email protected]>

vrothberg · 2021-11-15T11:54:08Z

Went back to force-installing to unblock the PR.

vrothberg · 2021-11-15T13:13:20Z

@containers/podman-maintainers lets get this in

edsantiago · 2021-11-15T14:46:10Z

contrib/cirrus/setup_environment.sh

+# catatonit 0.17.0.  Please remove once the images are updated.
+./hack/install_catatonit.sh --force


What will this mean for bodhi gating tests? I see catatonit-0.1.6-1 on f34 and f35. Will 0.17.0 be packaged for those, or will this podman version never exist for f34/f35?

It's currently in testing for 34/35 -> https://bodhi.fedoraproject.org/updates/?search=catatonit

ack, thanks. Guess I better go play with it & give it karma.

edsantiago · 2021-11-15T16:51:08Z

I built this on f34 and f35. Tested using catatonit-0.1.6-1, tests fail. Then 0.1.7-1, tests pass. This is not an LGTM, I haven't done an actual code review; it's just a super-basic sanity check.

cevich · 2021-11-15T21:25:43Z

Went back to force-installing to unblock the PR.

Hmmm, pretty sure I remember grabbing a VM and checking it was there and --version said 0.1.7. I think I checked the F35 image only though.

$ podman run -it --rm quay.io/libpod/fedora_podman:c4955591916388352 /usr/libexec/catatonit/catatonit --version
tini version 0.1.7_catatonit

podman run -it --rm quay.io/libpod/ubuntu_podman:c4955591916388352
root@a94ec970614f:/var/tmp/go/src/github.com/containers/podman# type -p catatonit
/usr/bin/catatonit
root@a94ec970614f:/var/tmp/go/src/github.com/containers/podman# catatonit --version
tini version 0.1.5_catatonit

Aww nuts. I think in Ubuntu it must get installed from a package, then the hack script finds it and doesn't install the newer one. We don't mention the package by name during image build, so it must get pulled in by dependency 😢

cevich · 2021-11-15T21:31:22Z

Update: Yep, found it (repo). @lsm5 - something in kubik testing has a dependency on catatonit 0.1.5, could you update that and the package to 0.1.7?

cevich · 2021-11-15T21:35:14Z

@vrothberg a --force install from the script (to /usr/libexec/...) could be dangerous given the packaged version is installed in /usr/bin/. That means the tests or whatever is running in the VM is a $PATH sequence|presence away from using the wrong binary 😞

Let's see if @lsm5 can turn around a package update quickly, then I can rebuild the VM images (doing it for 'main' branch should be mostly painless).

edsantiago · 2021-11-16T17:03:46Z

/lgtm
/hold

I'm totally OK with this approach, just to get this PR in and give it testing time. There are plenty of update-the-VMs PRs in flight and upcoming, we can remove the install-catatonit hack soon.

rhatdan · 2021-11-16T17:15:52Z

/hold cancel

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 8, 2021

Luap99 reviewed Nov 8, 2021

View reviewed changes

giuseppe approved these changes Nov 8, 2021

View reviewed changes

cevich mentioned this pull request Nov 8, 2021

Bump Catatonit up to v0.1.7 #12223

Merged

cevich added a commit to cevich/podman that referenced this pull request Nov 8, 2021

Bump Catatonit up to v0.1.7

5fbc67f

Ref: containers#12218 Signed-off-by: Chris Evich <[email protected]>

vrothberg force-pushed the pause-catatonit branch from c8d51a0 to cee42a4 Compare November 9, 2021 08:51

vrothberg mentioned this pull request Nov 9, 2021

FindHelperBinary(): allow override via envariable containers/common#823

Merged

edsantiago reviewed Nov 9, 2021

View reviewed changes

pkg/specgen/generate/pod_create.go Show resolved Hide resolved

vrothberg force-pushed the pause-catatonit branch 2 times, most recently from 41fe528 to a0a2746 Compare November 10, 2021 16:11

cevich reviewed Nov 10, 2021

View reviewed changes

vrothberg force-pushed the pause-catatonit branch from a0a2746 to dda2b3f Compare November 12, 2021 11:19

vrothberg force-pushed the pause-catatonit branch from b3d675c to 50447c9 Compare November 12, 2021 15:22

vrothberg added 3 commits November 15, 2021 12:53

Revert "add kubernetes pause"

58cf0d4

This reverts commit 9d2b8d2 since catatonit's new pause functionality can replace the `pause` binary entirely. Signed-off-by: Valentin Rothberg <[email protected]>

infra container: replace pause with catatonit

5934e4c

Podman has been using catatonit for a number of years already. Thanks to @giuseppe, catatonit is now able to run as a pause process which allows us to replace the pause binary entirely. Signed-off-by: Valentin Rothberg <[email protected]>

cirrus: force-install catatonit

c8790bf

A temporary workaround until the CI images are updated. Signed-off-by: Valentin Rothberg <[email protected]>

vrothberg force-pushed the pause-catatonit branch from 50447c9 to c8790bf Compare November 15, 2021 11:53

edsantiago reviewed Nov 15, 2021

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2021

openshift-ci bot assigned edsantiago Nov 16, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2021

openshift-merge-robot merged commit 197ebe8 into containers:main Nov 16, 2021

cevich mentioned this pull request Nov 16, 2021

Cirrus: Bump Fedora to release 35 #11795

Merged

vrothberg deleted the pause-catatonit branch November 17, 2021 07:21

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023

		# catatonit 0.17.0. Please remove once the images are updated.
		./hack/install_catatonit.sh --force

infra container: replace pause with catatonit #12218

infra container: replace pause with catatonit #12218

Conversation

vrothberg commented Nov 8, 2021

Luap99 left a comment

Choose a reason for hiding this comment

giuseppe left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 8, 2021

vrothberg commented Nov 8, 2021

rhatdan commented Nov 8, 2021

cevich commented Nov 8, 2021

vrothberg commented Nov 8, 2021

cevich commented Nov 8, 2021

vrothberg commented Nov 9, 2021

cevich commented Nov 9, 2021

vrothberg commented Nov 10, 2021

cevich commented Nov 10, 2021

vrothberg commented Nov 10, 2021

cevich commented Nov 10, 2021

vrothberg commented Nov 10, 2021

cevich Nov 10, 2021

Choose a reason for hiding this comment

edsantiago commented Nov 10, 2021

TomSweeneyRedHat commented Nov 11, 2021

vrothberg commented Nov 12, 2021

vrothberg commented Nov 12, 2021

vrothberg commented Nov 12, 2021

cevich commented Nov 12, 2021

rhatdan commented Nov 12, 2021

vrothberg commented Nov 12, 2021

vrothberg commented Nov 12, 2021

rhatdan commented Nov 12, 2021

siretart commented Nov 12, 2021 via email

vrothberg commented Nov 13, 2021

vrothberg commented Nov 15, 2021

vrothberg commented Nov 15, 2021

edsantiago Nov 15, 2021

Choose a reason for hiding this comment

vrothberg Nov 15, 2021

Choose a reason for hiding this comment

edsantiago Nov 15, 2021

Choose a reason for hiding this comment

edsantiago commented Nov 15, 2021

cevich commented Nov 15, 2021

cevich commented Nov 15, 2021

cevich commented Nov 15, 2021 • edited Loading

edsantiago commented Nov 16, 2021

rhatdan commented Nov 16, 2021

cevich commented Nov 15, 2021 •

edited

Loading