Commit graph

1354 commits

Author SHA1 Message Date
Sebastiaan van Stijn
949f8c9f2a
Merge pull request #46602 from vvoland/integration-TestPullNonExistingImage
TestPullNonExistingImage: Simplify and move to integration
2023-10-11 19:18:16 +02:00
Paweł Gronowski
0cd02ab1b4
integration/c8d: Skip TestBuildSquashParent
`docker build --squash` is an experimental feature which is not
implemented for containerd image store.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-10-11 15:45:43 +02:00
Paweł Gronowski
5f062032f7
TestPullNonExistingImage: Move to integration
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-10-10 13:36:29 +02:00
Bjorn Neergaard
c217e3c87a
vendor: github.com/moby/buildkit v0.12.2
The following changes were required:
* integration/build: progressui's signature changed in 6b8fbed01e
* builder-next: flightcontrol.Group has become a generic type in 8ffc03b8f0
* builder-next/executor: add github.com/moby/buildkit/executor/resources types, necessitated by 6e87e4b455
* builder-next: stub util/network/Namespace.Sample(), necessitated by 963f16179f

Co-authored-by: CrazyMax <crazy-max@users.noreply.github.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
2023-09-21 14:18:45 -06:00
Sebastiaan van Stijn
4dbfe7e17e
Merge pull request #46502 from rumpl/c8d-fix-diff
c8d: Fix `docker diff`
2023-09-20 21:16:08 +02:00
Djordje Lukic
207c4d537c c8d: Fix docker diff
Diffing a container yielded some extra changes that come from the
files/directories that we mount inside the container (/etc/resolv.conf
for example). To avoid that we create an intermediate snapshot that has
these files, with this we can now diff the container fs with its parent
and only get the differences that were made inside the container.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2023-09-20 14:16:22 +02:00
Djordje Lukic
d5425f28ae
Don't run tests twice
It's useless

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2023-09-19 11:37:55 +02:00
Djordje Lukic
28861e0c47
c8d: Skip TestRemoveImageGarbageCollector
This test checks how the layer store works, so we don't need it when we
use containerd as image store

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2023-09-18 14:35:57 +02:00
Albin Kerouanton
5e15ed314b
api: Improve error on ContainerCreate with multiple endpoints
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2023-09-15 14:30:03 +02:00
Albin Kerouanton
bbcd662532
api: Allow ContainerCreate to take several EndpointsConfig for >= 1.44
The API endpoint `/containers/create` accepts several EndpointsConfig
since v1.22 but the daemon would error out in such case. This check is
moved from the daemon to the api and is now applied only for API < 1.44,
effectively allowing the daemon to create containers connected to
several networks.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2023-09-15 10:07:29 +02:00
Sebastiaan van Stijn
39b2bf51ca
Merge pull request #46406 from akerouanton/issue-46404
daemon: fix under what conditions container's mac-address is applied
2023-09-13 23:35:07 +02:00
Albin Kerouanton
78479b1915
libnet: Make sure network names are unique
Fixes #18864, #20648, #33561, #40901.

[This GH comment][1] makes clear network name uniqueness has never been
enforced due to the eventually consistent nature of Classic Swarm
datastores:

> there is no guaranteed way to check for duplicates across a cluster of
> docker hosts.

And this is further confirmed by other comments made by @mrjana in that
same issue, eg. [this one][2]:

> we want to adopt a schema which can pave the way in the future for a
> completely decentralized cluster of docker hosts (if scalability is
> needed).

This decentralized model is what Classic Swarm was trying to be. It's
been superseded since then by Docker Swarm, which has a centralized
control plane.

To circumvent this drawback, the `NetworkCreate` endpoint accepts a
`CheckDuplicate` flag. However it's not perfectly reliable as it won't
catch concurrent requests.

