8000 Fix stopped containers with restart-policy showing as "restarting" by thaJeztah · Pull Request #38737 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix stopped containers with restart-policy showing as "restarting" #38737

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
Merged
merged 1 commit into from
Feb 28, 2019
Merged

Fix stopped containers with restart-policy showing as "restarting" #38737

merged 1 commit into from
Feb 28, 2019

Conversation

thaJeztah
Copy link
Member
@thaJeztah thaJeztah commented Feb 14, 2019 •
edited
Loading

fixes #38720 Flaky test: DockerDaemonSuite.TestDaemonRestartUnlessStopped

When manually stopping a container with a restart-policy, the container
would show as "restarting" in docker ps whereas its actual state
is "exited".

Stopping a container with a restart policy shows the container as "restarting"

docker run -d --name test --restart unless-stopped busybox false

docker stop test

docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS                       PORTS               NAMES
7e07409fa1d3        busybox             "false"             5 minutes ago       Restarting (1) 4 minutes ago                     test

However, inspecting the same container shows that it's exited:

docker inspect test --format '{{ json .State }}'
{
  "Status": "exited",
  "Running": false,
  "Paused": false,
  "Restarting": false,
  "OOMKilled": false,
  "Dead": false,
  "Pid": 0,
  "ExitCode": 1,
  "Error": "",
  "StartedAt": "2019-02-14T13:26:27.6091648Z",
  "FinishedAt": "2019-02-14T13:26:27.689427Z"
}

And killing the container confirms this;

docker kill test
Error response from daemon: Cannot kill container: test: Container 7e07409fa1d36dc8d8cb8f25cf12ee1168ad9040183b85fafa73ee2c1fcf9361 is not running

docker run -d --name test --restart unless-stopped busybox false

docker stop test

docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS                PORTS               NAMES
d0595237054a        busybox             "false"             5 minutes ago       Restarting (1)       4 minutes ago                       exit

Signed-off-by: Sebastiaan van Stijn github@gone.nl

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Copy link
Member Author

ping @x1022as @cpuguy83 @tonistiigi ptal

@tonistiigi
Copy link
Member

Could also be in

moby/daemon/monitor.go

Line 103 in 818d2dd

c.SetStopped(&exitStatus)

@codecov
Copy link
codecov bot commented Feb 14, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@91d934b). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #38737   +/-   ##
=========================================
  Coverage          ?   36.42%           
=========================================
  Files             ?      613           
  Lines             ?    45855           
  Branches          ?        0           
=========================================
  Hits              ?    16701           
  Misses            ?    26859           
  Partials          ?     2295

@thaJeztah thaJeztah force-pushed the fix_stopped_restart_containers branch from 63cd896 to ac35e50 Compare February 14, 2019 22:13
@thaJeztah
Copy link
Member Author

@tonistiigi thanks; that looks definitely better; updated

@cpuguy83
Copy link
Member

I'm pretty sure I've seen a PR similar to this.
But this seems like it will race with the call to checkpoint below.

@thaJeztah
Copy link
Member Author

hm, was actually looking at that yes 🤔

@thaJeztah thaJeztah mentioned this pull request Feb 15, 2019
Merged
@kolyshkin
Copy link
Contributor

SGTM

@thaJeztah thaJeztah force-pushed the fix_stopped_restart_containers branch from ac35e50 to 19deacb Compare February 27, 2019 23:14
@thaJeztah
Copy link
Member Author

ping @cpuguy83 updated to move the first checkpoint to above the 'restart' part; PTAL

cpuguy83
cpuguy83 approved these changes Feb 27, 2019< 8000 /relative-time>
View reviewed changes
Copy link
Member
@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah force-pushed the fix_stopped_restart_containers branch from 19deacb to 0c7f69e Compare February 27, 2019 23:17
@thaJeztah
Fix stopped containers with restart-policy showing as "restarting"
8c0ecb6 
When manually stopping a container with a restart-policy, the container
would show as "restarting" in `docker ps` whereas its actual state
is "exited".

Stopping a container with a restart policy shows the container as "restarting"

    docker run -d --name test --restart unless-stopped busybox false

    docker stop test

    docker ps
    CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS                       PORTS               NAMES
    7e07409fa1d3        busybox             "false"             5 minutes ago       Restarting (1) 4 minutes ago                     test

However, inspecting the same container shows that it's exited:

    docker inspect test --format '{{ json .State }}'
    {
      "Status": "exited",
      "Running": false,
      "Paused": false,
      "Restarting": false,
      "OOMKilled": false,
      "Dead": false,
      "Pid": 0,
      "ExitCode": 1,
      "Error": "",
      "StartedAt": "2019-02-14T13:26:27.6091648Z",
      "FinishedAt": "2019-02-14T13:26:27.689427Z"
    }

And killing the container confirms this;

    docker kill test
    Error response from daemon: Cannot kill container: test: Container 7e07409fa1d36dc8d8cb8f25cf12ee1168ad9040183b85fafa73ee2c1fcf9361 is not running

    docker run -d --name test --restart unless-stopped busybox false

    docker stop test

    docker ps
    CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS                PORTS               NAMES
    d0595237054a        busybox             "false"             5 minutes ago       Restarting (1)       4 minutes ago                       exit

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix_stopped_restart_containers branch from 0c7f69e to 8c0ecb6 Compare February 27, 2019 23:18
@thaJeztah
Copy link
Member Author

oh! you were fast; Moved the state counter as well

cpuguy83
cpuguy83 approved these changes Feb 27, 2019
View reviewed changes
Copy link
Member
@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 348d793 into moby:master Feb 28, 2019
@thaJeztah thaJeztah deleted the fix_stopped_restart_containers branch February 28, 2019 01:41
@kolyshkin kolyshkin mentioned this pull request Feb 28, 2019
Merged
@thaJeztah thaJeztah mentioned this pull request Mar 2, 2019
Merged
@thaJeztah thaJeztah mentioned this pull request Mar 26, 2019
Merged
@thaJeztah thaJeztah mentioned this pull request Aug 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Assignees
No one assigned
Labels
status/2-code-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flaky test: DockerDaemonSuite.TestDaemonRestartUnlessStopped
5 participants
@thaJeztah @tonistiigi @cpuguy83 @kolyshkin @GordonTheTurtle
0