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

CI VMs: bump to f39 + f38 #20162

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Conversation

edsantiago
Copy link
Member

...from f38 + f37.

Requires one minor e2e test change, to handle an error logging
change in conmon 2.1.8.

Also, this is important, requires crun-1.9.1 because of a kernel
symlink change; see containers/crun#1309
The VM images here were carefully built to include that. By the
time the next VM images get built, it should be default.

Signed-off-by: Ed Santiago [email protected]

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 27, 2023
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Changes LGTM

Restarted the two jobs which looked like flakes

@vrothberg
Copy link
Member

Windows Cross failure seems to persist but I do not understand it

@Luap99
Copy link
Member

Luap99 commented Sep 27, 2023

This version of pandoc has been compiled without Lua support.

Looks like the pandoc package was split into several packages https://hackage.haskell.org/package/pandoc-3.0/changelog

Seems to be already reported against the fedora package: https://bugzilla.redhat.com/show_bug.cgi?id=2238149
I guess to unblock this PR it would be possible to keep running this windows cross job on f38?

@edsantiago
Copy link
Member Author

Thank you for finding that, @Luap99. I saw "Windows" and my brain froze up.

Unless there is an urgent need to start testing on f39, I'm ok with waiting for a fixed pandoc. My VM-build PR is containers/automation_images#302 and it would be nice (IMO) to remove the special-case crun kludge by waiting a few days.

@cevich
Copy link
Member

cevich commented Sep 27, 2023

I guess to unblock this PR it would be possible to keep running this windows cross job on f38?

IIRC, the output from that job is consumed by the other windows tasks. I believe @ashley-cui is working on native builds for Windows, which can/should probably replace the cross-build. Is that you're eventual goal Ashley?

@cevich
Copy link
Member

cevich commented Sep 27, 2023

nice (IMO) to remove the special-case crun kludge by waiting a few days.

The F39 image should be using updates-testing, so may even be less that "a few days". I'm also waiting for this update to land in rawhide for containers/automation_images#309 and #19751

@cevich
Copy link
Member

cevich commented Sep 27, 2023

@edsantiago would you consider including the passthrough_envars() updates, in case my F38 update becomes mute? W/o it, there may be ambiguity over which passthrough_envars() is being used. Also a maintainer may become confused why their changes to PASSTHROUGH_ENV_RE are ineffective.

Also Paul's commit can be cherry-picked here safely.

@edsantiago
Copy link
Member Author

would you consider including the passthrough_envars() updates

Done (on my local branch).

Also Paul's commit can be cherry-picked here safely.

Done, ibid.

I am monitoring the pandoc BZ, also periodically checking bodhi for pandoc builds. If I see a build, I will rerun containers/automation_images#302 . If there's nothing by Monday, I will look into some other way to fix the Windows problem.

@ashley-cui
Copy link
Member

IIRC, the output from that job is consumed by the other windows tasks. I believe @ashley-cui is working on native builds for Windows, which can/should probably replace the cross-build. Is that you're eventual goal Ashley?

Yeah, eventual builds and tests should run on native Windows

@cevich
Copy link
Member

cevich commented Sep 27, 2023

Yeah, eventual builds and tests should run on native Windows

Great, you can get that implemented before Monday, right 😉 (kidding).

@cevich cevich mentioned this pull request Sep 27, 2023
@TomSweeneyRedHat
Copy link
Member

Changes LGTM
I'm not sure what to do given the Windows Cross failure.

@edsantiago edsantiago force-pushed the f39 branch 3 times, most recently from 2d63973 to dfccd2a Compare September 28, 2023 01:48
@edsantiago
Copy link
Member Author

Should this pass CI, here's what's important to review:

  • First and foremost, the CI-VM PR: Bump to Fedora 39 automation_images#302
    • It brings in crun-1.9.2-1 in a dirty way. This is an ends-justify-means kludge IMO. If you disagree, reject.
    • Even more so for pandoc, bringing in a scratch build of pandoc-with-lua in order to pass the Windows test
    • the rest of that PR should be noncontroversial. It might help to review as two commits (one of them is just s/egrep/grep -E/ because egrep is deprecated).
  • If the CI-VM PR looks good, then review this one:

As of this writing, Windows has passed (yay) which means pandoc works. I will be asleep long before any int/sys jobs complete.

Copy link
Member

@cevich cevich left a comment

Choose a reason for hiding this comment

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

LGTM with one small thing for consideration.

.cirrus.yml Outdated Show resolved Hide resolved
edsantiago and others added 3 commits September 28, 2023 11:22
...from f38 + f37.

Requires one minor e2e test change, to handle an error logging
change in conmon 2.1.8.

Also, this is important, requires crun-1.9.1 because of a kernel
symlink change; see containers/crun#1309
The VM images here were carefully built to include that. By the
time the next VM images get built, it should be default.

Since we've bumped crun, remove two obsolete skips

And, skip a flaky pasta test, containers#20170

Signed-off-by: Ed Santiago <[email protected]>
The `v4.3.1` version of the library defines a common
`passthrough_envars()` so it doesn't need to be duplicated in podman and
buildah CI.  It also includes an update to build-push which should make
debugging easier.

Finally, these images include setting of the en_US.UTF-8 locale to enable
removal of a workaroud in a future commit.

Signed-off-by: Chris Evich <[email protected]>
This reverts commit ed1f514.

The en_US.UTF-8 locale is now added in the images at build time,
containers/automation_images#295

Signed-off-by: Paul Holzinger <[email protected]>
Copy link
Member

@cevich cevich left a comment

Choose a reason for hiding this comment

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

LGTM assuming happy tests. Thanks Ed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 28, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@edsantiago
Copy link
Member Author

Tests are green again. @containers/podman-maintainers are we ready for f39?

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023
@openshift-merge-robot openshift-merge-robot merged commit 38f718e into containers:main Sep 28, 2023
97 of 98 checks passed
@edsantiago edsantiago deleted the f39 branch September 28, 2023 20:41
@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 Dec 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 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