IPv6 only: add API option enable/disable IPv4#48271
Conversation
Signed-off-by: Rob Murray <[email protected]>
corhere
left a comment
There was a problem hiding this comment.
LGTM! Small PRs for the win!
api/swagger.yaml
Outdated
| description: "Optional custom IP scheme for the network." | ||
| $ref: "#/definitions/IPAM" | ||
| EnableIPv4: | ||
| description: "Enable IPv4 on the network." |
There was a problem hiding this comment.
It'd great to leave a note about the experimentalness of this property, both to warn API users that its implementation / usage / semantics might evolve (although unlikely), and also to warn that it's not available if dockerd's experimental flag isn't set. For instance, we do something similar for CDISpecDirs.
| return nil, errdefs.InvalidParameter(fmt.Errorf("driver-opt %q is not a valid bool", netlabel.EnableIPv4)) | ||
| } | ||
| } | ||
| if !enableIPv4 && !daemon.config().Experimental && create.ConfigFrom == nil { |
There was a problem hiding this comment.
We're trying to move away from 'experimental' features as it's coarse-grained, to replace it with fine-grained feature flags. Would it make sense to introduce a new feature flag for that?
There was a problem hiding this comment.
This PR on its own is basically-useless (!). It'll carry on being basically-useless for a few more PRs yet.
I don't think we've decided to release/announce IPv6-only (EnableIPv4:false) as experimental, or behind a feature flag - but, we could do either if we decide to release with it still incomplete. Depending on what that looks like, we might want to feature-flag it for a specific network driver or something like that. Or, if it looks risky and likely to change, we might make the whole thing experimental / feature-flagged until we get some feedback. But, I don't think we'll know until we're at that point.
So, just locking it behind --experimental for now seems like the best approach?
There was a problem hiding this comment.
Fair enough, I won't dwell on that.
Signed-off-by: Rob Murray <[email protected]>
Similar to EnableIPv6:
- Set it if EnableIPv4 is specified in a create request.
- Otherwise, set it if included in `default-network-opts`.
- Apart from in a config-from network, so that it doesn't look
like the API request set the field.
- Include the new field in Network marshalling/unmarshalling test.
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
3d1d1eb to
1f542d5
Compare
|
I forgot to ignore
|
## Description Updates for moby 28.0 networking. ## Related issues or tickets Series of commits ... - Fix description of 'inhibit_ipv4' - Not changed in moby 28.0, updated to clarify difference from (new) IPv6-only networks. - Updates to default bridge address config - moby/moby#48319 - Describe IPv6-only network config - moby/moby#48271 - docker/cli#5599 - Update description of gateway modes - moby/moby#48594 - moby/moby#48596 - moby/moby#48597 - Describe gateway selection in the networking overview. - docker/cli#5664 - Describe gateway mode `isolated` - moby/moby#49262 ## Reviews <!-- Notes for reviewers here --> <!-- List applicable reviews (optionally @tag reviewers) --> - [ ] Technical review - [ ] Editorial review - [ ] Product review --------- Signed-off-by: Rob Murray <[email protected]>
- What I did
--ipv4in the CLI, equivalent to--ipv6).com.docker.network.enable_ipv4.default-network-opts.EnableIPv4=false.EnableIPv4=true.\EnableIPv4in a network create request if the API version is less than 1.47.--experimentalto disable IPv4.Follow-up PRs will make the option do-something.
- How I did it
The first commit here, bumping the API version to 1.47, is likely to disappear - as this will be merged after another PR that does the same thing. But, for now, it means there's somewhere to put an API
version-history.mdupdate.The rest is fairly machanical copying of
EnableIPv6behaviour.- How to verify it
Can't disable IPv4 without
--experimental:With
--experimental, can disable IPv4:Default for a new bridge network is
true:Predefined host network shows
EnableIPv4:false, like the existingEnableIPv6:false:Predefined bridge network has
EnableIPv4:true:Marshalling/unmarshalling a
libnetwork.NetworkwithEnableIPv4:trueis covered in an updated unit test.- Description for the changelog