-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix advanced options for backward compatibility #1963
Fix advanced options for backward compatibility #1963
Conversation
ping @wsong @andrewhsu @silvin-lubecki PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Codecov Report
@@ Coverage Diff @@
## master #1963 +/- ##
==========================================
+ Coverage 56.72% 56.73% +<.01%
==========================================
Files 310 310
Lines 21800 21803 +3
==========================================
+ Hits 12367 12370 +3
Misses 8518 8518
Partials 915 915 |
ping @tiborvass @andrewhsu @silvin-lubecki PTAL |
cli/command/container/opts.go
Outdated
// and only a single network is specified, omit the endpoint-configuration | ||
// on the client (the daemon will still create it when creating the container) | ||
if i == 0 && len(copts.netMode.Value()) == 1 { | ||
if ep == nil || reflect.DeepEqual(ep, &networktypes.EndpointSettings{}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ep == nil || reflect.DeepEqual(ep, &networktypes.EndpointSettings{}) { | |
if ep == nil || reflect.DeepEqual(*ep, networktypes.EndpointSettings{}) { |
For backward compatibility: if no custom options are provided for the network, and only a single network is specified, omit the endpoint-configuration on the client (the daemon will still create it when creating the container) This fixes an issue on older versions of legacy Swarm, which did not support `NetworkingConfig.EndpointConfig`. This was introduced in 5bc0963 (docker#1767) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
393c4f3
to
4d7e6bf
Compare
@tiborvass thanks! updated; ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
For backward compatibility: if no custom options are provided for the network,
and only a single network is specified, omit the endpoint-configuration
on the client (the daemon will still create it when creating the container)
This fixes an issue on older versions of legacy Swarm, which did not support
NetworkingConfig.EndpointConfig
.This was introduced in 5bc0963 (#1767)
fixes #1854
relates to docker-archive/classicswarm#2941