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:BUILD] rpm: hard dependency on gvisor-tap-vsock-gvforwarder #19953

Closed
wants to merge 1 commit into from

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Sep 12, 2023

With containers/gvisor-tap-vsock#268, gvforwarder is now provided via the subpackage
gvisor-tap-vsock-gvforwarder. FCOS needs this gvforwarder, hence the hard dependency.

gvisor-tap-vsock is a separate package for f38, so the subpackage conditional has been adjusted to fit that.

[NO NEW TESTS NEEDED]

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 Sep 12, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lsm5

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 Sep 12, 2023
@lsm5
Copy link
Member Author

lsm5 commented Sep 12, 2023

depends on containers/gvisor-tap-vsock#268

@lsm5 lsm5 marked this pull request as ready for review September 12, 2023 19:36
@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 Sep 12, 2023
@lsm5
Copy link
Member Author

lsm5 commented Sep 12, 2023

@martinpitt revdeps failed. seeing this elsewhere?

@lsm5
Copy link
Member Author

lsm5 commented Sep 12, 2023

@martinpitt revdeps failed. seeing this elsewhere?

whoops ignore me. Looks like it's because of the change I made.

@lsm5
Copy link
Member Author

lsm5 commented Sep 12, 2023

The revdep tests should pass once the gvisor-tap-vsock change is merged.

@@ -37,7 +37,7 @@
# include it. Official rawhide should be able to fetch the last active build of
# gvproxy, the min version requirement has been removed to allow it.
# Ref: https://bugzilla.redhat.com/show_bug.cgi?id=2224434
%if !%{defined copr_username} && 0%{?fedora} <= 38
%if !%{defined copr_username} && 0%{?fedora} <= 37
Copy link
Member

Choose a reason for hiding this comment

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

is this version down grade correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that's intentional. gvproxy is an independent package for Fedora 38 and higher.

@martinpitt
Copy link
Contributor

@lsm5: Yes, "nothing provides gvisor-tap-vsock-gvforwarder". Nevertheless, big thanks for the notification! Wrt. that, automatic notifications have a high chance of landing next Tuesday 🎉 packit/packit-service#2182

@lsm5 lsm5 marked this pull request as draft September 13, 2023 14:00
@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 Sep 13, 2023
@lsm5
Copy link
Member Author

lsm5 commented Sep 13, 2023

We don't need to make the change for f37. So, I'll revise this PR a bit.

@lsm5 lsm5 force-pushed the gvforwarder branch 2 times, most recently from d16be77 to a5519cd Compare September 13, 2023 19:18
With containers/gvisor-tap-vsock#268,
gvforwarder is now provided via the subpackage
`gvisor-tap-vsock-gvforwarder`. FCOS needs this gvforwarder, hence the
hard dependency.

This change is only intended for f38 and higher.

[NO NEW TESTS NEEDED]

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5 lsm5 marked this pull request as ready for review September 13, 2023 19:24
@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 Sep 13, 2023
@lsm5
Copy link
Member Author

lsm5 commented Sep 14, 2023

the revdep failure should go away once containers/gvisor-tap-vsock#271 is merged. I didn't see any conflict with the rpms built on that PR and the one in fedora.

@rhatdan
Copy link
Member

rhatdan commented Sep 14, 2023

LGTM

@lsm5
Copy link
Member Author

lsm5 commented Sep 14, 2023

the gvisor PR has been merged. I'll wait a while and rerun the revdep tests here to ensure we're green.

@lsm5
Copy link
Member Author

lsm5 commented Sep 14, 2023

really weird why the rpm conflict still occurs on f39 but not on rawhide and f38. Looking..

@lsm5 lsm5 marked this pull request as draft September 14, 2023 20:06
@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 Sep 14, 2023
@martinpitt
Copy link
Contributor

@lsm5 : For the F38 run , vsock-gvforwarder was taken from your COPR:

 gvisor-tap-vsock-gvforwarder   x86_64  103:0.7.0-1.20230914182756041449.main.39.g9ebd937                copr:copr.fedorainfracloud.org:rhcontainerbot:podman-next  1.8 M

but for the F39 run it came from Fedora:

gvisor-tap-vsock-gvforwarder   x86_64  6:0.7.0-6.fc39                                                   updates-testing                                            1.8 M

Presumably because the podman-next COPR's most recent build is 6:0.7.0-1.20230914182756041449.main.39.g9ebd937 , i.e. the 1.2023.. is smaller than Fedora's -6 revision.

However, I don't know about these "6:" vs. "103:" prefixes. Are these epochs? But an epoch of "103:" sounds pretty absurd. Or are these priority= in the repo file? 🤔

@martinpitt
Copy link
Contributor

Our TF workaround hack landed, so feel free to retry all failed packit tests in recent PRs (I can't unfortunately)

@lsm5
Copy link
Member Author

lsm5 commented Sep 21, 2023

obsoleted by #20039 . f39 didn't have gvisor build earlier which is why the f39 test failed.

@martinpitt yes the 102/103 were set as absurdly high epochs for copr builds to override default packages. Is priority settable for copr repos when you enable them using dnf copr enable ? I can't seem to find any such option.

@lsm5 lsm5 closed this Sep 21, 2023
@lsm5 lsm5 deleted the gvforwarder branch September 21, 2023 14:02
@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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 21, 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. 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.

4 participants