Skip to content

[release/1.3] cherry-pick: runtime v2: Close platform in runc shim's Shutdown method.#3907

Merged
crosbymichael merged 1 commit intocontainerd:release/1.3from
estesp:cp-platform-close-fix
Dec 23, 2019
Merged

[release/1.3] cherry-pick: runtime v2: Close platform in runc shim's Shutdown method.#3907
crosbymichael merged 1 commit intocontainerd:release/1.3from
estesp:cp-platform-close-fix

Conversation

@estesp
Copy link
Copy Markdown
Member

@estesp estesp commented Dec 20, 2019

Previously, the platform was closed as part of the Delete method when the
process was an init for a task and there were no more tasks after its deletion.
This can create problems if another task is created within the shim right after
the delete runs, which results in the platform being closed but the shim
continuing to run.

This change moves closing the platform to the Shutdown method after the shim's
context is canceled, which ensures the platform is only closed once the shim
is sure its done servicing containers.

Signed-off-by: Erik Sipsma [email protected]
Signed-off-by: Phil Estes [email protected]

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 20, 2019

Build succeeded.

@estesp
Copy link
Copy Markdown
Member Author

estesp commented Dec 20, 2019

Needs #3908 to pass CI

Previously, the platform was closed as part of the Delete method when the
process was an init for a task and there were no more tasks after its deletion.
This can create problems if another task is created within the shim right after
the delete runs, which results in the platform being closed but the shim
continuing to run.

This change moves closing the platform to the Shutdown method after the shim's
context is canceled, which ensures the platform is only closed once the shim
is sure its done servicing containers.

Signed-off-by: Erik Sipsma <[email protected]>
Signed-off-by: Phil Estes <[email protected]>
@estesp estesp force-pushed the cp-platform-close-fix branch from c038238 to e7a6dda Compare December 21, 2019 02:15
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 21, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 21, 2019

Codecov Report

Merging #3907 into release/1.3 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release/1.3    #3907   +/-   ##
============================================
  Coverage        42.07%   42.07%           
============================================
  Files              129      129           
  Lines            14322    14322           
============================================
  Hits              6026     6026           
  Misses            7395     7395           
  Partials           901      901
Flag Coverage Δ
#linux 45.59% <ø> (ø) ⬆️
#windows 37.01% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbc39d6...e7a6dda. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

for discoverability:

backport of #3896
fixes #3895 for the 1.3 branch

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael crosbymichael merged commit bc43dc0 into containerd:release/1.3 Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants