-
Notifications
You must be signed in to change notification settings - Fork 2.9k
pkg/systemd: expose [Pod] ExitPolicy key for pod create --exit-policy #26466
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
pkg/systemd: expose [Pod] ExitPolicy key for pod create --exit-policy #26466
Conversation
|
DCO is wrong. Signed-off-by is in the commit message. |
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
The reason DCO check complains is because you git username ( |
Luap99
left a comment
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.
Tests are failing because the argument order between --exit-policy=stop and --replace is swapped now. That is fine as it works in the same way but you will have to update the basic.pod test to make them pass.
cc @ygalblum
ygalblum
left a comment
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 for the code.
As @Luap99 wrote, basic.pod need to be adjusted.
| Set custom DNS search domains. Use **DNSSearch=.** to remove the search domain. | ||
|
|
||
| This key can be listed multiple times. | ||
| ### `ExitPolicy=` |
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.
Please add an extra empty line
|
Should we add a section to the man page explaining the usage of |
06db239 to
25defe7
Compare
|
Looks like I accidentally added a file while trying to fix the test. I'll squash again with the newline in docs added. |
Add ExitPolicy key to pod quadlets with logic to default to stop. Docs updated with clarifcation on default value and usage example. Simple assert added to bats to verify default constraint exists. Changed argument order in ginkgo basic pod unit test Signed-off-by: Neil Bailey <nbsp@nbailey.net>
25defe7 to
5989370
Compare
| podman.add("--exit-policy") | ||
| if found { | ||
| podman.add(exitPolicy) | ||
| } else { |
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.
no biggie but if one of these options is kind of a default, we will often set that and use a single condition as an override. maybe consider fix if you repush with someone else's comment (only if you consider one of them to be the default).
|
I'm not sure what to make of the debian and rawhide tests. |
Luap99
left a comment
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
I restart the test it is a known flake
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, nbspsemicolon 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 |
617cdc4
into
containers:main
Add ExitPolicy key to pod quadlets with logic to default to stop.
Docs updated with clarifcation on default value and usage example.
Simple assert added to bats to verify default constraint exists.
Addresses #25596
See Also: #26453
Does this PR introduce a user-facing change?