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

Cirrus: Update CI VM images to F37beta #15760

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented Sep 12, 2022

Fixes: #15349
Depends on: #15871 #15890
Image build PR: containers/automation_images#195

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 12, 2022
@cevich
Copy link
Member Author

cevich commented Sep 12, 2022

@umohnani8 this PR also stubs-in a spot for you to setup and run your kube tests. I added it to run after the system-tests, but am not married to the order if you want it to run sooner. However, since this PR brings in a new Fedora version as well, it might get hairy depending on how CI is feeling today 😄 If it goes south, you're welcome to split out only the kube-additions into a separate PR using image-suffix c5167094042984448 from containers/automation_images#192 In other words, those images are the good 'ole F35 & F36. The important diffs for this are (edited):

As always, ping me if you need help.

@cevich cevich force-pushed the enable_kube_tests branch 3 times, most recently from 6b05ccd to 7205c5d Compare September 13, 2022 14:11
@umohnani8
Copy link
Member

Thanks @cevich!

Is this still WIP or can we get this merged in?

@cevich
Copy link
Member Author

cevich commented Sep 13, 2022

Is this still WIP or can we get this merged in?

That's the 10-billion dollar question. Historically, bringing in a new OS version into CI is fraught with "challenges". Sometimes though, it goes smoothly. So this PR is me rolling the dice and crossing my fingers.

Initially, the validate failures don't give me confidence 😢

@umohnani8
Copy link
Member

Looks like the package name for io/ioutil changed with the Go version bump, maybe updating the package name in the files it is complaining about will fix the issue.

@cevich
Copy link
Member Author

cevich commented Sep 13, 2022

maybe updating the package name in the files it is complaining about will fix the issue.

You're talking to a guy who dreams in bash 😀

I'm going to take a wild-guess that we're going to hit that same error in every single containers-repo 😢

@lsm5
Copy link
Member

lsm5 commented Sep 13, 2022

from rerun with terminal

# rpm -q golang
golang-1.19.1-1.fc37.x86_64

@lsm5
Copy link
Member

lsm5 commented Sep 13, 2022

I'm now hitting the can't load fmt issue open at golangci/golangci-lint#3107 both locally and on cirrus . Seems like there's a workaround at go-critic/go-critic#1126 (comment) but didn't work for me.

@cevich
Copy link
Member Author

cevich commented Sep 14, 2022

Crud, so @containers/podman-maintainers we're potentially going to see this pop up everywhere then until there's a fix and we bump all our golangci-lint version variables in every repo. 😞

@umohnani8
Copy link
Member

@cevich can we separate the fedora version bump from the addition of minikube so we don't block on that? I can open another PR with just the minikube changes

@cevich
Copy link
Member Author

cevich commented Sep 15, 2022

I can open another PR with just the minikube changes

Yes absolutely, please do this, and poke me if you run into trouble. My comment above links to the minikube-enablement bits you need. Pay careful attention to the image ID called out there, since everything built after that is using F36/F37.

@cevich
Copy link
Member Author

cevich commented Sep 15, 2022

Force-push: Removed the mini-kube addition.

@lsm5 lsm5 linked an issue Sep 19, 2022 that may be closed by this pull request
@cevich cevich force-pushed the enable_kube_tests branch 3 times, most recently from 936022c to 3344501 Compare September 20, 2022 19:42
@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Sep 20, 2022
@cevich cevich force-pushed the enable_kube_tests branch 2 times, most recently from 3f22fb9 to ab7898b Compare September 21, 2022 14:34
@github-actions github-actions bot removed the kind/api-change Change to remote API; merits scrutiny label Sep 21, 2022
@cevich cevich force-pushed the enable_kube_tests branch 3 times, most recently from 22f984f to aa8a296 Compare September 23, 2022 14:12
@cevich cevich force-pushed the enable_kube_tests branch from 1c6e10c to 5e51ee2 Compare October 19, 2022 17:37
@cevich
Copy link
Member Author

cevich commented Oct 19, 2022

Update: I re-ran a F36 system test task, all the others appear related to #16107

@rhatdan
Copy link
Member

rhatdan commented Oct 28, 2022

@cevich Any update on this?

