Skip to content

[release/1.4 backport] Fix missing Body.Close() calls on push to docker remote#5717

Merged
estesp merged 1 commit into
containerd:release/1.4from
thaJeztah:1.4_backport_docker_push_close_body
Jul 12, 2021
Merged

[release/1.4 backport] Fix missing Body.Close() calls on push to docker remote#5717
estesp merged 1 commit into
containerd:release/1.4from
thaJeztah:1.4_backport_docker_push_close_body

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Jul 12, 2021

Copy link
Copy Markdown
Member

note: This is a slightly modified version of the commit in main, because
pusher.Commit() uses a regular http.Response, not a wrapped one.

backport of #5712

Discovered this while using HTTP tracing via OpenTelemetry inside of
buildkitd, where the trace spans were not being reported for the
registry PUT http requests. The spans are only reported on the Close
for the Body, after adding these Close calls, the spans are reported as
expected.

Signed-off-by: coryb [email protected]
(cherry picked from commit 894b6ae)
Signed-off-by: Sebastiaan van Stijn [email protected]

@theopenlab-ci

theopenlab-ci Bot commented Jul 12, 2021

Copy link
Copy Markdown

Build succeeded.

@thaJeztah

Copy link
Copy Markdown
Member Author

Ah, hmmm.. perhaps not needed in 1.4.x, or in a slightly different fashion;

level=warning msg="[runner] Can't run linter goanalysis_metalinter: vrp: failed prerequisites: [email protected]/containerd/containerd/runtime/restart"
level=warning msg="[runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/remotes/docker/pusher.go:342:13: resp.Response undefined (type *http.Response has no field or method Response) -: could not load export data: no export data for \"github.com/containerd/containerd/remotes/docker\"]"
level=error msg="Running error: buildssa: analysis skipped: errors in package: [/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/remotes/docker/pusher.go:342:13: resp.Response undefined (type *http.Response has no field or method Response) -: could not load export data: no export data for \"github.com/containerd/containerd/remotes/docker\"]"
Makefile:129: recipe for target 'check' failed
make: *** [check] Error 3
Error: Process completed with exit code 2.

> note: This is a slightly modified version of the commit in main, because
> pusher.Commit() uses a regular http.Response, not a wrapped one.

Discovered this while using HTTP tracing via OpenTelemetry inside of
buildkitd, where the trace spans were not being reported for the
registry PUT http requests.  The spans are only reported on the Close
for the Body, after adding these Close calls, the spans are reported as
expected.

Signed-off-by: coryb <[email protected]>
(cherry picked from commit 894b6ae)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
thaJeztah force-pushed the 1.4_backport_docker_push_close_body branch from f219b40 to 591744a Compare July 12, 2021 19:09
@theopenlab-ci

theopenlab-ci Bot commented Jul 12, 2021

Copy link
Copy Markdown

Build succeeded.

@thaJeztah
thaJeztah marked this pull request as draft July 12, 2021 19:09
Comment thread remotes/docker/pusher.go
@dmcgowan dmcgowan modified the milestones: 1.5.3, 1.4.7 Jul 12, 2021
@thaJeztah
thaJeztah marked this pull request as ready for review July 12, 2021 20:29
@thaJeztah

Copy link
Copy Markdown
Member Author

could use some extra eyes on this one to verify this is all correct for the 1.4 implementation (as some things changed; see above), and this code is not trivial

@mikebrow mikebrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@estesp estesp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@estesp
estesp merged commit e6d4da0 into containerd:release/1.4 Jul 12, 2021
@thaJeztah
thaJeztah deleted the 1.4_backport_docker_push_close_body branch July 12, 2021 22:09
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