Fix cni config type.

[Random-Liu]

Fix the network type.

The network type is set to a wrong value, hence it stops working.

Signed-off-by: Lantao Liu lantaol@google.com

[Random-Liu]

/cc @jterry75

[nagiesek]

You're testing this on nat? I think this will break sdnbridge. Overlay should still work. Ideally we could remove type from the networkInfo, and remove the create network portion.

[Random-Liu]

@nagiesek Yes. I'm using nat. Are we intentionally using config.Name as the type?

It doesn't look correct to me. config.Name is an arbitrary network name, for example I use a random string containerd-nat as the network name.

Based on https://docs.microsoft.com/en-us/windows-server/networking/technologies/hcn/hcn-json-document-schemas, the type should be a predefined enum, isn't it?

Without this fix, I get Invalid Json File error from Windows with the example, because the network type natNetwork is not valid.

[Random-Liu]

Any updates on this?

I tried to keep nat in the name field, but it doesn't work, because it conflicts with the nat network interface:

{
    "cniVersion": "0.2.0",
    "name": "nat",
    "type": "nat",
    "master": "Ethernet",
    "ipam": {
        "subnet": "192.168.100.0/24",
        "routes": [
            {
                "gateway": "192.168.100.1"
            }
        ]
    },
    "capabilities": {
        "portMappings": true,
        "dns": true
    }
}

After applying this fix, and have the containerd-nat in the type field, it can work now:

$ cat /c/Program\ Files/containerd/cni/conf/0-containerd-nat.conf
{
    "cniVersion": "0.2.0",
    "name": "nat",
    "type": "nat",
    "master": "Ethernet",
    "ipam": {
        "subnet": "192.168.100.0/24",
        "routes": [
            {
                "gateway": "192.168.100.1"
            }
        ]
    },
    "capabilities": {
        "portMappings": true,
        "dns": true
    }
}
$ ipconfig

Windows IP Configuration


Ethernet adapter Ethernet:

   Connection-specific DNS Suffix  . : c.random-liu-gke-dev.internal
   Link-local IPv6 Address . . . . . : fe80::b814:a550:a36d:4ca2%4
   IPv4 Address. . . . . . . . . . . : 10.128.0.18
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Default Gateway . . . . . . . . . : 10.128.0.1

Ethernet adapter vEthernet (nat):

   Connection-specific DNS Suffix  . :
   Link-local IPv6 Address . . . . . : fe80::d1d8:c8fb:8a30:1fc5%9
   IPv4 Address. . . . . . . . . . . : 172.31.128.1
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Default Gateway . . . . . . . . . :

Ethernet adapter vEthernet (containerd-nat):

   Connection-specific DNS Suffix  . :
   Link-local IPv6 Address . . . . . : fe80::ad7d:fdae:c15c:1980%16
   IPv4 Address. . . . . . . . . . . : 192.168.100.1
   Subnet Mask . . . . . . . . . . . : 255.255.255.0
   Default Gateway . . . . . . . . . :

[nagiesek]

The short answer is I can merge this, but there needs to be some sort of switch statement that selects the correct network type for the plugin being used, as this will break the network creation code path for sdnbridge & sdnoverlay. Should be just a short control setting the value of Type : in the network info to be "nat" for nat.exe "l2bridge" for sdnbridge.exe, "overlay" for sdnoverlay.exe

However my intention is to remove the code that conditionally creates the network if it cannot be found, and make it a seperate setup requirement for using these cni plugins. So you could also just change the line here to return an error instead of trying to create the network, and that'd help me out too.

[jterry75]

@nagiesek - I'm not sure I follow. Can you give me a code sample of what you want @Random-Liu to do here?

[Random-Liu]

I worked around this in the test environment by reusing the nat interface in windows containerd/cri@c77f496#diff-b4444f4cc9d6999c0dde9b0a21c41bbeR57

@nagiesek However, it is really not clear what I should put in the name field in the CNI config. Using the network name as the CNI type seems a bit hacky to me. Is there a document about how it should be used?

[nagiesek]

@nagiesek - I'm not sure I follow. Can you give me a code sample of what you want @Random-Liu to do here?

       
	ninfo := &network.NetworkInfo{
		ID:            config.Name,
		Name:          config.Name,
		Type:          network.NetworkType(config.Type),
		Subnets:       subnets,
		InterfaceName: "",
		DNS:           dnsSettings,
	}

        var networkType string
        // in case .exe is appended
       cniType := strings.Split(config.Type, ".exe")[0].ToLower()
        if cniType == "nat" {
              networkType = "nat"
        } else if cniType == "sdnbridge" {
              networkType = "l2bridge"
       }  else if cniType == "sdnoverlay" {
              networkType = "overlay"
      } else {
           logrus.Debugf("Encountered unsupported cni, network type unsupported")
           panic("Unrecognized cni type, unable to create network")
     }
     ninfo.Type = network.NetworkType(networkType)

@nagiesek However, it is really not clear what I should put in the name field in the CNI config. Using the network name as the CNI type seems a bit hacky to me. Is there a document about how it should be used?

Sorry, I really should add some long form documentation here, I'll file an issue on myself.

We're not using the network name as the cni type here. You are right to suspect hackiness though. We use network name to determine the network type. Essentially what this code path does is try to find a network (in a very simplified model: grouping of a vswitch and a subnet) with the name specified in the cni Name field. This is all well and fine if the network specified in the name field has been created (which is what I strongly reccommend), but if the network does not exist we will try to create a network for the user with type set to network name.

If your network is named "nat" that will work as "nat" is a valid type for us, but if your network is named "MyNetwork" the code will try to create a network of type "MyNetwork" which will fail. So putting cni type into the field here will also not work, but from the cni type we can determine the correct network type.

All in all the network creation does not belong in the cni plugins, and I will remove this conditional network creation. My requirement is that the network is created beforehand, as the setup is complicated for the non-nat case.

[nagiesek]

fixed in #54 by removing network creation logic