Quadlet - use the default runtime

[ygalblum]

Do not set the runtime when processing a .container file Let Podman choose the runtime based on its configuration

Does this PR introduce a user-facing change?

Yes

Quadlet: .container file may be used with any runtime

[ygalblum]

This PR comes instead of #17587

[vrothberg]

LGTM, thanks @ygalblum !

@alexlarsson @giuseppe @rhatdan PTAL

[edsantiago]

/hold

Please wait for #17305 to merge, then rebase and resubmit. Without 17305, none of this can be tested, and there's a good chance it will break CI once runc is enabled. Thank you for understanding.

[ygalblum]

@edsantiago Not sure I understand you comment. This change removes the hard-coded value and lets Podman decide which runtime to use. I would have understood if it were the other way around

[edsantiago]

The situation today: we do not test runc in CI. Therefore, for CI purposes, your change is a NOP.

#17503 adds testing with runc. These are the three possibilities:

1. your PR merges before 17503. This will require 17503 to rebase and pick up your PR, and if there are quadlet-runc failures. spend time trying to figure those out. 17503 has cost a huge amount of time to a lot of developers. There have been countless snags. It would be inhumane to impose more pain on that PR.
2. 17503 merges before your PR. You then rebase, and if there are problems, you (the SME) get to fix them. This is the ideal situation.
3. Both PRs merge semi-simultaneously (in order, obviously, but without either one rebasing). When there are problems, the next poor schmo submitting a PR will need to deal with the problems. This is the worst-case scenario.

I hope that makes sense.

[giuseppe]

LGTM

[ygalblum]

@edsantiago At the end of the day Quadlet just translates systemd unit keys to podman command line arguments. As a result, there are no quadlet-runc issues, while there may be podman-runc issues. So, if the second case occurs, I will still need to bring Podman SMEs into the discussion.
Having said that, I can wait in order to avoid the third case. But, this means we are keeping the original issue of the missing requirement in the RPM while not fixing the hard-coded value in Quadlet

[edsantiago]

Thanks @ygalblum . Let me try to phrase this another way: if for any reason the quadlet tests break under runc, those problems must be resolved in this PR. Not in 17305, and not by some innocent victim. That is all.

[vrothberg]

Let's wait for #17305 get in. If there's a risk that both PRs combined break CI, it's best to defer the merge here and rebase once #17305, is in.

I assume it won't take long until #17305 merged.

[ygalblum]

Just to make it clear, we are talking about #17305, not #17503 (which is a closed issue)

[vrothberg]

Just to make it clear, we are talking about #17305, not #17503 (which is a closed issue)

Thanks! I updated the comment.

[edsantiago]

@ygalblum #17305 has merged, please rebase and repush when convenient. Thanks again for your understanding.

[vrothberg]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, vrothberg, ygalblum

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 [giuseppe,vrothberg,ygalblum]

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

[vrothberg]

Thanks, @ygalblum !

[vrothberg]

/hold cancel

[dcermak]

Can this please be cherry picked to the 4.4 branch?

[rhatdan]

/cherry-pick v4.4

[openshift-cherrypick-robot]

@rhatdan: #17601 failed to apply on top of branch "v4.4":

Applying: Quadlet - use the default runtime
Using index info to reconstruct a base tree...
M	pkg/systemd/quadlet/quadlet.go
M	test/e2e/quadlet/basepodman.container
M	test/e2e/quadlet/basic.container
Falling back to patching base and 3-way merge...
Auto-merging test/e2e/quadlet/basic.container
CONFLICT (content): Merge conflict in test/e2e/quadlet/basic.container
Auto-merging test/e2e/quadlet/basepodman.container
CONFLICT (content): Merge conflict in test/e2e/quadlet/basepodman.container
Auto-merging pkg/systemd/quadlet/quadlet.go
CONFLICT (content): Merge conflict in pkg/systemd/quadlet/quadlet.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Quadlet - use the default runtime
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick v4.4

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.

[vrothberg]

Can this please be cherry picked to the 4.4 branch?

@dcermak would you backport?

[dcermak]

Can this please be cherry picked to the 4.4 branch?

@dcermak would you backport?

Sure, here you go: #17641