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: Use released F37 CI VM images #16692

Closed
wants to merge 3 commits into from

Conversation

cevich
Copy link
Member

@cevich cevich commented Nov 30, 2022

Signed-off-by: Chris Evich [email protected]

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 30, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cevich
Once this PR has been reviewed and has the lgtm label, please assign saschagrunert for approval by writing /assign @saschagrunert in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 cevich force-pushed the released_F37_images branch 8 times, most recently from 1f4b40a to 511a135 Compare December 7, 2022 18:27
@cevich cevich marked this pull request as draft December 7, 2022 18:28
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 7, 2022
@cevich cevich force-pushed the released_F37_images branch 5 times, most recently from 5defd5b to 00ffce6 Compare December 9, 2022 19:49
@cevich
Copy link
Member Author

cevich commented Dec 12, 2022

@adrianreber ping - I'm seeing this all over the place in our tests with CI VM Images built late last week:

could not load `libcriu.so.2`

This wasn't happening in my previous image update a week or so ago. Is there perhaps a broken package update that was released or did I screw something else up in the environment?

@adrianreber
Copy link
Collaborator

This sounds like it is related to crun changes by @giuseppe

@giuseppe any ideas why crun has problems loading the SO?

@giuseppe
Copy link
Member

could it be that the rpm is not part of the image?

@cevich is criu-libs installed?

@adrianreber
Copy link
Collaborator

adrianreber commented Dec 13, 2022

I guess the crun RPM needs a Recommends: criu-libs to have the functionality installed by default but if not wanted it can be removed.

Currently the crun RPM has a Requires: criu which just pulls in the binary but not the library. As the library is not detected automatically as it is only loaded on demand the automatic dependency parser of RPM cannot find it.

The best fix would be to change the crun spec file. The easy fix is to install criu-libs.

@giuseppe
Copy link
Member

@adrianreber should be both Recommends?

Recommends: criu >= 3.17.1
Recommends: criu-libs

@giuseppe
Copy link
Member

yes it seems to work fine with both dependencies, I'll fix that in the rpm

@adrianreber
Copy link
Collaborator

I think criu is automatically pulled in by libs, so you can drop the non-libs dependency if you want.

@cevich
Copy link
Member Author

cevich commented Dec 13, 2022

Thanks for taking a look guys. The CI VM builds scripts try not to spell out all the dependencies for exactly this reason: exposing dependency/requirement problems. Indeed, the build for these images only include criu and no libs.

I'm glad a fix will make it's way into the rpm, can I assume that should land in Fedora within a month or so?

Assuming so, it sounds like I need to rebuild images with a workaorund install of criu-libs. That's not a problem, I just want to make sure I comment the workaround appropriately.

@cevich cevich force-pushed the released_F37_images branch 4 times, most recently from 977f287 to 6e93f3f Compare December 15, 2022 16:11
@cevich
Copy link
Member Author

cevich commented Dec 15, 2022

@Luap99 you know about pasta, or am I confusing it with something else? Any idea why with a new-ish image update, I'm suddenly seeing these failures across all our rootless system tests?

Installing things from a make file is almost never a good idea.
Hard-coding paths to tools is also almost never a good idea.
Nevertheless, this is now done as of c782795 and very difficult to
undo.  Further it renders mute, attempts to optimize by pre-installing
tools during CI VM image production.  Many attempts were made to correct
this situation, all introduced further complexities and fragility.
Abandon all hope and simply use the hard-coded paths unless/until
somebody else has the mind to tackle this end-to-end.

Ref: containers/automation_images#240

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the released_F37_images branch from 6e93f3f to 98d57cf Compare January 10, 2023 15:26
@cevich
Copy link
Member Author

cevich commented Jan 10, 2023

Force-push: Rebased.

@cevich
Copy link
Member Author

cevich commented Jan 10, 2023

closing in favor of #16525

@cevich cevich closed this Jan 10, 2023
@cevich cevich deleted the released_F37_images branch April 18, 2023 14:52
@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 Aug 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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.

3 participants