@cevich cevich force-pushed the enable_kube_tests branch from 5e51ee2 to 95599a1 Compare October 31, 2022 16:58
@cevich
Copy link
Member Author

cevich commented Oct 31, 2022

c5178639502278656 has systemd 251.7-611

@cevich cevich changed the title [WIP] Cirrus: Update CI VM images to F37beta Cirrus: Update CI VM images to F37beta Oct 31, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2022
@edsantiago
Copy link
Member

rootless f36 is thinking that it's netavark:

[+0008s] # Arch:amd64 [...]  Cgroups:v2+systemd Net:netavark

I think we need a CI_DESIRED_NETWORKING mechanism like the RUNTIME one added in #14912.

I have an f36 VM but cannot find any way to make it CNI. I've tried rm -rf /var/lib/containers/*, I obviously created /etc/cni/net.d/87-blahblah, tried rebooting, nothing makes it go to CNI.

@Luap99
Copy link
Member

Luap99 commented Nov 2, 2022

The only way to force cni/netavark is via containers.conf network_backend option or --network-backend option for each podman command.

@edsantiago
Copy link
Member

OK, we need #15413 then.

Signed-off-by: Chris Evich <[email protected]>
Building Ubuntu VM images is temporarily broken due to dependency
problems on (missing) netavaro for the (required) podman package.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the enable_kube_tests branch from 95599a1 to 0d2acb1 Compare November 2, 2022 17:39
Comments added to code for clarity.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the enable_kube_tests branch from 0d2acb1 to 8530724 Compare November 2, 2022 17:42
@cevich
Copy link
Member Author

cevich commented Nov 2, 2022

Status: Finally Green!

I'm rolling up one final fresh set of images so I can remove a workaround Jason did for the new windows tests. I'll slap that in here when it's ready and then (:crossed_fingers:) this will be good to go (finally :sweat_smile:)

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One request (important, but I won't block over it) and one clarification/confusion question

@@ -227,6 +236,9 @@ use_netavark() {
export NETWORK_BACKEND=netavark # needed for install_test_configs()
msg "Removing any/all CNI configuration"
rm -rvf /etc/cni/net.d/*
# N/B: The netavark/aardvark-dns packages are still installed and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean CNI packages?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh woops, yes I did mean CNI. I'll fix it, thanks.

#PRIOR_FEDORA_CACHE_IMAGE_NAME: "prior-fedora-${IMAGE_SUFFIX}"
UBUNTU_CACHE_IMAGE_NAME: "ubuntu-${IMAGE_SUFFIX}"
PRIOR_FEDORA_CACHE_IMAGE_NAME: "prior-fedora-${IMAGE_SUFFIX}"
#UBUNTU_CACHE_IMAGE_NAME: "ubuntu-${IMAGE_SUFFIX}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a hecka lot of commented-out stuff here. Some day someone will need to add them back, and will miss one even if they do a git log -1 -p of this commit to make sure.

Could you add # FIXME: reenable once <BUGID> is resolved to each commented-out section?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I can/should add a summary comment. No problem. I thought I made this a separate commit so it can just be reverted, no?

I'm hoping to get the debian stuff rolling before too long, so hopefully that can replace Ubuntu entirely.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert assumes an unchanging .cirrus.yml, and that will be void within (I hope) one day of this PR merging.

@edsantiago
Copy link
Member

CI is green, I'm OK with merging this since it conflicts with my network-check PR. I will fix the comments there.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2022
@edsantiago edsantiago added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: cevich

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

@cevich
Copy link
Member Author

cevich commented Nov 3, 2022

Ugh...but...but....there was one more thing I needed to add...sigh...#16396

@edsantiago
Copy link
Member

Timing:

type distro user local remote container
int fedora-36 root 29:41 30:06 29:21
int fedora-37 root 26:19 31:16 26:01
int fedora-36 rootless 27:00
int fedora-37 rootless 25:23
sys fedora-36 root 24:47 16:36
sys fedora-37 root 25:14 16:26
sys fedora-37-aarch64 root 29:45 20:08
sys fedora-36 rootless 23:39
sys fedora-37 rootless 22:37 13:50
bud fedora-37 root 25:13 25:16

@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 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 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.

Cirrus: enable F37 testing
7 participants