Deprecate podman generate systemd #19414

rhatdan · 2023-07-28T15:00:31Z

Now that Quadlets are fully supported, it is time to Depracate podman generate systemd command.

Does this PR introduce a user-facing change?

The `podman generate systemd` command is deprecated.  Use Quadlet for running containers and pods under systemd.

rhatdan · 2023-07-28T15:00:42Z

@vrothberg PTAL

cmd/podman/generate/systemd.go

docs/source/markdown/podman-generate-systemd.1.md

vrothberg · 2023-07-31T07:26:44Z

docs/source/markdown/podman-generate-systemd.1.md

@@ -306,7 +310,7 @@ CONTAINER ID  IMAGE                            COMMAND  CREATED        STATUS
 bb310a0780ae  docker.io/library/alpine:latest  /bin/sh  3 minutes ago  Created                  busy_moser
 ```
 ## SEE ALSO
-**[podman(1)](podman.1.md)**, **[podman-container(1)](podman-container.1.md)**, **systemctl(1)**, **systemd.unit(5)**, **systemd.service(5)**, **[conmon(8)](https://github.com/containers/conmon/blob/main/docs/conmon.8.md)**
+**[podman(1)](podman.1.md)**, **[podman-container(1)](podman-container.1.md)**, **systemctl(1)**, **systemd.unit(5)**, **systemd.service(5)**, **[conmon(8)](https://github.com/containers/conmon/blob/main/docs/conmon.8.md)**, [Quadlet](https://github.com/containers/conmon/blob/main/docs/podman-systemd.unit.5.md)


Since quadlet docs are packaged with podman, can't we use the other syntax?

vrothberg

CI isn't happy yet:

hack/man-page-checker

Inconsistent subcommand descriptions:
podman-generate-systemd.1.md = 'Generate systemd unit file(s) for a container or pod'
podman-generate.1.md = '[DEPRECATED] Generate systemd unit file(s) for a container or pod'

Luap99

In principle I think this is the right direction, we should get users into quadlet.
However we should have actual docs on how to convert to quadlet before advertising this as deprecated. Just looking at podman-systemd.unit(5) is not super helpful as there is no description of a migration part.

Also AFAIK quadlet has no pod support besides k8s yaml and I don't agree that this is a perfect replacement. K8s yaml lacks some podman specific options so migration may not be possible for all users. And then everything is run as part of one unit which totally changes the way on how to monitor and interact with these units.

As soon as people know it is deprecated people will look into it and possibly get frustrated due lacking docs on what they should do. I know we have decent docs on what quadlet is and what options we support but as user I would expect some form step by step instructions on how to convert from the old units to quadlet.

cmd/podman/generate/systemd.go

Luap99 · 2023-08-01T11:04:56Z

cmd/podman/generate/systemd.go

@@ -119,6 +127,7 @@ func init() {
 }

 func systemd(cmd *cobra.Command, args []string) error {
+	fmt.Fprint(os.Stderr, deprecation)


Do we really need to print this? I am personally not the biggest fan of the warnings. I mean its great to get the message out but it is also very annoying.
Not a blocker for me just saying.

It's indeed very annoying but I think it may be worth giving a shot. I think that many users are still not familiar with Quadlet and that's a way to let them know about it - very loudly :)

In case users bark back, we can revert.

vrothberg · 2023-08-01T11:28:59Z

Improving docs SGTM

Also AFAIK quadlet has no pod support besides k8s yaml and I don't agree that this is a perfect replacement. K8s yaml lacks some podman specific options so migration may not be possible for all users. And then everything is run as part of one unit which totally changes the way on how to monitor and interact with these units.

That probably boils down to documenting some of the limitations.

edsantiago · 2023-08-01T11:35:42Z

One possible fix:

diff --git a/docs/source/markdown/podman-generate-systemd.1.md b/docs/source/markdown/podman-generate-systemd.1.md
index 5a6452f69..3fc491246 100644
--- a/docs/source/markdown/podman-generate-systemd.1.md
+++ b/docs/source/markdown/podman-generate-systemd.1.md
@@ -1,7 +1,7 @@
 % podman-generate-systemd 1
 
 ## NAME
-podman\-generate\-systemd - Generate systemd unit file(s) for a container or pod
+podman\-generate\-systemd - [DEPRECATED] Generate systemd unit file(s) for a container or pod
 
 ## SYNOPSIS
 **podman generate systemd** [*options*] *container|pod*
@@ -310,7 +310,7 @@ CONTAINER ID  IMAGE                            COMMAND  CREATED        STATUS
 bb310a0780ae  docker.io/library/alpine:latest  /bin/sh  3 minutes ago  Created                  busy_moser

 ## SEE ALSO
-**[podman(1)](podman.1.md)**, **[podman-container(1)](podman-container.1.md)**, **systemctl(1)**, **systemd.unit(5)**, **systemd.service(5)**, **[conmon(8)](https://github.com/containers/conmon/blob/main/docs/conmon.8.md)**, [Quadlet](podman-systemd.unit.5.md)
+**[podman(1)](podman.1.md)**, **[podman-container(1)](podman-container.1.md)**, **systemctl(1)**, **systemd.unit(5)**, **systemd.service(5)**, **[conmon(8)](https://github.com/containers/conmon/blob/main/docs/conmon.8.md)**, **[podman-systemd.unit(5)](podman-systemd.unit.5.md)**
 
 ## HISTORY
 April 2020, Updated details and added use case to use generated .service files as root and non-root, by Sujil Shah (sushah at redhat dot com)

rhatdan · 2023-08-02T12:11:46Z

@Luap99 Bottom line it is depracated, everyone who reports an issue on podman generate systemd is told to look at Quadlet.
If there are some shortcomings of running k8s under quadlet versus generate systemd, we need to know what users are hitting and work on improving them in Quadlet.

We need to move people to quadlet and the best way to do this is by telling them that generate systemd is deprecated.
In podman 4.8, I plan on hiding the options altogether, and then dropping it in podman 5.0.

Luap99 · 2023-08-02T12:43:54Z

As said I have nothing against this but users need docs, we cannot expect them to understand how to migrate on their own.
So I argue we first need to create such a doc before telling everyone that it is deprecated. There is not a single time the word pod in the quadlet man page. I would be very frustrated as a user if I have a pod unit, it is not trivial to migrate.
There are easy steps to make this simple, i.e. run podman kube generate in the existing pod and then create a kube unit based on that. But this still needs to written out somewhere.
I am not expecting you to fix all shortcomings before saying it is deprecated, I just want a simple docs where we can point people to if they have questions.

rhatdan · 2023-08-02T13:27:53Z

Do you see this as something to add to the quadlet man page?

vrothberg · 2023-08-02T13:30:24Z

Do you see this as something to add to the quadlet man page?

That makes sense, yes. The deprecation warnings should point to the quadlet man page where we could add a "migration" section or something similar.

Hiding the generate systemd command SGTM. Entirely removing is always risky; it's used internally and externally a lot. Such breaking changes are always painful - as much as I want the code and command to go away.

vrothberg

One last nit, then I think we can merge.

Let's move the migration parts into another PR. I can volunteer to open it and then we can improve.

cmd/podman/generate/systemd.go

vrothberg · 2023-08-03T08:01:45Z

cmd/podman/generate/systemd.go

@@ -119,6 +127,7 @@ func init() {
 }

 func systemd(cmd *cobra.Command, args []string) error {
+	fmt.Fprint(os.Stderr, deprecation)


It's indeed very annoying but I think it may be worth giving a shot. I think that many users are still not familiar with Quadlet and that's a way to let them know about it - very loudly :)

In case users bark back, we can revert.

Now that Quadlets are fully supported, it is time to Depracate podman generate systemd command. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

vrothberg

LGTM

openshift-ci · 2023-08-03T11:15:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [rhatdan,vrothberg]

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

rhatdan · 2023-08-04T12:30:05Z

@edsantiago @TomSweeneyRedHat @Luap99 @mheon @ashley-cui @giuseppe @baude @ygalblum PTAL

TomSweeneyRedHat

LGTM

TomSweeneyRedHat · 2023-08-04T18:03:32Z

/lgtm

openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 28, 2023

rhatdan force-pushed the systemd branch 2 times, most recently from 411c0ca to 090eb8e Compare July 30, 2023 13:07

vrothberg reviewed Jul 31, 2023

View reviewed changes

rhatdan force-pushed the systemd branch 2 times, most recently from 158d4c5 to c58141b Compare August 1, 2023 10:10

vrothberg reviewed Aug 1, 2023

View reviewed changes

Luap99 reviewed Aug 1, 2023

View reviewed changes

rhatdan force-pushed the systemd branch 4 times, most recently from eeda7bb to 82ccb1c Compare August 2, 2023 10:09

vrothberg reviewed Aug 3, 2023

View reviewed changes

rhatdan force-pushed the systemd branch 2 times, most recently from c529902 to e55ade9 Compare August 3, 2023 10:01

Deprecate podman generate systemd

44f159e

Now that Quadlets are fully supported, it is time to Depracate podman generate systemd command. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

rhatdan force-pushed the systemd branch from e55ade9 to 44f159e Compare August 3, 2023 10:59

vrothberg approved these changes Aug 3, 2023

View reviewed changes

TomSweeneyRedHat reviewed Aug 4, 2023

View reviewed changes

openshift-ci bot assigned TomSweeneyRedHat Aug 4, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2023

openshift-merge-robot merged commit 9e18e9f into containers:main Aug 4, 2023
87 checks passed

brandonp42 mentioned this pull request Oct 22, 2023

Systemd files to run fetchit should be updated to quadlet files containers/fetchit#311

Open

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 Nov 3, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate podman generate systemd #19414

Deprecate podman generate systemd #19414

rhatdan commented Jul 28, 2023

rhatdan commented Jul 28, 2023

vrothberg Jul 31, 2023

vrothberg left a comment

Luap99 left a comment

Luap99 Aug 1, 2023

vrothberg Aug 3, 2023

vrothberg commented Aug 1, 2023

edsantiago commented Aug 1, 2023

rhatdan commented Aug 2, 2023

Luap99 commented Aug 2, 2023

rhatdan commented Aug 2, 2023

vrothberg commented Aug 2, 2023

vrothberg left a comment

vrothberg Aug 3, 2023

vrothberg left a comment

openshift-ci bot commented Aug 3, 2023

rhatdan commented Aug 4, 2023

TomSweeneyRedHat left a comment

TomSweeneyRedHat commented Aug 4, 2023

Deprecate podman generate systemd #19414

Deprecate podman generate systemd #19414

Conversation

rhatdan commented Jul 28, 2023

Does this PR introduce a user-facing change?

rhatdan commented Jul 28, 2023

vrothberg Jul 31, 2023

Choose a reason for hiding this comment

vrothberg left a comment

Choose a reason for hiding this comment

Luap99 left a comment

Choose a reason for hiding this comment

Luap99 Aug 1, 2023

Choose a reason for hiding this comment

vrothberg Aug 3, 2023

Choose a reason for hiding this comment

vrothberg commented Aug 1, 2023

edsantiago commented Aug 1, 2023

rhatdan commented Aug 2, 2023

Luap99 commented Aug 2, 2023

rhatdan commented Aug 2, 2023

vrothberg commented Aug 2, 2023

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg Aug 3, 2023

Choose a reason for hiding this comment

vrothberg left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Aug 3, 2023

rhatdan commented Aug 4, 2023

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

TomSweeneyRedHat commented Aug 4, 2023