Due to this design decision, API clients like Compose have to implement
workarounds to make sure names are really unique (eg.
docker/compose#9585). And the daemon itself has seen a string of issues
due to that decision, including some that aren't fixed to this day (for
instance moby/moby#40901):

> The problem is, that if you specify a network for a container using
> the ID, it will add that network to the container but it will then
> change it to reference the network by using the name.

To summarize, this "feature" is broken, has no practical use and is a
source of pain for Docker users and API consumers. So let's just remove
it for _all_ API versions.

[1]: https://github.com/moby/moby/issues/18864#issuecomment-167201414
[2]: https://github.com/moby/moby/issues/18864#issuecomment-167202589

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2023-09-12 10:40:13 +02:00
Sebastiaan van Stijn
5e7eade1f7
integration: don't poll for containers to be running
container.Run() should be a synchronous operation in normal circumstances;
the container is created and started, so polling after that for the
container to be in the "running" state should not be needed.

This should also prevent issues when a container (for whatever reason)
exited immediately after starting; in that case we would continue
polling for it to be running (which likely would never happen).

Let's skip the polling; if the container is not in the expected state
(i.e. exited), tests should fail as well.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-09-08 23:05:43 +02:00
Albin Kerouanton
6cc6682f5f
daemon: fix under what conditions container's mac-address is applied
The daemon would pass an EndpointCreateOption to set the interface MAC
address if the network name and the provided network mode were matching.
Obviously, if the network mode is a network ID, it won't work. To make
things worse, the network mode is never normalized if it's a partial ID.

To fix that: 1. the condition under what the container's mac-address is
applied is updated to also match the full ID; 2. the network mode is
normalized to a full ID when it's only a partial one.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2023-09-08 18:15:00 +02:00
Sebastiaan van Stijn
5f59f7bb49
integration/container: combine TestResize tests into subtests
Reduce some of the boiler-plating, and by combining the tests, we skip
the testenv.Clean() in between each of the tests. Performance gain isn't
really measurable, but every bit should help :)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-09-08 01:36:12 +02:00
Sebastiaan van Stijn
a4ceb0e4ac
integration/container: TestResize, TestResizeWithInvalidSize: rm poll.WaitOn
container.Run should be an synchronous operation; the container should
be running after the request was made (or produce an error). Simplify
these tests, and remove the redundant polling.

These were added as part of 8f800c9415,
but no such polls were in place before the refactor, and there's no
mention of these during review of the PR, so I assume these were just
added either as a "precaution", or a result of "copy/paste" from another
test.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-09-08 01:34:45 +02:00
Sebastiaan van Stijn
ee7ca6822a
integration/container: fix flaky TestResizeWhenContainerNotStarted
This test was failing frequently on Windows, where the test was waiting
for the container to exit before continuing;

    === FAIL: github.com/docker/docker/integration/container TestResizeWhenContainerNotStarted (18.69s)
    resize_test.go:58: timeout hit after 10s: waiting for container to be one of (exited), currently running

It looks like this test is merely validating that a container in any non-
running state should produce an error, so there's no need to run a container
(waiting for it to stop), and just "creating" a container (which would be
in `created` state) should work for this purpose.

Looking at 8f800c9415, I see `createSimpleContainer`
and `runSimpleContainer` utilities were added, so I'm even wondering if the
original intent was to use `createSimpleContainer` for  this test.

While updating, also check if we get the expected error-type, instead of
only checking for the error-message.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-09-08 01:33:22 +02:00
Brian Goff
9b7784781d Add test for buildkit history trace propagation
This test ensures that we are able to propagate traces into buildkit's
history API.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2023-09-07 18:38:22 +00:00
Brian Goff
e8dc902781 Wire up tests to support otel tracing
Integration tests will now configure clients to propagate traces as well
as create spans for all tests.

Some extra changes were needed (or desired for trace propagation) in the
test helpers to pass through tracing spans via context.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2023-09-07 18:38:22 +00:00
Brian Goff
642e9917ff Add otel support
This uses otel standard environment variables to configure tracing in
the daemon.
It also adds support for propagating trace contexts in the client and
reading those from the API server.

