Adding csv format parameters to network option in service create/update #32975
Conversation
|
ping @thaJeztah |
|
Thanks! Should we have the same format added to |
|
Also this relates to #31964 (although not all of the options are implemented yet) |
|
Hm, is this by design, or overlooked? docker network create -d overlay docknet
docker service create \
--name web \
--network name=docknet,alias=web1,alias=web2 \
--network name=docknet,alias=web3,alias=web4 \
nginx:alpine |
|
Hm, looks like that's already possible in 17.05 (attach twice to the same network), gonna check 17.03/17.04 |
|
@thaJeztah : you beat me to it. Yes its possible in the current code as well. I can add a sanity check . |
|
@thaJeztah we should get this for And yes, this is related to #31964 the only 2 networks options that we support today for |
| case networkOptAlias: | ||
| netAttach.Aliases = append(netAttach.Aliases, value) | ||
| default: | ||
| return fmt.Errorf("invalid field key %s", key) |
There was a problem hiding this comment.
I think we should make the error messages consistent with what we have for --mount, e.g.
Line 129 in 49376cd
| } | ||
|
|
||
| if len(parts) != 2 { | ||
| return fmt.Errorf("invalid field %s", field) |
There was a problem hiding this comment.
Same here for consistent error messages
Line 82 in 49376cd
There was a problem hiding this comment.
@thaJeztah I was. following the error message https://github.com/moby/moby/blob/master/opts/port.go#L43
| return fmt.Errorf("invalid field key %s", key) | ||
| } | ||
| } | ||
| n.networkAttachments = append(n.networkAttachments, netAttach) |
There was a problem hiding this comment.
Should we validate here if netAttach.name (or ID?) is set?
There was a problem hiding this comment.
both work. So shouldn't be a problem
|
@thaJeztah thanks for taking a quick look. I will split this pr in docker/cli and here to continue the discussion further. |
The commit adds capability to accept csv parameters for network option in service create/update commands. With this the following will be supported docker service create --name web --network name=docknet,alias=web1,alias=web2 nginx docker service create --name web --network docknet nginx docker service update web --network-add name=docknet,alias=web1 docker service update web --network-rm docknet Signed-off-by: Abhinandan Prativadi <abhi@docker.com>
Signed-off-by: Abhinandan Prativadi <abhi@docker.com>
|
closing this in favor of #33130 |
- What I did
Added capability to service create/update to accept network option in csv format and add test case to verify the same.
- How I did it
Changed
--network,--network-addoptions to accept csv format key/value by create a new network option type.- How to verify it
- A picture of a cute animal (not mandatory but encouraged)