Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[18.03] Don't make container mount unbindable#516

Merged
andrewhsu merged 36 commits into
docker-archive:18.03from
thaJeztah:18.03-backport-booo
Apr 19, 2018
Merged

[18.03] Don't make container mount unbindable#516
andrewhsu merged 36 commits into
docker-archive:18.03from
thaJeztah:18.03-backport-booo

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

This PR is based on top of #513 to get a clean cherry-pick; I'll rebase once that's merged; only the last commit is the change for this PR

backport of moby/moby#36768 for 18.03

git cherry-pick -s -S -x -Xsubtree=components/engine 4c000662feb3c8e3d63cbcb044a47f627cd9bb45

no conflicts

kolyshkin and others added 30 commits April 17, 2018 15:09
In case ContainerExport() is called for an unmounted container, it leads
to a daemon panic as container.BaseFS, which is dereferenced here, is
nil.

To fix, do not rely on container.BaseFS; use the one returned from
rwlayer.Mount().

Fixes: 7a7357d ("LCOW: Implemented support for docker cp + build")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 81f6307)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 7a7357d ("LCOW: Implemented support for docker cp + build")
changed `container.BaseFS` from being a string (that could be empty but
can't lead to nil pointer dereference) to containerfs.ContainerFS,
which could be be `nil` and so nil dereference is at least theoretically
possible, which leads to panic (i.e. engine crashes).

Such a panic can be avoided by carefully analysing the source code in all
the places that dereference a variable, to make the variable can't be nil.
Practically, this analisys are impossible as code is constantly
evolving.

Still, we need to avoid panics and crashes. A good way to do so is to
explicitly check that a variable is non-nil, returning an error
otherwise. Even in case such a check looks absolutely redundant,
further changes to the code might make it useful, and having an
extra check is not a big price to pay to avoid a panic.

This commit adds such checks for all the places where it is not obvious
that container.BaseFS is not nil (which in this case means we do not
call daemon.Mount() a few lines earlier).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit d6ea46c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test case checks that a container created before start
of the currently running dockerd can be exported (as reported
in #36561). To satisfy this condition, either a pre-existing
container is required, or a daemon restart after container
creation.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 6e7141c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: John Howard <jhoward@microsoft.com>
(cherry picked from commit 0f5fe3f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit ef01dea)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 073963e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
gty-migrate-from-testify --ignore-build-tags

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 6be0f70)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit c9e52bd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 7d8815e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 58de627)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As mentioned in commit 9e31938, test cases that use t.Parallel()
and start a docker daemon might step on each other toes as they
try to configure iptables during startup, resulting in flaky tests.

To avoid this, --iptables=false should be used while starting daemon.

Fixes: eaa5192 ("Make container resource mounts unbindable")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit c125e10)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 4bb0f24)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… mainly by skipping if daemon is remote.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 6016e79d2552b21643f4bfd093ce76d8ef956d79)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…spect_test, container/ps_test, container/stop_test

Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
(cherry picked from commit 78e4be91332e2237c0fa14eb3ba0fb5b915c3256)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit e55d6fc8573580f6eea009cd7f1034aa912128ef)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adding `busybox:latest` and `busybox:glibc` as the frozen images

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit 3ae45c5)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Using the `busybox:glibc` instead of `busybox:latest` to the
network related test cases (`ping` issue).

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit 0d31dee)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All `Macvlan` related test on `DockerSuite` and `DockerNetworkSuite`
are migrated to `macvlan_test.go`.

Also, as `macvlan` seems to be out of experimental, this removes
the *skip* when the run is not experimental (and doesn't start a
daemon with experimental either).

The end goal being to remove the `experimental` builds.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit ef5bc603266b9fa5df525319d67329ebc14a8ee7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… and do not use the `docker` cli in it. One of the reason of this
move is to not make `integration` package using legacy
`integration-cli` package.

Next move will be to support swarm within this package *and* provide
some helper function using the api (compared to the one using cli in
`integration-cli/daemon` package).

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit f0d277fe84a72b29c0d2d541c20d5a9c4d7e4884)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
	TestServiceWithPredefinedNetwork test case was failing
	at times. To fix the issue, added new API to check
	for services after we clean up all services. Tested
	multiple times and this sould fix flaky issue.

Signed-off-by: selansen <elango.siva@docker.com>
(cherry picked from commit dabffd806c98ab13dbc25e57bee21c5291b9a50c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add the default function per resource to override the `pollSettings`
which will be re-used where it's needed.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit ee6959addc5664a5c55765f2c721f84414ea4779)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Using the default PollSettings functions to adjust the timeout
value instead of changing the value each time when needed.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit b8912feeffcdfd489c9fc1212277840adac2719c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This remove the daemon.Swarm construction by make the new test Daemon
struct aware of swarm.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 83d18cf4e3e84055f7034816eed2a10c04e777ca)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All `Ipvlan` related test on `DockerSuite` and `DockerNetworkSuite`
are migrated to `ipvlan_test.go`.

The end goal being to remove the `experimental` builds.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 24f934751120ea420b7ba4d2e314df805f3eff06)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 0ab6116ce868a0fc671a49a89696e3a1ce35e26d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… making each folder/suites quicker to run

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit a3323d2e4349b7e8d449c6e571ca3d4aa3e53d63)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Move the code from `integration-cli` to `internal/test`.
- Use `testingT` and `assert` when creating the registry.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 66de2e6e3b6d927a3396743cd7c363aa9f7b776e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Move go package used by both `integration-cli` and `integration` to
  `internal/test/fixtures`.
- Remove fixtures that are not used anymore (moved to `docker/cli` a
  while ago) : deploy, notary, secrets.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 5f56503f583f21d655394f755f71849381bd58c7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All `docker build` tests that require an `ExperimentalDaemon` are
migrated to `integration/build` package and start an experimental
daemon to test on it.

The end goal being to remove the `experimental` builds.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 183076e89df64928bd2e94ad0da9725b482367cd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 239a8a518904dfb51fe62087d8702519c20ce808)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
yongtang and others added 6 commits April 17, 2018 18:34
This fix converts some `client.ContainerCreate` to `container.Create`,
and removes some unneeded `name` fields when test containers are created.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit ab9bb47b05b1dde445a5e4ba78ae97303208dc8b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 062564084a22f71cf5807ae5dfad7d29afb12e04)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This fix migrates image tag tests from integration-cli to api tests.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit 9bcb960508a6066811cffcca1e35ca44d7f1cf94)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Anda Xu <anda.xu@docker.com>
(cherry picked from commit 7380935331f0c35315003578258f6c1f47c1a586)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A minor nit. `test01` never been created and used in
`TestDockerNetworkInspectCustomSpecified()` function, so correct it.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
(cherry picked from commit f041953d04bffa2be05466173f02dd016c68286d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit 4c000662feb3c8e3d63cbcb044a47f627cd9bb45)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 18.03.1 milestone Apr 18, 2018
@kolyshkin
Copy link
Copy Markdown
Contributor

last commit LGTM

@andrewhsu andrewhsu merged commit db5f5c0 into docker-archive:18.03 Apr 19, 2018
@thaJeztah thaJeztah deleted the 18.03-backport-booo branch April 19, 2018 00:02
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 31, 2020
[18.03] Don't make container mount unbindable
docker-jenkins pushed a commit that referenced this pull request Dec 15, 2020
Bump buildx to v0.5.1
Upstream-commit: 4865d03
Component: packaging
akrasnov-drv pushed a commit to drivenets/docker-ce that referenced this pull request Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.