-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Install podman-systemd.unit man page, make quadlet discoverable #17351
Conversation
@lsm5 PTAL This needs to be added to podman-4.4.1 |
@lsm5 Could you verify that I did the podman.spec.rpkg correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/cherry-pick v4.4. |
@vrothberg: once the present PR merges, I will cherry-pick it on top of v4.4. in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
e72bb92
to
dc160af
Compare
I want to remove the podman-quadlet subpackage. it should be installed by default and is not big enough to require a separate package. @lsm5 I probably screwed up the spec file. |
podman.spec.rpkg
Outdated
@@ -240,6 +230,9 @@ fi | |||
%{_bindir}/%{name} | |||
%dir %{_libexecdir}/%{name} | |||
%{_libexecdir}/%{name}/rootlessport | |||
%{_libexecdir}/%{name}/quadlet | |||
%_prefix/lib/systemd/system-generators/podman-system-generator | |||
%_prefix/lib/systemd/user-generators/podman-user-generator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are easier:
%{_systemdgeneratordir}/%{name}-system-generator
%{_systemdusergeneratordir}/%{name}-user-generator
I don't know if rhel 8 and 9 have these defined already, let me take a quick look and get back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, these 2 macros should work on rhel8 and 9.
@@ -84,6 +84,7 @@ Requires: iptables | |||
Requires: nftables | |||
Recommends: catatonit | |||
Suggests: qemu-user-static | |||
Conflicts: quadlet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also have a Provides: %{name}-quadlet
here if we are removing the subpackage. An obsoletes would also be nice, but I gotta figure out the last build that shipped a separate podman-quadlet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me disable podman autobuilds and get you the last NVR to add here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add these as well.
Obsoletes: %{name}-quadlet <= 101:0.0.git.17877.f247b4d4-1
Provides: %{name}-quadlet = %{epoch}:%{version}-%{release}
I will resume podman autobuilds after this merges.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhatdan could you add these please? That should keep upgrades smooth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been addressed? See Below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, my bad. LGTM
I don't see any additional systemd unit installed, there's only the installation of the unitfile manpage, among other things. |
@rhatdan so looks like I should wait for v4.4.1 to move quadlet files into the main package on Fedora? |
Currently we are shipping no data about quadlet, since the podman-systemd.unit file is not shipped. Also want to add the quadlet name to the description of the man page so that man -k quadlet will help users find the man page. Also add a link file such that if the user types in man quadlet man will show the podman-systemd.unit file. Also eliminate the subpackage podman-quadlet Fixes: containers#17349 Signed-off-by: Daniel J Walsh <[email protected]>
Sort options alphabetically Add kubernetes example. Signed-off-by: Daniel J Walsh <[email protected]>
@lsm5 PTANOTHERLOOK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go after https://github.com/containers/podman/pull/17351/files#r1095784822 is addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/hold
Since this was addressed, I am going to merge. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lsm5, rhatdan, 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:
Approvers can indicate their approval by writing |
/hold cancel |
rebuilds enabled. |
@vrothberg: cannot checkout In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherry-pick v4.4 |
@ashley-cui: new pull request created: #17404 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Currently we are shipping no data about quadlet, since the
podman-systemd.unit file is not shipped. Also want to add the
quadlet name to the description of the man page so that
man -k quadlet
will help users find the man page.
Also add a link file such that if the user types in
man quadlet
man will show the podman-systemd.unit file.
Also eliminate the subpackage podman-quadlet
Fixes: #17349
Signed-off-by: Daniel J Walsh [email protected]
Does this PR introduce a user-facing change?