ipam/host-local: support multiple IP ranges #12
Conversation
|
build failure is due to a minor Travis misconfiguration |
| } | ||
|
|
||
| // Releases all IPs allocated for the container with given ID | ||
| // Relase clears all IPs allocated for the container with given ID |
| // This is not critical, we just lose round-robin this one time. | ||
| lastReservedIP, err := a.store.LastReservedIP(a.rangeID) | ||
| if err != nil && !os.IsNotExist(err) { | ||
| log.Printf("Error retriving last reserved ip: %v", err) |
| } | ||
|
|
||
| // CNI spec 0.2.0 and below supported only one v4 and v6 address | ||
| if numV4 > 1 || numV6 > 1 { |
There was a problem hiding this comment.
Shouldn't this be a check for "if numV4 + numV6 > 1". I thought the CNI versions <= 0.2.0 only supported a single IP address in the stdout sent back to whatever invokes the plugin (either v4-only or v6-only)?
There was a problem hiding this comment.
Nah, 0.1.0 supports one v4 and v6.
| "type": "host-local", | ||
| "ranges": [ | ||
| { | ||
| "subnet": "10.1.2.0/24", |
There was a problem hiding this comment.
Should there be a "Should detect overlap" test case for IPv6?
| "master": "foo0", | ||
| "args": { | ||
| "cni": { | ||
| "ips": [ "10.1.2.11", "11.11.11.11"] |
There was a problem hiding this comment.
Should there be a "Should parse config args" test case for IPv6?
| @@ -289,4 +323,203 @@ var _ = Describe("host-local Operations", func() { | |||
| Expect(err).NotTo(HaveOccurred()) | |||
| Expect(strings.Index(string(out), "Error retriving last reserved ip")).To(Equal(-1)) | |||
| }) | |||
|
|
|||
| It("allocates a custom IP when requested by config args", func() { | |||
There was a problem hiding this comment.
Do we need an "allocates a custom IP..." test case for IPv6, or is the custom IP logic agnostic of family?
| RangeStart: net.ParseIP("10.0.0.128"), | ||
| }, | ||
| false), | ||
| Entry("same subnet, overlapping start + end", |
There was a problem hiding this comment.
Need overlapping, non-overlapping, and containing range tests for V6?
|
@leblancd updated; we don't need specific v6 in most cases since the logic is family agnostic. The important functions ( I added a few more end-to-end v6 tests but duplicating every test case isn't needed. |
plugins/ipam/host-local/README.md
Outdated
| { | ||
| "subnet": "3ffe:ffff:0:01ff::/64", | ||
| "rangeStart": "3ffe:ffff:0:01ff::0010", | ||
| "rangeEnd": "3ffe:ffff:0:01ff::0020", |
There was a problem hiding this comment.
Tiny nit: line should not end with a comma
plugins/ipam/host-local/README.md
Outdated
| @@ -49,34 +60,66 @@ We can test it out on the command-line: | |||
| ```bash | |||
| $ export CNI_COMMAND=ADD | |||
| $ export CNI_CONTAINERID=f81d4fae-7dec-11d0-a765-00a0c91e6bf6 | |||
| $ echo '{ "name": "default", "ipam": { "type": "host-local", "subnet": "203.0.113.0/24" } }' | ./host-local | |||
| $ echo '{ "cniVersion": "0.3.1", "name": "examplenet", "ipam": { "type": "host-local", "ranges": [ {"subnet": "203.0.113.0/24"}, {"subnet": "2001:db8:1::/64"}], "dataDir": "/tmp/cni-example" } }' | CNI_COMMAND=ADD CNI_CONTAINERID=example CNI_NETNS=/dev/null CNI_IFNAME=dummy0 CNI_PATH=. ./host-local | |||
There was a problem hiding this comment.
Looks like you're setting CNI_CONTAINERID and CNI_COMMAND in two places.
squeed
force-pushed
the
host-local-multi
branch
2 times, most recently
from
3901043
to
b155c70
Compare
|
This is ready to merge - can I get a second approver? |
This change allows the host-local allocator to allocate multiple IPs. This is intended to enable dual-stack, but is not limited to only two subnets or separate address families.
…tch-1 build_linux.sh: remove no_openssl
This change allows the host-local allocator to allocate multiple IPs. This is intended to enable dual-stack, but is not limited to only two subnets or separate address families.
Migrated from containernetworking/cni#451.