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.

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

[release/1.7] Fix runc shim to only defer init process exits #10037

Merged
merged 1 commit into from Apr 5, 2024

Conversation

laurazard
Copy link
Member

backports #9999

(cherry picked from commit 6d00c3a)

In order to make sure that we don't publish task exit events for init
processes before we do for execs in that container, we added logic to
`processExits` in 892dc54 to skip these
and let the pending exec's `handleStarted` closure process them.

However, the conditional logic in `processExits` added was faulty - we
should only defer processing of exit events related to init processes,
not other execs. Due to this missing condition,
892dc54 introduced a bug where, if
there are many concurrent execs for the same container/init pid, exec
exits are skipped and then never published, resulting in hanging
clients.

This commit adds the missing logic to `processExits`.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit 6d00c3a)
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
@laurazard laurazard requested a review from a team April 5, 2024 13:38
@laurazard laurazard self-assigned this Apr 5, 2024
Copy link
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

@dmcgowan dmcgowan added the area/runtime Runtime label Apr 5, 2024
@dmcgowan dmcgowan changed the title [release/1.7] runc-shim: only defer init process exits [release/1.7] Fix runc shim to only defer init process exits Apr 5, 2024
@dmcgowan dmcgowan merged commit 52fc8ab into containerd:release/1.7 Apr 5, 2024
56 checks passed
Mengkzhaoyun pushed a commit to open-beagle/containerd that referenced this pull request Apr 26, 2024
containerd 1.7.15

Welcome to the v1.7.15 release of containerd!

The fifteenth patch release for containerd 1.7 contains various fixes; one for a
regression introduced in v1.7.14 in the way process exits were handled.

* Adds mediatype to OCI index record on export ([#9990](containerd/containerd#9990))

* Fix runc shim to only defer init process exits ([#10037](containerd/containerd#10037))

Please try out the release binaries and report any issues at
https://github.com/containerd/containerd/issues.

* Derek McGowan
* Phil Estes
* Austin Vazquez
* Laura Brehm
* Sebastiaan van Stijn
* Talon
<details><summary>12 commits</summary>
<p>

* Prepare for v1.7.15 release ([#10039](containerd/containerd#10039))
  * [`4d4759b54`](containerd/containerd@4d4759b) Prep v1.7.15 release
* Fix runc shim to only defer init process exits ([#10037](containerd/containerd#10037))
  * [`21df46766`](containerd/containerd@21df467) runc-shim: only defer init process exits
* Fix compile from version control system (source) use case ([#10012](containerd/containerd#10012))
  * [`2a054213e`](containerd/containerd@2a05421) Fix compile from version control system (source) use case
* Adds mediatype to OCI index record on export ([#9990](containerd/containerd#9990))
  * [`6605c47a4`](containerd/containerd@6605c47) adds mediatype to oci index record
* vendor: google.golang.org/protobuf 1.33.0, github.com/golang/protobuf v1.5.4 ([#9975](containerd/containerd#9975))
  * [`e6d91d843`](containerd/containerd@e6d91d8) vendor: github.com/golang/protobuf v1.5.4
  * [`2d136c5f5`](containerd/containerd@2d136c5) build(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0
  * [`a1a7af7a3`](containerd/containerd@a1a7af7) build(deps): bump google.golang.org/protobuf from 1.31.0 to 1.32.0
</p>
</details>

* **github.com/golang/protobuf**  v1.5.3 -> v1.5.4
* **google.golang.org/protobuf**  v1.31.0 -> v1.33.0

Previous release can be found at [v1.7.14](https://github.com/containerd/containerd/releases/tag/v1.7.14)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants