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: shimv1 leak issue #9344

Merged
merged 1 commit into from Nov 8, 2023
Merged

Conversation

fuweid
Copy link
Member

@fuweid fuweid commented Nov 7, 2023

// Delete the initial process and container
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
        p, err := s.getInitProcess()
        if err != nil {
                return nil, err
        }
        if err := p.Delete(ctx); err != nil {
                return nil, errdefs.ToGRPC(err)
        }

	// The client might canceled the request but the shim service still
	// moved on. The `delete(s.processes, s.id)` was executed
	// successfully. So the next Delete call will return `container
	// must be created` error. The client side should ignore this
	// issue.

        s.mu.Lock()
        delete(s.processes, s.id)
        s.mu.Unlock()
        s.platform.Close()
        return &shimapi.DeleteResponse{
                ExitStatus: uint32(p.ExitStatus()),
                ExitedAt:   protobuf.ToTimestamp(p.ExitedAt()),
                Pid:        uint32(p.Pid()),
        }, nil
}

introduced by #9003
fixes: #9309

```go
// Delete the initial process and container
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
        p, err := s.getInitProcess()
        if err != nil {
                return nil, err
        }
        if err := p.Delete(ctx); err != nil {
                return nil, errdefs.ToGRPC(err)
        }

	// The client might canceled the request but the shim service still
	// moved on. The `delete(s.processes, s.id)` was executed
	// successfully. So the next Delete call will return `container
	// must be created` error. The client side should ignore this
	// issue.

        s.mu.Lock()
        delete(s.processes, s.id)
        s.mu.Unlock()
        s.platform.Close()
        return &shimapi.DeleteResponse{
                ExitStatus: uint32(p.ExitStatus()),
                ExitedAt:   protobuf.ToTimestamp(p.ExitedAt()),
                Pid:        uint32(p.Pid()),
        }, nil
}
```

introduced by containerd#9003
fixes: containerd#9309

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@fuweid
Copy link
Member Author

fuweid commented Nov 7, 2023

ping @mikebrow @kiashok

It can address the issue you mention in #9326 (comment)

@fuweid fuweid marked this pull request as ready for review November 7, 2023 14:27
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks!

@fuweid fuweid merged commit 124c95e into containerd:release/1.7 Nov 8, 2023
49 checks passed
@fuweid fuweid deleted the fix-9309 branch November 8, 2023 02:27
Mengkzhaoyun pushed a commit to open-beagle/containerd that referenced this pull request Dec 20, 2023
containerd 1.7.9

Welcome to the v1.7.9 release of containerd!

The ninth patch release for containerd 1.7 contains various fixes and updates.

* **update runc binary to v1.1.10::** ([#9359](containerd/containerd#9359))
* **vendor: upgrade OpenTelemetry to v1.19.0 / v0.45.0** ([#9301](containerd/containerd#9301))
* **Expose usage of cri-api v1alpha2** ([#9336](containerd/containerd#9336))
* **integration: deflake TestIssue9103** ([#9354](containerd/containerd#9354))
* **fix: shimv1 leak issue** ([#9344](containerd/containerd#9344))
* **cri: add deprecation warnings for mirrors, auths, and configs** ([#9327](containerd/containerd#9327))
* **Update hcsshim tag to v0.11.4** ([#9326](containerd/containerd#9326))
* **Expose usage of deprecated features** ([#9315](containerd/containerd#9315))

See the changelog for complete list of changes

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

* Samuel Karp
* Kazuyoshi Kato
* Wei Fu
* Kirtana Ashok
* Derek McGowan
* Milas Bowman
* Sebastiaan van Stijn
* ruiwen-zhao
<details><summary>28 commits</summary>
<p>

* [release/1.7] Add release notes for v1.7.9 ([#9333](containerd/containerd#9333))
  * [`4b912af52`](containerd/containerd@4b912af) Add release notes for v1.7.9
* [release/1.7 backport] update runc binary to v1.1.10 ([#9359](containerd/containerd#9359))
  * [`eff291713`](containerd/containerd@eff2917) update runc binary to v1.1.10
* [release/1.7] vendor: upgrade OpenTelemetry to v1.19.0 / v0.45.0 ([#9301](containerd/containerd#9301))
  * [`bd9428ff7`](containerd/containerd@bd9428f) vendor: upgrade OpenTelemetry to v1.19.0 / v0.45.0
* [release/1.7] Expose usage of cri-api v1alpha2 ([#9336](containerd/containerd#9336))
  * [`d62cba40c`](containerd/containerd@d62cba4) Expose usage of cri-api v1alpha2
* [release/1.7] integration: deflake TestIssue9103 ([#9354](containerd/containerd#9354))
  * [`5dbc258a8`](containerd/containerd@5dbc258) integration: deflake TestIssue9103
* [release/1.7] fix: shimv1 leak issue ([#9344](containerd/containerd#9344))
  * [`449912857`](containerd/containerd@4499128) fix: shimv1 leak issue
* [release/1.7] cri: add deprecation warnings for mirrors, auths, and configs ([#9327](containerd/containerd#9327))
  * [`152c57e91`](containerd/containerd@152c57e) cri: add deprecation warning for configs
  * [`689a1036d`](containerd/containerd@689a103) cri: add deprecation warning for auths
  * [`8c38975bf`](containerd/containerd@8c38975) cri: add deprecation warning for mirrors
  * [`1fbce40c4`](containerd/containerd@1fbce40) cri: add ability to emit deprecation warnings
* [release/1.7] Update hcsshim tag to v0.11.4 ([#9326](containerd/containerd#9326))
  * [`73f15bdb6`](containerd/containerd@73f15bd) Update hcsshim tag to v0.11.4
* [release/1.7] Expose usage of deprecated features ([#9315](containerd/containerd#9315))
  * [`60d48ffea`](containerd/containerd@60d48ff) ctr: new deprecations command
  * [`74a06671a`](containerd/containerd@74a0667) plugin: record deprecation for dynamic plugins
  * [`fa5f3c91a`](containerd/containerd@fa5f3c9) server: add ability to record config deprecations
  * [`f7880e7f0`](containerd/containerd@f7880e7) pull: record deprecation warning for schema 1
  * [`1dd2f2c02`](containerd/containerd@1dd2f2c) introspection: add support for deprecations
  * [`aaf000c18`](containerd/containerd@aaf000c) api/introspection: deprecation warnings in server
  * [`9b7ceee54`](containerd/containerd@9b7ceee) warning: new service for deprecations
  * [`b708f8bfa`](containerd/containerd@b708f8b) deprecation: new package for deprecations
</p>
</details>

* **github.com/Microsoft/hcsshim**                                                 v0.11.1 -> v0.11.4
* **github.com/cenkalti/backoff/v4**                                               v4.2.0 -> v4.2.1
* **github.com/go-logr/logr**                                                      v1.2.3 -> v1.2.4
* **github.com/grpc-ecosystem/grpc-gateway/v2**                                    v2.7.0 -> v2.16.0
* **go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc**  v0.40.0 -> v0.45.0
* **go.opentelemetry.io/otel**                                                     v1.14.0 -> v1.19.0
* **go.opentelemetry.io/otel/exporters/otlp/otlptrace**                            v1.14.0 -> v1.19.0
* **go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc**              v1.14.0 -> v1.19.0
* **go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp**              v1.14.0 -> v1.19.0
* **go.opentelemetry.io/otel/metric**                                              v0.37.0 -> v1.19.0
* **go.opentelemetry.io/otel/sdk**                                                 v1.14.0 -> v1.19.0
* **go.opentelemetry.io/otel/trace**                                               v1.14.0 -> v1.19.0
* **go.opentelemetry.io/proto/otlp**                                               v0.19.0 -> v1.0.0

Previous release can be found at [v1.7.8](https://github.com/containerd/containerd/releases/tag/v1.7.8)
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

4 participants