Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Ubuntu -> Debian SID #17305

Merged
merged 8 commits into from
Feb 22, 2023

Conversation

cevich
Copy link
Member

@cevich cevich commented Jan 31, 2023

Ref: containers/automation_images#250

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none labels Jan 31, 2023
@cevich cevich force-pushed the swap_ubuntu_debian branch 4 times, most recently from 30bcbc8 to 4500b9a Compare January 31, 2023 21:14
@cevich
Copy link
Member Author

cevich commented Jan 31, 2023

@edsantiago ping - any chance you've seen this or a similar error (log) before:

...cut...
+ make completions  # ./contrib/cirrus/postbuild.sh:25 in main()
...cut...
# key = shell, value = completion filename
declare -A outfiles=([bash]=%s [zsh]=_%s [fish]=%s.fish [powershell]=%s.ps1);\
for shell in ${!outfiles[*]}; do \
    for remote in "" "-remote"; do \
	podman="podman$remote"; \
	outfile=$(printf "completions/$shell/${outfiles[$shell]}" $podman); \
	./bin/$podman completion $shell >| $outfile; \
    done;\
done
/bin/sh: 1: Syntax error: "(" unexpected
make: *** [Makefile:446: completions] Error 2

Exit status: 2

NOTE: This is on a brand-new Debian SID VM, it seems highly-likely I've screwed something up in the setup.

@cevich cevich force-pushed the swap_ubuntu_debian branch from 4500b9a to 7e725d5 Compare January 31, 2023 21:25
@edsantiago
Copy link
Member

I was just looking at this PR. The /bin/sh concerns me: it really should be bash. Can you think of somewhere that is using sh instead of bash?

Also, Cirrus or VMs are acting weird today. example

@cevich
Copy link
Member Author

cevich commented Jan 31, 2023

Can you think of somewhere that is using sh instead of bash?

When this was ubuntu, it used something called "dash" which it went to great lengths to pass-off as "bash". I just double-checked, and this is something I removed from the debian setup. Maybe this is to blame?

Also, Cirrus or VMs are acting weird today.

Okay, that's proper W-3-1-R-|). Like a git-clone silently failed to obtain all the files....oh! Look at the WebUI for that task. it show "skipped" for "clone", "setup" and "main". That can't be right. Ahh, at the top it says "PR was closed!" So maybe this was simply a task being canceled at a strange point?

@cevich
Copy link
Member Author

cevich commented Jan 31, 2023

Maybe dash is it. I looked in a "rerun with terminal" and found:

root@cirrus-task-6633121018281984:/# echo $CIRRUS_SHELL
/bin/bash
root@cirrus-task-6633121018281984:/# ls -la /bin/sh
lrwxrwxrwx 1 root root 4 Jan  5 13:20 /bin/sh -> dash
root@cirrus-task-6633121018281984:/# 

On Fedora, /bin/sh -> bash, so perhaps the Makefile is using "sh" but treating it as if it's really bash?

@cevich
Copy link
Member Author

cevich commented Jan 31, 2023

Ahh, yup make defaults to /bin/sh if $SHELL is unset. So that's a simple fix I think - define SHELL in our Makefile.

@cevich
Copy link
Member Author

cevich commented Feb 1, 2023

Well that's good news, only 12 test failures and the problem/solution is rather obvious (log):

msg="aardvark-dns binary not found, container dns will not be enabled"

I checked the build logs, and indeed the podman package only brings in netavark, not aardvark-dns.

@cevich
Copy link
Member Author

cevich commented Feb 1, 2023

@siretart ping - On SID, it seems installing the podman package doesn't bring in all necessary dependencies. Debian SID CI VM Image build-log shows it only installs netavark and not aardvark-dns. The script that sets up the image mentions 'podman' and assumes that will haul in everything necessary. Is this an incorrect assumption for Debian?

The netavark and aardvark-dns packages should be basically locked-together-at-the-hip together. They should always be installed together, with matching versions. Podman won't work without both, or the alternative containernetworking-plugins.

Can you help fix this or is there some packaging policy I'm misunderstanding for Debian?

@cevich cevich force-pushed the swap_ubuntu_debian branch from 7e725d5 to a67e6cc Compare February 1, 2023 18:21
@siretart
Copy link
Contributor

siretart commented Feb 2, 2023

@cevich I believe you found and oversight on my part, I need to add a package relationship from netavark to aardvark-dns.

Can you please help me decide whether this should be an absolute, unbreakable dependency (meaning, you can't install netavark witout aardvark-dns) or wither there are some situation where instaling netavark makes sense not having aardvark-dns installed (in which case, the relationship should be a recommends)?

@cevich
Copy link
Member Author

cevich commented Feb 2, 2023

Can you please help me decide

Happy to, with the caveat that I'm not a packaging-guy. IMHO, they should be locked-at-the-hip together. Their versions need to 100% match and they should always be installed/uninstalled/upgraded together. They can safely coexist with containernetworking-plugins, but that's a legacy solution. For new installations, nv/av should be preferred unless the user explicitly wants containernetworking-plugins.

Does that help?

@lsm5 or @Luap99 anything to add?

@Luap99
Copy link
Member

Luap99 commented Feb 2, 2023

@siretart
You should be able to install netavark without aardvark-dns, aardvark is only needed when a users wants dns for their container names. Almost all people want this so it should be installed by default, I assume this maps to an recommends in packaging terms.

@cevich cevich force-pushed the swap_ubuntu_debian branch from a55d740 to eef1b82 Compare February 2, 2023 20:03
@cevich
Copy link
Member Author

cevich commented Feb 2, 2023

@giuseppe got a strange one here: The new debian-12 VM is using CgroupsV1 and runc. The podman play kube with disabled cgroup test behaves counter-intuitively (to me at least). The results are:

@cevich
Copy link
Member Author

cevich commented Feb 2, 2023

@containers/podman-maintainers PTAL this odd failure, seems (to me) like it shouldn't have anything to do with runc-vs-crun. Regardless, it reproduces on int podman debian-12 root host and int remote debian-12 root host too:

Edit: Oh, also fails in the rootless debian test.

podman uidmapping and gidmapping with an idmapped volume
...cut...
         # podman [options] run --uidmap=0:1:500 --gidmap=0:200:5000 -v my-foo-volume:/foo:Z,idmap alpine stat -c #%u:%g# /foo
         #65534:65534#
...cut...
           Expected
               <string>: #65534:65534#
           to contain substring
               <string>: #0:0#
...cut...

Also (bad): I don't see any documentation for using an idmap option under the --volume man page section. It does show up for --mount so maybe the test code is wrong?

@edsantiago
Copy link
Member

Package versions:

Kernel:  6.1.0-3-cloud-amd64
Cgroups:  tmpfs
dpkg-query: no packages found matching containers-common
dpkg-query: no packages found matching cri-o-runc
conmon-2.1.3+ds1-1-amd64
containernetworking-plugins-1.1.1+ds1-3+b1-amd64
criu-3.17.1-2-amd64
crun-1.5+dfsg-1+b1-amd64
golang-2:1.19~1-amd64
libseccomp2-2.5.4-1+b3-amd64
podman-4.3.1+ds1-5+b1-amd64
runc-1.1.4+ds1-1+b1-amd64
skopeo-1.9.3+ds1-1-amd64
slirp4netns-1.2.0-1-amd64

@cevich
Copy link
Member Author

cevich commented Feb 2, 2023

Thanks Ed. I smell on the winds of change: "Upgrade runc" 🤣

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2023
@siretart
Copy link
Contributor

siretart commented Feb 3, 2023

@siretart ping - On SID, it seems installing the podman package doesn't bring in all necessary dependencies. Debian SID CI VM Image build-log shows it only installs netavark and not aardvark-dns.

I've fixed this oversight in version netavark_1.4.0-3, also cf. https://tracker.debian.org/netavark

Thanks for pinging me. If I miss a ping in the future, please file a bug in Debian.

@cevich
Copy link
Member Author

cevich commented Feb 3, 2023

Thanks for pinging me. If I miss a ping in the future, please file a bug in Debian.

Will do, thanks for the quick fix and update.

@cevich cevich force-pushed the swap_ubuntu_debian branch from 8c37e52 to 3f592a0 Compare February 6, 2023 20:43
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2023
@cevich cevich force-pushed the swap_ubuntu_debian branch from 3f592a0 to c048b99 Compare February 7, 2023 21:51
@cevich
Copy link
Member Author

cevich commented Feb 21, 2023

Ugh...and more 17598 and 17216 again (re-runs). These two are bad! Thanks @edsantiago for tracking them down.

@edsantiago
Copy link
Member

Sigh. pasta tests failing repeatedly despite multiple reruns. @cevich I think you'll need to rebuild VMs and downgrade pasta: older version fails, but it's a flake; this version is a hard fail. Issue being tracked in #17598.

@edsantiago
Copy link
Member

@cevich there's a new pasta build in progress.

There are also at least two in-flight PRs that I am blocking from merging, because they are runc-related and I don't want more CI disasters.

Building new VMs with new pasta will take time and (sigh) introduce new surprise breakages, maybe pasta, maybe some other package.

PROPOSAL: just skip all the pasta tests in this PR (or maybe just the right subset, if you can confidently identify those). Get this to pass, merge it, then do pasta cleanup in a followup PR. That will allow the stuck runc PRs to proceed, and will give time to debug new VMs.

@cevich
Copy link
Member Author

cevich commented Feb 22, 2023

Thanks @edsantiago working on en-mass skipping now w/ ref: #17598

When undefined make defaults to `/bin/sh` which is *NOT* the same on all
platforms.  For example, on Fedora it's a symlink to `/bin/bash` but on
Debian, it's a symlink to `/bin/dash`.  Remove any/all ambiguity by
declaring the shell to be bash forever and evermore.

Signed-off-by: Chris Evich <[email protected]>
* Skip play-kube test when runc is in use containers#17436
* Skip uid/gidmapping idmapped-volume test containers#17433

Signed-off-by: Chris Evich <[email protected]>
Test emits nasty warning message:
`Resource limits are not supported and ignored on cgroups V1 rootless
systems`

Ref: issue containers#17582

Signed-off-by: Chris Evich <[email protected]>
Test is completely broken, see buildah issue 4396.

Thanks to @edsantiago for the patch.

Signed-off-by: Chris Evich <[email protected]>
A horrible timeout-flake exists in the version presently in CI VM images
`c20230221t162829z-f37f36d12`.  Since the PR for adding the 2023-02-21
images is more urgently needed (containers#17305) than a pasta fix, skip all pasta
tests while waiting for a fix.

Signed-off-by: Chris Evich <[email protected]>
Also remove disused `gitlab` test setup.  This test was disabled a
while ago and is unlikely to ever be revived.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the swap_ubuntu_debian branch from 2f40e47 to 93e7cc1 Compare February 22, 2023 15:55
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2023
@cevich
Copy link
Member Author

cevich commented Feb 22, 2023

Force-push: Rebased, resolved merge conflict, re-worded temp. -> perm. test skip, added skip for all pasta tests.

@cevich
Copy link
Member Author

cevich commented Feb 22, 2023

@cevich
Copy link
Member Author

cevich commented Feb 22, 2023

@containers/podman-maintainers CI is Green!

@rhatdan
Copy link
Member

rhatdan commented Feb 22, 2023

/lgtm
/approve
/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 22, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2023
@openshift-merge-robot openshift-merge-robot merged commit efbc356 into containers:main Feb 22, 2023
@github-actions 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 8, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants