Support network options in rest api #199
Conversation
| @@ -145,6 +146,159 @@ func (c *NetworkConfiguration) Validate() error { | |||
| return nil | |||
| } | |||
|
|
|||
| // FromMap retrieve the configuration data from the map form. | |||
| func (c *NetworkConfiguration) FromMap(data map[string]interface{}) error { | |||
| if i, ok := data["BridgeName"]; ok { | |||
There was a problem hiding this comment.
in addition to checking for ok, i think you should also check for i != nil.
This is because, you have assumed that the client will use a map[string]string to populate the data, but if the client decides to populate it via say json marshalling, then by default all the fields are marshalled with string values marked as nil.
Hence, the map lookup will succeed with ok = true, but the value (i) will be nil and the subsequent type assertion will fail at runtime.
There was a problem hiding this comment.
Thanks, will take care of that.
|
@mavenugo I took care of your comments, please take a look when you get a chance. |
| ops := options.Generic{ | ||
| netlabel.EnableIPv6: true, | ||
| netlabel.GenericData: options.Generic{ | ||
| "AllowNodDefaultName": "true", |
There was a problem hiding this comment.
shouldnt this be AllowNoDefaultName ?
|
ping @mavenugo |
| ) | ||
|
|
||
| const ( | ||
| nullNetType = "null" | ||
| defaultDriverType = "bridge" |
There was a problem hiding this comment.
I prefer to have this patch in client/network.go as part of the multi-net PR. That is the most appropriate PR that can make bridge as the default driver.
There was a problem hiding this comment.
Makes sense. Will move it to the PR #202
|
@aboch circle-ci is failing |
- Also unexporting configuration structures in bridge - Changes in dnet/network.go to set bridge name = network name Signed-off-by: Alessandro Boch <aboch@docker.com>
|
@aboch Thanks for addressing all the comments. |
|
LGTM |
Support network options in rest api
Signed-off-by: Alessandro Boch aboch@docker.com