67ebcd6dcf added an exception for
the "host-gateway" magic value to the validation rules, but didn't
add thise value to any of the tests.
This patch adds the magic value to tests, to verify the validation
is skipped for this magic value.
Note that validation on the client side is "optional" and mostly
done to provide a more user-friendly error message for regular
values (IP-addresses).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These options should never be changed, so using a const for them
instead of a var. Given that these are only used in the opt
package itself, they can be un-exported.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Relates to - moby/moby 40007
The above PR added support in moby, that detects if
a special string "host-gateway" is added to the IP
section of --add-host, and if true, replaces it with
a special IP value (value of --host-gateway-ip Daemon flag
which defaults to the IP of the default bridge).
This PR is needed to skip the validation for the above
feature
Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comments should have a leading space unless the comment is
for special purposes (go:generate, nolint:)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When using advanced syntax for setting config and secret values, default
the target value to the source value when the user does not specify a
target.
Signed-off-by: Nick Adcock <nick.adcock@docker.com>
Allow the use of the advanced source=x syntax for config and secret values when there is no comma
Before this change the following would fail with config not found:
docker service create --name hello1 --config source=myconfig nginx:alpine
And the following would fail with secret not found:
docker service create --name hello2 --secret source=mysecret nginx:alpine
Signed-off-by: Nick Adcock <nick.adcock@docker.com>
```
opts/network_test.go:74:35: Using the variable on range scope `tc` in function literal (scopelint)
assert.NilError(t, network.Set(tc.value))
^
opts/network_test.go:102:40: Using the variable on range scope `tc` in function literal (scopelint)
assert.ErrorContains(t, network.Set(tc.value), tc.expectedError)
^
opts/opts_test.go:270:30: Using the variable on range scope `tc` in function literal (scopelint)
val, err := ValidateLabel(tc.value)
^
opts/opts_test.go:271:7: Using the variable on range scope `tc` in function literal (scopelint)
if tc.expectedErr != "" {
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For now, just verifying that an error is returned, but not checking the
error message itself, because those are not under our control, and may
change with different Go versions.
```
=== Failed
=== FAIL: opts TestParseDockerDaemonHost (0.00s)
hosts_test.go:87: tcp tcp:a.b.c.d address expected error "Invalid bind address format: tcp:a.b.c.d" return, got "parse tcp://tcp:a.b.c.d: invalid port \":a.b.c.d\" after host" and addr
hosts_test.go:87: tcp tcp:a.b.c.d/path address expected error "Invalid bind address format: tcp:a.b.c.d/path" return, got "parse tcp://tcp:a.b.c.d/path: invalid port \":a.b.c.d\" after host" and addr
=== FAIL: opts TestParseTCP (0.00s)
hosts_test.go:129: tcp tcp:a.b.c.d address expected error Invalid bind address format: tcp:a.b.c.d return, got parse tcp://tcp:a.b.c.d: invalid port ":a.b.c.d" after host and addr
hosts_test.go:129: tcp tcp:a.b.c.d/path address expected error Invalid bind address format: tcp:a.b.c.d/path return, got parse tcp://tcp:a.b.c.d/path: invalid port ":a.b.c.d" after host and addr
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This allows setting the ip/ipv6 address as an option in the
advanced `--network` syntax;
```
docker run --network name=mynetwork,ip=172.20.88.22,ip6=2001:db8::8822
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This refactors the way networking options are parsed, and makes the
client able to pass options for multiple networks. Currently, the
daemon does not yet accept multiple networks when creating a container,
and will produce an error.
For backward-compatibility, the following global networking-related
options are associated with the first network (in case multiple
networks are set);
- `--ip`
- `--ip6`
- `--link`
- `--link-local-ip`
- `--network-alias`
Not all of these options are supported yet in the advanced notation,
but for options that are supported, setting both the per-network option
and the global option will produce a "conflicting options" error.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The commit contains cli changes to support driver options for a network in
docker run and docker network connect cli's. The driver-opt, aliases is now
supported in the form of csv as per network option in service commands in
swarm mode since docker/cli#62 . This commit extends this support to docker
run command as well.
For docker connect command `--driver-opt` is added to pass driver specific
options for the network the container is connecting to.
Signed-off-by: Abhinandan Prativadi <abhi@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch fixes a bug where labels use the same behavior as `--env`, resulting
in a value to be copied from environment variables with the same name as the
label if no value is set (i.e. a simple key, no `=` sign, no value).
An earlier pull request addressed similar cases for `docker run`;
2b17f4c8a8, but this did not address the
same situation for (e.g.) `docker service create`.
Digging in history for this bug, I found that use of the `ValidateEnv`
function for labels was added in the original implementation of the labels feature in
abb5e9a077 (diff-ae476143d40e21ac0918630f7365ed3cR34)
However, the design never intended it to expand environment variables,
and use of this function was either due to either a "copy/paste" of the
equivalent `--env` flags, or a misunderstanding (the name `ValidateEnv` does
not communicate that it also expands environment variables), and the existing
`ValidateLabel` was designed for _engine_ labels (which required a value to
be set).
Following the initial implementation, other parts of the code followed
the same (incorrect) approach, therefore leading the bug to be introduced
in services as well.
This patch:
- updates the `ValidateLabel` to match the expected validation
rules (this function is no longer used since 31dc5c0a9a),
and the daemon has its own implementation)
- corrects various locations in the code where `ValidateEnv` was used instead of `ValidateLabel`.
Before this patch:
```bash
export SOME_ENV_VAR=I_AM_SOME_ENV_VAR
docker service create --label SOME_ENV_VAR --tty --name test busybox
docker service inspect --format '{{json .Spec.Labels}}' test
{"SOME_ENV_VAR":"I_AM_SOME_ENV_VAR"}
```
After this patch:
```bash
export SOME_ENV_VAR=I_AM_SOME_ENV_VAR
docker service create --label SOME_ENV_VAR --tty --name test busybox
docker container inspect --format '{{json .Config.Labels}}' test
{"SOME_ENV_VAR":""}
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
gofmt/goimports changed some heuristics in 1.11 and the code is now
formatted slightly differently.
No functional change, just whitespace.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This changes the experimental --console flag to --progress following
feedback indicating avoidable confusion.
In addition to naming changes, the help output now has an additional
clarification, specifically: container output during builds are only
shown when progress output is set to plain. Not mentioning this was also
a big cause of confusion.
Signed-off-by: Tibor Vass <tibor@docker.com>
- Use `Contains` instead of `Include`
- Use `ToJSON` instead of `ToParam`
- Remove usage of `ParseFlag` as it is deprecated too
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
parsing an environment file should give an error in case a zero-length
variable name (definition w/o a variable name) is encountered.
previously these lines went through unnoticed not informing the user about
a potential configuration error.
Signed-off-by: Tom Klingenberg <tklingenberg@lastflood.net>
previously docker did import environment variables if they were present
but created them if they were not when it was asked via a --env-file
cli option to import but not create them.
fix is to only import the variable into the environment if it is present.
additionally do not import variable names of zero-length (which are lines
w/ a potential variable definition w/o a variable name).
refs:
- https://github.com/docker/for-linux/issues/284
Signed-off-by: Tom Klingenberg <tklingenberg@lastflood.net>
test to show current behavior is wrong at parsing an environment file
defining an undefined variable - it must not be defined!
NOTE: this test assume the $HOME variable is always set (see POSIX, this
normally is the case, e.g. the test suite remains stable).
Signed-off-by: Tom Klingenberg <tklingenberg@lastflood.net>
`--label-file` has the exact same behavior as `--env-file`, meaning any
placeholder (i.e. a simple key, no `=` sign, no value), it will get the
value from the environment variable.
For `--label-file` it should just add an empty label.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
If no fields related to an update config or restart policy are
specified, these structs should not be created as part of the service,
to avoid hardcoding the current defaults.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This type is only used by CLI code. It duplicates SecretReference in the
types/swarm package. Change the CLI code to use that type instead.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This adds 'consistency' mode flags to the mount command line argument.
Initially, the valid 'consistency' flags are 'consistent', 'cached',
'delegated', and 'default'.
Signed-off-by: David Sheets <dsheets@docker.com>
Signed-off-by: Jeremy Yallop <yallop@docker.com>
We ignored errors for simple syntax in `PortOpt` (missed that in the
previous migration of this code). This make sure we don't ignore
`nat.Parse` errors.
Test has been migrate too (errors are not exactly the same as before
though -_-)
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
This fix made several updates:
1. Update opts.MemBytes so that default value will not show up.
The reason is that in case a default value is decided by daemon,
instead of client, we actually want to not show default value.
2. Move `docker run/create/build` to use opts.MemBytes for `--shm-size`
This is to bring consistency between daemon and docker run
3. docs updates.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This fix fixes issue raised in 29492 where it was not
possible to specify a default `--default-shm-size` in daemon
configuration for each `docker run``.
The flag `--default-shm-size` which is reloadable, has been
added to the daemon configuation.
Related docs has been updated.
This fix fixes 29492.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This fix catches the case where there is a single container port
and a dynamic host port and will fail out gracefully
Example docker-compose.yml snippet:
port:
ports:
- "8091-8093:8091"
- "80:8080"
Signed-off-by: Tony Abboud <tdabboud@hotmail.com>
Commit a77f2450c70312f8c26877a18bfe2baa44d4abb9 switched `docker run`
to use the `pflags` package. Due to this change, the usage output for
the `--blkio-weight-device` and `--device-*` flags changed and now
showed `weighted-device`, and `throttled-device` as value type. As a
result, the output of `docker run --help` became a lot wider.
This patch changes the output to show `list` instead, which is
consistent with other options that allow to be set multiple times.
Output before this change;
Usage: docker run [OPTIONS] IMAGE [COMMAND] [ARG...]
Run a command in a new container
Options:
--blkio-weight uint16 Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)
--blkio-weight-device weighted-device Block IO weight (relative device weight) (default [])
--device list Add a host device to the container (default [])
--device-read-bps throttled-device Limit read rate (bytes per second) from a device (default [])
--device-read-iops throttled-device Limit read rate (IO per second) from a device (default [])
--device-write-bps throttled-device Limit write rate (bytes per second) to a device (default [])
--device-write-iops throttled-device Limit write rate (IO per second) to a device (default [])
-w, --workdir string Working directory inside the container
Output after this change;
Usage: docker run [OPTIONS] IMAGE [COMMAND] [ARG...]
Run a command in a new container
Options:
--blkio-weight uint16 Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)
--blkio-weight-device list Block IO weight (relative device weight) (default [])
--device list Add a host device to the container (default [])
--device-read-bps list Limit read rate (bytes per second) from a device (default [])
--device-read-iops list Limit read rate (IO per second) from a device (default [])
--device-write-bps list Limit write rate (bytes per second) to a device (default [])
--device-write-iops list Limit write rate (IO per second) to a device (default [])
-w, --workdir string Working directory inside the container
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… or could be in `opts` package. Having `runconfig/opts` and `opts`
doesn't really make sense and make it difficult to know where to put
some code.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
`--publish-add 8081:81 --publish-add 8082:82 --publish-rm 80
--publish-rm 81/tcp --publish-rm 82/tcp` would thus result in 81 and
82 to be published.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Currently `--publish-rm` only accepts `<TargetPort>` or `<TargetPort>[/Protocol]`
though there are some confusions.
Since `--publish-add` accepts `<PublishedPort>:<TargetPort>[/Protocol]`, some user
may provide `--publish-rm 80:80`. However, there is no error checking so the incorrect
provided argument is ignored silently.
This fix adds the check to make sure `--publish-rm` only accepts `<TargetPort>[/Protocol]`
and returns error if the format is invalid.
The `--publish-rm` itself may needs to be revisited to have a better UI/UX experience,
see discussions on:
https://github.com/docker/swarmkit/issues/1396https://github.com/docker/docker/issues/25200#issuecomment-236213242https://github.com/docker/docker/issues/25338#issuecomment-240787002
This fix is short term measure so that end users are not misled by the silently ignored error
of `--publish-rm`.
This fix is related to (but is not a complete fix):
https://github.com/docker/swarmkit/issues/1396
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This fix is based on the comment:
https://github.com/docker/docker/pull/28147#discussion_r86996347
Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.
On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)
For that reason this fix removes the `-ptr`.
Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.
This fix is related to #28147.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This fix tries to address the proposal raised in 27921 and add
`--cpus` flag for `docker run/create`.
Basically, `--cpus` will allow user to specify a number (possibly partial)
about how many CPUs the container will use. For example, on a 2-CPU system
`--cpus 1.5` means the container will take 75% (1.5/2) of the CPU share.
This fix adds a `NanoCPUs` field to `HostConfig` since swarmkit alreay
have a concept of NanoCPUs for tasks. The `--cpus` flag will translate
the number into reused `NanoCPUs` to be consistent.
This fix adds integration tests to cover the changes.
Related docs (`docker run` and Remote APIs) have been updated.
This fix fixes 27921.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
The code for default port was already there but
it didn’t work because split function errored out
before. This should be the desired behavior that
matches daemon listen address with swarm listen
address.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
As described in our ROADMAP.md, introduce new Swarm management API
endpoints relying on swarmkit to deploy services. It currently vendors
docker/engine-api changes.
This PR is fully backward compatible (joining a Swarm is an optional
feature of the Engine, and existing commands are not impacted).
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Victor Vieux <vieux@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Signed-off-by: Madhu Venugopal <madhu@docker.com>
This patch will allow users to specify namespace specific "kernel parameters"
for running inside of a container.
Signed-off-by: Dan Walsh <dwalsh@redhat.com>
This changes the default transport for Windows from unencrypted TCP to
npipe. This is similar to how Linux runs with the unix socket transport by
default.
Signed-off-by: John Starks <jostarks@microsoft.com>
This adds an npipe protocol option for Windows hosts, akin to unix
sockets for Linux hosts. This should become the default transport
for Windows, but this change does not yet do that.
It also does not add support for the client side yet since that
code is in engine-api, which will have to be revendored separately.
Signed-off-by: John Starks <jostarks@microsoft.com>
Sebastiaan van Stijn
Arko Dasgupta
Nick Adcock
Silvin Lubecki
Silvin Lubecki
Abhinandan Prativadi
Tibor Vass
Akihiro Suda
Lifubang
Kir Kolyshkin
Vincent Demeester
Vincent Demeester
Tom Klingenberg
Andrew Hsu
Sami Tabet
Daniel Nephin
Wataru Ishida
Josh Soref
Sebastiaan van Stijn
Aaron Lehmann
Evan Hazlett
David Sheets
Yong Tang
Tony Abboud
yuexiao-wang
Victor Vieux
wefine
Jana Radhakrishnan
Michael Crosby
allencloud
Tonis Tiigi
Dan Walsh
John Howard
John Starks
David Calavera