See
https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/
for details on otel environment variables.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2023-09-07 18:38:19 +00:00
Sebastiaan van Stijn
791549508a
Merge pull request #46084 from rumpl/fix-test-arch
test: Remove DOCKER_ENGINE_GOARCH from the tests
2023-09-05 18:08:57 +02:00
Sebastiaan van Stijn
9c4e82435e
Merge pull request #46351 from thaJeztah/api_events_actions_enum
api/types/events: define "Action" type and consts
2023-09-05 11:11:42 +02:00
Djordje Lukic
84a4f37cf7
test: use info from the version endpoint for arch checks
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2023-08-31 09:36:48 +02:00
Paweł Gronowski
aef703fa1b
integration/liveRestore: Check volume content
Make sure that the content in the live-restored volume mounted in a new
container is the same as the content in the old container.
This checks if volume's _data directory doesn't get unmounted on
startup.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-08-29 11:46:29 +02:00
Sebastiaan van Stijn
0f871f8cb7
api/types/events: define "Action" type and consts
Define consts for the Actions we use for events, instead of "ad-hoc" strings.
Having these consts makes it easier to find where specific events are triggered,
makes the events less error-prone, and allows documenting each Action (if needed).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-29 00:38:08 +02:00
Sebastiaan van Stijn
a65c948e7e
Merge pull request #46335 from thaJeztah/api_move_checkpoint_types
api/types: move checkpoint-types to api/types/checkpoint
2023-08-28 19:02:19 +02:00
Sebastiaan van Stijn
8309206160
Merge pull request #46350 from thaJeztah/strongtype_eventstype
api/types/events: make events.Type an actual type
2023-08-28 16:44:26 +02:00
Sebastiaan van Stijn
fa79b5d59f
integration/container: TestPause: don't depend on deprecated fields
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-28 13:17:01 +02:00
Sebastiaan van Stijn
70ad5b818f
api/types/events: make events.Type an actual type
This type was added in 247f4796d2, and
at the time was added as an alias for string;

> api/types/events: add "Type" type for event-type enum
>
> Currently just an alias for string, but we can change it to be an
> actual type.

Now that all code uses the defined types, we should be able to make
this an actual type.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-28 13:12:38 +02:00
Sebastiaan van Stijn
8569e8684f
Merge pull request #46338 from thaJeztah/daemon_events_cleanup
daemon: clean up event handling-code, and remove some dead code
2023-08-28 13:12:10 +02:00
Sebastiaan van Stijn
5a02ed5e84
integration: use events-consts in tests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-27 23:44:25 +02:00
Sebastiaan van Stijn
fa13b0715f
integration/internal/swarm: rename max/min as it collides with go1.21 builtin
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-26 22:02:25 +02:00
Sebastiaan van Stijn
350223201e
integration/container: TestCheckpoint: remove intermediate vars
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-26 12:44:16 +02:00
Sebastiaan van Stijn
b688af2226
api/types: move checkpoint-types to api/types/checkpoint
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-26 12:37:41 +02:00
Sebastiaan van Stijn
f10e182ca7
Merge pull request #46317 from thaJeztah/gofumpt_all_the_things
Format code with gofumpt
2023-08-25 01:06:05 +02:00
Sebastiaan van Stijn
07e6b0ac70
integration: format code with gofumpt
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-24 17:55:14 +02:00
Sebastiaan van Stijn
bc7f341f29
daemon: WithNamespaces(): fix incorrect error for PID, IPC namespace
`Daemon.getPidContainer()` was wrapping the error-message with a message
("cannot join PID of a non running container") that did not reflect the
actual reason for the error; `Daemon.GetContainer()` could either return
an invalid parameter (invalid / empty identifier), or a "not found" error
if the specified container-ID could not be found.

In the latter case, we don't want to return a "not found" error through
the API, as this would indicate that the container we're _starting_ was
not found (which is not the case), so we need to convert the error into
an `errdefs.ErrInvalidParameter` (the container-ID specified for the PID
namespace is invalid if the container doesn't exist).

This logic is similar to what we do for IPC namespaces. which received
a similar fix in c3d7a0c603.

This patch updates the error-types, and moves them into the getIpcContainer
and getPidContainer container functions, both of which should return
an "invalid parameter" if the container was not found.

It's worth noting that, while `WithNamespaces()` may return an "invalid
parameter" error, the `start` endpoint itself may _not_ be. as outlined
in commit bf1fb97575, starting a container
that has an invalid configuration should be considered an internal server
error, and is not an invalid _request_. However, for uses other than
container "start", `WithNamespaces()` should return the correct error
to allow code to handle it accordingly.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-24 16:19:07 +02:00
Sebastiaan van Stijn
64f5d9b119
Merge pull request #46213 from thaJeztah/daemon_remove_errors
daemon: cleanupContainer: don't fail if container is already stopped
2023-08-24 13:34:43 +02:00
Sebastiaan van Stijn
4b0d38de06
TestDiskUsage: don't panic if results don't match
This test is currently failing with containerd-integration, which should
be looked into, but let's start with preventing it from panicking, to make
the test-failures less noisy;

        --- FAIL: TestDiskUsage/after_container.Run (0.26s)
    panic: runtime error: index out of range [0] with length 0 [recovered]
        panic: runtime error: index out of range [0] with length 0

    goroutine 280 [running]:
    testing.tRunner.func1.2({0xb07a00, 0x40002006a8})
        /usr/local/go/src/testing/testing.go:1526 +0x1c8
    testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1529 +0x364
    panic({0xb07a00, 0x40002006a8})
        /usr/local/go/src/runtime/panic.go:884 +0x1f4
    github.com/docker/docker/integration/system.TestDiskUsage.func3(0x0?, {0x0, {0x14ea4a8, 0x0, 0x0}, {0x14ea4a8, 0x0, 0x0}, {0x14ea4a8, 0x0, ...}, ...})
        /go/src/github.com/docker/docker/integration/system/disk_usage_test.go:82 +0x7e4
    github.com/docker/docker/integration/system.TestDiskUsage.func4(0x4000235c80?)
        /go/src/github.com/docker/docker/integration/system/disk_usage_test.go:118 +0x8c

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-23 19:15:21 +02:00
Sebastiaan van Stijn
c0568a95d8
integration/container: check some error-types in tests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-23 15:50:46 +02:00
Sebastiaan van Stijn
2b583c0923
daemon: cleanupContainer: slightly cleanup error messages
Also remove integration-cli: `DockerAPISuite.TestContainerAPIDeleteConflict`,
which was testing the same conditions as `TestRemoveContainerRunning` in
integration/container.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-23 15:50:43 +02:00
Djordje Lukic
6cbe06ff3f
test: Skip graph driver tests when using containerd snapshotters
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2023-08-23 09:48:27 +02:00
Sebastiaan van Stijn
2be118379e
api/types/container: add RestartPolicyMode type and enum
Also move the validation function to live with the type definition,
which allows it to be used outside of the daemon as well.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-22 16:40:57 +02:00
Sebastiaan van Stijn
1c937c5816
Merge pull request #46189 from vvoland/c8d-more-mount-refcount
c8d integration: Use refcount mounter for diff and export
2023-08-18 15:29:13 +02:00
Sebastiaan van Stijn
17571ff199
integration/internal/container: add WithPIDMode option
Some files used aliases, others didn't, and they didn't appear to be
required.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-12 19:06:01 +02:00
Sebastiaan van Stijn
917dae58e1
integration/internal/container: remove import aliases
Some files used aliases, others didn't, and they didn't appear to be
required.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-12 19:06:01 +02:00
Paweł Gronowski
303e2b124e
integration: Add test for not breaking overlayfs
Check that operations that could potentially perform overlayfs mounts
that could cause undefined behaviors.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-08-11 15:30:29 +02:00
Sebastiaan van Stijn
74feadacf8
integration/internal/container: refactor CreateExpectingErr
This utility was only used for a single test, and it was very limited
in functionality as it only allowed for a certain error-string to be
matched.

Let's change it into a more generic function; a helper that allows a
container to be created from a `TestContainerConfig` (which can be
constructed using `NewTestConfig`) and that returns the response from
client.ContainerCreate(), so that any result from that can be tested,
leaving it up to the test to check the results.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-11 14:03:08 +02:00
Sebastiaan van Stijn
0899ba4a3f
integration/internal/container: add NewTestConfig utility
Introduce a NewTestConfig utility, to allow using the available utilities
for constructing a config, and use them with the regular API client.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-11 14:03:08 +02:00
Sebastiaan van Stijn
3cb52a6359
integration/internal/container: use consistent name for api-client
The `client` variable was colliding with the `client` import. In some cases
the confusing `cli` name (it's not the "cli") was used. Given that such names
can easily start spreading (through copy/paste, or "code by example"), let's
make a one-time pass through all of them in this package to use the same name.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-11 14:02:17 +02:00