api: Make EnableIPv6 optional (impl #2 - Option-based)#47872
api: Make EnableIPv6 optional (impl #2 - Option-based)#47872akerouanton wants to merge 1 commit intomoby:masterfrom
Conversation
6657775 to
99e315b
Compare
99e315b to
400b431
Compare
api/types/types.go
Outdated
| Driver string // Driver is the driver-name used to create the network (e.g. `bridge`, `overlay`) | ||
| Scope string // Scope describes the level at which the network exists (e.g. `swarm` for cluster-wide or `local` for machine level). | ||
| EnableIPv6 bool // EnableIPv6 represents whether to enable IPv6. | ||
| EnableIPv6 optional.Option[bool] // EnableIPv6 represents whether to enable IPv6. |
There was a problem hiding this comment.
| EnableIPv6 optional.Option[bool] // EnableIPv6 represents whether to enable IPv6. | |
| EnableIPv6 optional.Option[bool] `json:",omitempty"` // EnableIPv6 represents whether to enable IPv6. |
to suppress marhsalling {"EnableIPv6": null}.
400b431 to
84bb887
Compare
Currently, starting dockerd with `--default-network-opt=bridge=com.docker.network.enable_ipv6=true` has no effect as `NetworkCreateRequest.EnableIPv6` is a basic bool. This change uses a Rust-like `Option` type to mark the field as optional. If clients don't specify it, the default-network-opt will be applied if it's specified, otherwise it defaults to false. Signed-off-by: Albin Kerouanton <[email protected]>
84bb887 to
578f1a9
Compare
|
Discussed during today's maintainers call and internally. Overall, everyone agrees We might still do that change at a latter time, but for all optional fields at once. And the 2nd problem could be overcomed by the use of an interface to not force a specific implementation. Since #47867 has been merged, let's close this one. |
Interfaces are reference types, which have the same aliasing issues as plain pointers. I suggested adding a concrete facade type to the type Option[T] struct { v optional.Option[T] }
func (o Option[T]) IsSome() bool { return o.v.IsSome() }
func (o option[T]) IsNone() bool { return o.v.IsNone() }
// etc... |
EnableIPv6optional.Optiontype to make the field optional.- What I did
Currently, starting dockerd with
--default-network-opt=bridge=com.docker.network.enable_ipv6=truehas no effect asNetworkCreateRequest.EnableIPv6is a basic bool.This change uses a Rust-like
Optiontype to mark the field as optional. If clients don't specify it, the default-network-opt will be applied if it's specified, otherwise it defaults to false.- How to verify it
CI -- a new integration test has been added but will fail until #47853 is merged.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)