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.

Sign up for GitHub

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

Jump to bottom

Exit code change BZ #2052697 #13450

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Exit code change BZ #2052697 #13450

merged 2 commits into from
Mar 16, 2022

Conversation

jwhonce
Copy link
Member

@jwhonce jwhonce commented Mar 7, 2022 •
edited
Loading

  • On SIGTERM, podman service will return exit code 0. SIGKILL will return exit code 1. This better matches expected behavior from systemctl stop podman.service
  • Add e2e test to cover case above plus ensuring containers are still running after podman.service is stopped via systemctl stop podman.service

See https://bugzilla.redhat.com/show_bug.cgi?id=2052697

Signed-off-by: Jhon Honce [email protected]

@jwhonce jwhonce added kind/bug Categorizes issue or PR as related to a bug. HTTP API Bug is in RESTful API labels Mar 7, 2022
@jwhonce jwhonce requested a review from edsantiago March 7, 2022 23:18
@jwhonce jwhonce self-assigned this Mar 7, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwhonce

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

@jwhonce jwhonce marked this pull request as draft March 7, 2022 23:18
@openshift-ci openshift-ci bot added 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. labels Mar 7, 2022
edsantiago
edsantiago suggested changes Mar 8, 2022
View reviewed changes
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Note that tests are hanging. I haven't tried reproducing the hang, will leave that up to you, but I have some suggestions inline.

test/apiv2/test-apiv2 Outdated Show resolved Hide resolved
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
@jwhonce jwhonce force-pushed the bz/2052697 branch 2 times, most recently from ce86f6d to 51d7025 Compare March 8, 2022 16:29
edsantiago
edsantiago reviewed Mar 8, 2022
View reviewed changes
test/apiv2/80-systemd.at Outdated Show resolved Hide resolved
@jwhonce jwhonce force-pushed the bz/2052697 branch 9 times, most recently from a6a45a4 to 8149ec1 Compare March 10, 2022 22:28
@jwhonce jwhonce requested a review from baude March 10, 2022 22:29
@jwhonce jwhonce force-pushed the bz/2052697 branch 2 times, most recently from fcc110d to 6144717 Compare March 11, 2022 18:08
@jwhonce jwhonce marked this pull request as ready for review March 15, 2022 15: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 Mar 15, 2022
@jwhonce jwhonce changed the title Integration test for BZ #2052697 Exit code change BZ #2052697 Mar 15, 2022
@baude
Copy link
Member

baude commented Mar 15, 2022

code lgtm to me ... i want @mheon to take a peek as well.

@mheon
Copy link
Member

mheon commented Mar 15, 2022

LGTM

@jwhonce
Exit with 0 when receiving SIGTERM
ca7376b 
* systemctl stop podman.service will now return exit code 0
* Update test framework to support JSON boolean and numeric values

Signed-off-by: Jhon Honce <[email protected]>
@jwhonce jwhonce force-pushed the bz/2052697 branch 2 times, most recently from 5e1b54a to 0bb21d4 Compare March 16, 2022 17:00
@jwhonce
Add test for BZ #2052697
1387b5b 
Signed-off-by: Jhon Honce <[email protected]>
@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2022
@openshift-merge-robot openshift-merge-robot merged commit b1d37a7 into containers:main Mar 16, 2022
@mheon mheon mentioned this pull request Apr 28, 2022
Merged
@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 Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mheon mheon mheon left review comments

@edsantiago edsantiago edsantiago requested changes

@baude baude Awaiting requested review from baude

Assignees

@jwhonce jwhonce

@TomSweeneyRedHat TomSweeneyRedHat

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. HTTP API Bug is in RESTful API kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jwhonce @baude @mheon @TomSweeneyRedHat @edsantiago @openshift-merge-robot