[release/1.6 backport] pushWriter: correctly propagate errors #7990
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the refactor from 926b9c7, the error handling was substantially reworked, and changed the types of errors returned. Notably, in the case of a network error, instead of propogating the error through to return from pushWriter.Write (as previously), it would be propagated through to pushWriter.Commit - however, this is too late, since we've already closed the io.Pipe by the time we would have reached this function. Therefore, we get the generic error message "io: read/write on closed pipe" for *every network error*. This patch corrects this behavior to ensure that the correct error object is always returned as early as possible, by checking the error result after writing and detecting a closed pipe. Additionally, we do some additional hardening - specifically we prevent falling through when resetting the content or detecting errors, and update the tests to explicitly check for the ErrReset message. Signed-off-by: Justin Chadwell <[email protected]> (cherry picked from commit 9f6058d) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Member
Author
|
@jedevc @tonistiigi @dmcgowan @estesp ptal 🤗 |
jedevc
approved these changes
Jan 24, 2023
dmcgowan
approved these changes
Jan 24, 2023
Member
Author
|
thx! |
aravindhp
added a commit
to openshift/containerd
that referenced
this pull request
Feb 9, 2023
containerd 1.6.16 Welcome to the v1.6.16 release of containerd! The sixteenth patch release for containerd 1.6 includes various bug fixes and updates. * **Fix push error propagation** ([containerd#7990](containerd#7990)) * **Fix slice append error with HugepageLimits for Linux** ([containerd#7995](containerd#7995)) * **Update default seccomp profile for PKU and CAP_SYS_NICE** ([containerd#8001](containerd#8001)) * **Fix overlayfs error when upperdirlabel option is set** ([containerd#8002](containerd#8002)) 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. * Akihiro Suda * Derek McGowan * Samuel Karp * Sebastiaan van Stijn * Phil Estes * Craig Ingram * Justin Chadwell * Qasim Sarfraz * Wei Fu * bin liu * cardy.tang * rongfu.leng <details><summary>30 commits</summary> <p> * [release/1.6] Prepare v1.6.16 ([containerd#8016](containerd#8016)) * [`d3c595aa3`](containerd@d3c595a) Prepare release notes for v1.6.16 * [release/1.6 backport] Fix tx closed error when upperdirlabel specified ([containerd#8002](containerd#8002)) * [`8c704036a`](containerd@8c70403) Fix tx closed error when upperdirlabel specified * [release/1.6 backport] assorted test-fixes ([containerd#8000](containerd#8000)) * [`91a68edd7`](containerd@91a68ed) cri: Fix TestUpdateOCILinuxResource for host w/o swap controller * [`5594f706e`](containerd@5594f70) Fix TestUpdateContainerResources_Memory* on cgroup v2 hosts * [release/1.6 backport] seccomp updates ([containerd#8001](containerd#8001)) * [`7037f5313`](containerd@7037f53) seccomp: add get_mempolicy, mbind, set_mempolicy, with CAP_SYS_NICE * [`d22919a1c`](containerd@d22919a) seccomp: seccomp: add syscalls related to PKU in default policy * [release/1.6 backport] Harden GITHUB_TOKEN permissions ([containerd#7999](containerd#7999)) * [`8b8a21fe4`](containerd@8b8a21f) Harden GITHUB_TOKEN permissions * [release/1.6 backport] assorted updates to Vagrantfile ([containerd#7996](containerd#7996)) * [`8009948bb`](containerd@8009948) Vagrantfile: fix comments about SELinux * [`550424f92`](containerd@550424f) Vagrantfile: install-rootless-podman: remove `setenforce 0` * [`2c32f8559`](containerd@2c32f85) CI: update Fedora to 37 * [`556bb0cc8`](containerd@556bb0c) Vagrantfile: explicitly specify rsync as the shared folder driver * [`edfac1834`](containerd@edfac18) fix install cni script * [`91d5e53fb`](containerd@91d5e53) Vagrantfile: dump containerd log after critest * [release/1.6 backport] Fix slice append error ([containerd#7995](containerd#7995)) * [`ab193eb20`](containerd@ab193eb) pkg/cri: optimize slice initialization * [`e6cf5ec58`](containerd@e6cf5ec) Fix slice append error * [release/1.6] update to go1.18.10 ([containerd#7992](containerd#7992)) * [`6a8a6531f`](containerd@6a8a653) [release/1.6] update to go1.18.10 * [release/1.6 backport] release/Dockerfile: set DEBIAN_FRONTEND=noninteractive ([containerd#7991](containerd#7991)) * [`d0dc7988a`](containerd@d0dc798) release/Dockerfile: set DEBIAN_FRONTEND=noninteractive * [release/1.6 backport] pushWriter: correctly propagate errors ([containerd#7990](containerd#7990)) * [`1584c2581`](containerd@1584c25) pushWriter: correctly propagate errors * [release/1.6] mod: update github.com/pelletier/[email protected] ([containerd#7942](containerd#7942)) * [`545f22091`](containerd@545f220) mod: update github.com/pelletier/[email protected] </p> </details> * **github.com/pelletier/go-toml** v1.9.3 -> v1.9.5 Previous release can be found at [v1.6.15](https://github.com/containerd/containerd/releases/tag/v1.6.15)
aravindhp
added a commit
to openshift/containerd
that referenced
this pull request
Feb 9, 2023
containerd 1.6.16 Welcome to the v1.6.16 release of containerd! The sixteenth patch release for containerd 1.6 includes various bug fixes and updates. * **Fix push error propagation** ([containerd#7990](containerd#7990)) * **Fix slice append error with HugepageLimits for Linux** ([containerd#7995](containerd#7995)) * **Update default seccomp profile for PKU and CAP_SYS_NICE** ([containerd#8001](containerd#8001)) * **Fix overlayfs error when upperdirlabel option is set** ([containerd#8002](containerd#8002)) 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. * Akihiro Suda * Derek McGowan * Samuel Karp * Sebastiaan van Stijn * Phil Estes * Craig Ingram * Justin Chadwell * Qasim Sarfraz * Wei Fu * bin liu * cardy.tang * rongfu.leng <details><summary>30 commits</summary> <p> * [release/1.6] Prepare v1.6.16 ([containerd#8016](containerd#8016)) * [`d3c595aa3`](containerd@d3c595a) Prepare release notes for v1.6.16 * [release/1.6 backport] Fix tx closed error when upperdirlabel specified ([containerd#8002](containerd#8002)) * [`8c704036a`](containerd@8c70403) Fix tx closed error when upperdirlabel specified * [release/1.6 backport] assorted test-fixes ([containerd#8000](containerd#8000)) * [`91a68edd7`](containerd@91a68ed) cri: Fix TestUpdateOCILinuxResource for host w/o swap controller * [`5594f706e`](containerd@5594f70) Fix TestUpdateContainerResources_Memory* on cgroup v2 hosts * [release/1.6 backport] seccomp updates ([containerd#8001](containerd#8001)) * [`7037f5313`](containerd@7037f53) seccomp: add get_mempolicy, mbind, set_mempolicy, with CAP_SYS_NICE * [`d22919a1c`](containerd@d22919a) seccomp: seccomp: add syscalls related to PKU in default policy * [release/1.6 backport] Harden GITHUB_TOKEN permissions ([containerd#7999](containerd#7999)) * [`8b8a21fe4`](containerd@8b8a21f) Harden GITHUB_TOKEN permissions * [release/1.6 backport] assorted updates to Vagrantfile ([containerd#7996](containerd#7996)) * [`8009948bb`](containerd@8009948) Vagrantfile: fix comments about SELinux * [`550424f92`](containerd@550424f) Vagrantfile: install-rootless-podman: remove `setenforce 0` * [`2c32f8559`](containerd@2c32f85) CI: update Fedora to 37 * [`556bb0cc8`](containerd@556bb0c) Vagrantfile: explicitly specify rsync as the shared folder driver * [`edfac1834`](containerd@edfac18) fix install cni script * [`91d5e53fb`](containerd@91d5e53) Vagrantfile: dump containerd log after critest * [release/1.6 backport] Fix slice append error ([containerd#7995](containerd#7995)) * [`ab193eb20`](containerd@ab193eb) pkg/cri: optimize slice initialization * [`e6cf5ec58`](containerd@e6cf5ec) Fix slice append error * [release/1.6] update to go1.18.10 ([containerd#7992](containerd#7992)) * [`6a8a6531f`](containerd@6a8a653) [release/1.6] update to go1.18.10 * [release/1.6 backport] release/Dockerfile: set DEBIAN_FRONTEND=noninteractive ([containerd#7991](containerd#7991)) * [`d0dc7988a`](containerd@d0dc798) release/Dockerfile: set DEBIAN_FRONTEND=noninteractive * [release/1.6 backport] pushWriter: correctly propagate errors ([containerd#7990](containerd#7990)) * [`1584c2581`](containerd@1584c25) pushWriter: correctly propagate errors * [release/1.6] mod: update github.com/pelletier/[email protected] ([containerd#7942](containerd#7942)) * [`545f22091`](containerd@545f220) mod: update github.com/pelletier/[email protected] </p> </details> * **github.com/pelletier/go-toml** v1.9.3 -> v1.9.5 Previous release can be found at [v1.6.15](https://github.com/containerd/containerd/releases/tag/v1.6.15)
aravindhp
added a commit
to openshift/containerd
that referenced
this pull request
Feb 9, 2023
containerd 1.6.16 Welcome to the v1.6.16 release of containerd! The sixteenth patch release for containerd 1.6 includes various bug fixes and updates. * **Fix push error propagation** ([containerd#7990](containerd#7990)) * **Fix slice append error with HugepageLimits for Linux** ([containerd#7995](containerd#7995)) * **Update default seccomp profile for PKU and CAP_SYS_NICE** ([containerd#8001](containerd#8001)) * **Fix overlayfs error when upperdirlabel option is set** ([containerd#8002](containerd#8002)) 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. * Akihiro Suda * Derek McGowan * Samuel Karp * Sebastiaan van Stijn * Phil Estes * Craig Ingram * Justin Chadwell * Qasim Sarfraz * Wei Fu * bin liu * cardy.tang * rongfu.leng <details><summary>30 commits</summary> <p> * [release/1.6] Prepare v1.6.16 ([containerd#8016](containerd#8016)) * [`d3c595aa3`](containerd@d3c595a) Prepare release notes for v1.6.16 * [release/1.6 backport] Fix tx closed error when upperdirlabel specified ([containerd#8002](containerd#8002)) * [`8c704036a`](containerd@8c70403) Fix tx closed error when upperdirlabel specified * [release/1.6 backport] assorted test-fixes ([containerd#8000](containerd#8000)) * [`91a68edd7`](containerd@91a68ed) cri: Fix TestUpdateOCILinuxResource for host w/o swap controller * [`5594f706e`](containerd@5594f70) Fix TestUpdateContainerResources_Memory* on cgroup v2 hosts * [release/1.6 backport] seccomp updates ([containerd#8001](containerd#8001)) * [`7037f5313`](containerd@7037f53) seccomp: add get_mempolicy, mbind, set_mempolicy, with CAP_SYS_NICE * [`d22919a1c`](containerd@d22919a) seccomp: seccomp: add syscalls related to PKU in default policy * [release/1.6 backport] Harden GITHUB_TOKEN permissions ([containerd#7999](containerd#7999)) * [`8b8a21fe4`](containerd@8b8a21f) Harden GITHUB_TOKEN permissions * [release/1.6 backport] assorted updates to Vagrantfile ([containerd#7996](containerd#7996)) * [`8009948bb`](containerd@8009948) Vagrantfile: fix comments about SELinux * [`550424f92`](containerd@550424f) Vagrantfile: install-rootless-podman: remove `setenforce 0` * [`2c32f8559`](containerd@2c32f85) CI: update Fedora to 37 * [`556bb0cc8`](containerd@556bb0c) Vagrantfile: explicitly specify rsync as the shared folder driver * [`edfac1834`](containerd@edfac18) fix install cni script * [`91d5e53fb`](containerd@91d5e53) Vagrantfile: dump containerd log after critest * [release/1.6 backport] Fix slice append error ([containerd#7995](containerd#7995)) * [`ab193eb20`](containerd@ab193eb) pkg/cri: optimize slice initialization * [`e6cf5ec58`](containerd@e6cf5ec) Fix slice append error * [release/1.6] update to go1.18.10 ([containerd#7992](containerd#7992)) * [`6a8a6531f`](containerd@6a8a653) [release/1.6] update to go1.18.10 * [release/1.6 backport] release/Dockerfile: set DEBIAN_FRONTEND=noninteractive ([containerd#7991](containerd#7991)) * [`d0dc7988a`](containerd@d0dc798) release/Dockerfile: set DEBIAN_FRONTEND=noninteractive * [release/1.6 backport] pushWriter: correctly propagate errors ([containerd#7990](containerd#7990)) * [`1584c2581`](containerd@1584c25) pushWriter: correctly propagate errors * [release/1.6] mod: update github.com/pelletier/[email protected] ([containerd#7942](containerd#7942)) * [`545f22091`](containerd@545f220) mod: update github.com/pelletier/[email protected] </p> </details> * **github.com/pelletier/go-toml** v1.9.3 -> v1.9.5 Previous release can be found at [v1.6.15](https://github.com/containerd/containerd/releases/tag/v1.6.15)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ctr images pushwhile pushing large images #7972In the refactor from 926b9c7, the error handling was substantially reworked, and changed the types of errors returned.
Notably, in the case of a network error, instead of propogating the error through to return from pushWriter.Write (as previously), it would be propagated through to pushWriter.Commit - however, this is too late, since we've already closed the io.Pipe by the time we would have reached this function. Therefore, we get the generic error message "io: read/write on closed pipe" for every network error.
This patch corrects this behavior to ensure that the correct error object is always returned as early as possible, by checking the error result after writing and detecting a closed pipe.
Additionally, we do some additional hardening - specifically we prevent falling through when resetting the content or detecting errors, and update the tests to explicitly check for the ErrReset message.
(cherry picked from commit 9f6058d)