Support CNI_ARGS in static IPAM plugin #165
Conversation
This change is to add CNI_ARGS support in static IPAM plugin. When IP/SUBNET/GATEWAY are given in CNI_ARGS, static IPAM adds these info in addition to config files. To configure ip address only from CNI_ARGS, 'address' field in config is changed to optional from required.
s1061123
force-pushed
the
dev/static-args
branch
from
dc25e50
to
6da1cb7
Compare
plugins/ipam/static/README.md
Outdated
| * `address` (string, required): CIDR notation IP address. | ||
| * `gateway` (string, optional): IP inside of "subnet" to designate as the gateway. | ||
| * `routes` (string, optional): list of routes add to the container namespace. Each route is a dictionary with "dst" and optional "gw" fields. If "gw" is omitted, value of "gateway" will be used. | ||
| * `dns` (string, optional): the dictionary with "nameservers", "domain" and "search". | ||
|
|
||
| ## Supported arguments | ||
| The following [CNI_ARGS](https://github.com/containernetworking/cni/blob/master/SPEC.md#parameters) are supported: |
There was a problem hiding this comment.
Missing new line before this one. Please keep that in sync with rest of document.
plugins/ipam/static/README.md
Outdated
| ## Supported arguments | ||
| The following [CNI_ARGS](https://github.com/containernetworking/cni/blob/master/SPEC.md#parameters) are supported: | ||
|
|
||
| * `IP`: request a specific IP address from a subnet |
plugins/ipam/static/README.md
Outdated
| @@ -38,8 +38,17 @@ static IPAM is very simple IPAM plugin that assigns IPv4 and IPv6 addresses stat | |||
| ## Network configuration reference | |||
|
|
|||
| * `type` (string, required): "static" | |||
| * `addresses` (array, required): an array of arrays of ip address objects: | |||
| * `addresses` (array, optional): an array of arrays of ip address objects: | |||
There was a problem hiding this comment.
That's incorrect even in master - it's an array of ip addresses, not an array of arrays.
|
Seems fine, except it adds a new runtime convention. others might care about this. |
|
@squeed thank you for your comments. Let me clarify that your comment indicate to remove runtime convention, correct? I will remove runtime and update PR. |
s1061123
force-pushed
the
dev/static-args
branch
from
cac6be4
to
5bd342c
Compare
s1061123
force-pushed
the
dev/static-args
branch
from
5bd342c
to
cc3ad26
Compare
|
@squeed I've taken care of your comments. Could you please take a look into it? |
|
@s1061123 thanks for the update, discussed this one during maintainer's meeting. Instead of having two variables for IP and SUBNET, could we instead combine them into just IP that would have a CIDR format eg "1.2.3.4/24"? You can give that to net.ParseCIDR() and you'll get both the IP address itself and the subnet mask out. |
|
@dcbw , OK, I got it. I just align with host-local and there is no additional meaning. So let me change it and update it again. |
s1061123
changed the title
|
@dcbw and other CNI committer, I need to modify a bit to support multiple ip address ("10.2.2.42/24" and "2001:db8::5/64"), hence I add "[WIP]". Once finished. I will remove it. |
s1061123
changed the title
s1061123
force-pushed
the
dev/static-args
branch
from
210a095
to
61c1361
Compare
|
@dcbw , I updated the PR. |
|
|
||
| for i := range n.IPAM.Addresses { | ||
| if n.IPAM.Addresses[i].Address.Contains(gwip) { | ||
| n.IPAM.Addresses[i].Gateway = gwip |
There was a problem hiding this comment.
I would parse GATEWAY before IP, and hten assign the gateways to each Address while parsing the IPs. We should also make sure that the GATEWAY array is the same length as the IP array.
There was a problem hiding this comment.
In some case, there is no gateway corresponding to certain IP address, especially multiple interface case. So I'd like to parse IP first then match gateway if there exists. Is that okey to you?
This change is to add CNI_ARGS support in static IPAM plugin.
When IP/SUBNET/GATEWAY are given in CNI_ARGS, static IPAM adds
these info in addition to config files.
To configure ip address only from CNI_ARGS, 'address' field in config
is changed to optional from required.