Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instead of type shouldn't we have a network field ? #128

Open
asridharan opened this issue Feb 18, 2016 · 9 comments
Open

Instead of type shouldn't we have a network field ? #128

asridharan opened this issue Feb 18, 2016 · 9 comments

Comments

@asridharan
Copy link

Currently the type field defines the type of plugin to be used. Depending on the type field there are multiple fields that are possible. These fields might be relevant to using the plugin defined in the type field. It becomes very confusing to try and relate fields in the networking spec, to plugins defined in the type field. For e.g., if the type is bridge, then the field bridge is relevant and is defined in the bridge.md (which is a documentation for the bridge plugin).

Instead, if we have a field called network which can be a dictionary, and encapsulate each plugin type into this dictionary (possibly as dictionaries themselves), the relationships would be more explicit.

@steveej
Copy link
Member

steveej commented Feb 23, 2016

@asridharan, please make a full example to demonstrate your thoughts. Please include an IPAM plugin in your example to give a complete snapshot of your idea.

@asridharan
Copy link
Author

This is an example of the current network config for the "bridge" plugin:

{
    "name": "mynet",
    "type": "bridge",
    "bridge": "cni0",
    "isGateway": true,
    "ipMasq": true,
    "ipam": {
        "type": "host-local",
        "subnet": "10.22.0.0/16",
        "routes": [
            { "dst": "0.0.0.0/0" }
        ]
    }

I am proposing the type filed be replaced by "network", which in itself be a dictionary:

{
   "name": "mynet",
    "network": {
           "type": "bridge", // This represents the type of network a plugin represents.
           "plugin": {
               "name" : "bridge"  // This is the name of the binary.
               "params" : {
                    "bridge": "cni0"
                } 
          }
    } 
    "isGateway": true,
    "ipMasq": true,
    "ipam": {
        "type": "host-local",
        "subnet": "10.22.0.0/16",
        "routes": [
            { "dst": "0.0.0.0/0" }
        ]
    }
}

The params field becomes plugin specific, and will allow the container run-time to identify and pass the parameters to the plugin.

@steveej
Copy link
Member

steveej commented Feb 25, 2016

Do I get the notion right that this puts ipam and network on the same level, in contrast to the hierarchy we currently have?
It bothers me that isGateway and ipMasq currently stand out. They are not compatible with every plugin/ipam combination anyway, which cries for a more elegant solution.

@asridharan
Copy link
Author

Yes you are right. I want to keep IPAM and network at the same level, since IP networks by nature should be independent of the way the IP addresses are assigned. I do see your point about the isGateWay and ipMasq fields. I would agree that these fields should be move into the "network/plugin" layer as well, and made more plugin specific.

@ramukima
Copy link

ramukima commented Mar 9, 2016

👍 Also, I do not understand what is the intention to have, "routes" under "ipam" specification. Is it necessary for the IPAM system to perform that 'routing' configuration ? Does an IPAM in general know 'how to configure routes' ? I may be missing the intention here. I suppose, having the gateway address specified for the subnet is sufficient and 'routing' could be separated from the core specification for ipam.

@dcbw
Copy link
Member

dcbw commented Mar 9, 2016

@ramukima typically the thing doing the IPAM does handle routing as well, like with DHCP or IPv6 SLAAC. Routing also has to know the specifics of the IP address and subnets assigned to the interface. So they are closely related. There are some cases where independent routing could be done without needing the IPAM information, but that's a parallel case and I'm not sure it's very common.

@ramukima
Copy link

@dcbw surely Routing function has to know about IP address and subnets. However, I have not seen the opposite always true. An IPAM system may notify other systems of IP acquisitions/release activities, but I am not certain that routing can be treated as a sub-function of IPAM. In my opinion, the 'routing' function can grow in need depending on underlying infrastructure topology and there may be a specific logic to perform the routing configuration driven through various routing function plugins.

Just to correlate this to an example, when I ask Openstack to create a network subnet, I do not specify any routing needs (I just indicate dhcp enabled or not). Interface addresses to a VM are allocated when I specify the networks a VM needs to be associated with when I am creating the VM. At this time, the networking agent in openstack surely talks to IPAM to get an IP address from respective network subnets. However, the routes are not written by that IPAM. Once the allocation is done, routes may be written by another component that may actually be using a plugin to drive the routing configuration all through the stack.

Again, I may be understanding the intent here slightly differently but to me, it makes sense to allow for a routing plugin instead of embedding the routing needs into an IPAM plugin.

@qianzhangxa
Copy link
Contributor

@asridharan, in your example below, it seems type and plugin.name are duplicated, maybe we should only have type field?

"network": {
           "type": "bridge", // This represents the type of network a plugin represents.
           "plugin": {
               "name" : "bridge"  // This is the name of the binary.
               "params" : {
                    "bridge": "cni0"
                } 
          }
    } 

@squeed
Copy link
Member

squeed commented Aug 22, 2017

Calling this field "type" instead of "pluginName" is definitely one of the warts in the spec. We might consider renaming it for 1.0.0, if there's enough traction.

However, I don't think we'll otherwise change the format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants