Merge pull request #349 from luxas/containerd_cni

Simplify the CNI code by vendoring github.com/containerd/go-cni

go.mod

@@ -12,9 +12,10 @@ require (
 	github.com/containerd/containerd v1.3.0-beta.1
 	github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02 // indirect
 	github.com/containerd/fifo v0.0.0-20190816180239-bda0ff6ed73c // indirect
+	github.com/containerd/go-cni v0.0.0-20190813230227-49fbd9b210f3
 	github.com/containerd/ttrpc v0.0.0-20190613183316-1fb3814edf44 // indirect
 	github.com/containerd/typeurl v0.0.0-20190515163108-7312978f2987 // indirect
-	github.com/containernetworking/cni v0.7.1
+	github.com/containernetworking/cni v0.7.1 // indirect
 	github.com/containers/image v2.0.0+incompatible
 	github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect
 	github.com/docker/distribution v2.7.1+incompatible // indirect

go.sum

@@ -39,6 +39,8 @@ github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02 h1:tN9D97v5A
 github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=
 github.com/containerd/fifo v0.0.0-20190816180239-bda0ff6ed73c h1:KFbqHhDeaHM7IfFtXHfUHMDaUStpM2YwBR+iJCIOsKk=
 github.com/containerd/fifo v0.0.0-20190816180239-bda0ff6ed73c/go.mod h1:ODA38xgv3Kuk8dQz2ZQXpnv/UZZUHUCL7pnLehbXgQI=
+github.com/containerd/go-cni v0.0.0-20190813230227-49fbd9b210f3 h1:owkX+hC6Inv1XUep/pAjF7qJpnZWjbtETw5r1DVYFPo=
+github.com/containerd/go-cni v0.0.0-20190813230227-49fbd9b210f3/go.mod h1:2wlRxCQdiBY+OcjNg5x8kI+5mEL1fGt25L4IzQHYJsM=
 github.com/containerd/ttrpc v0.0.0-20190613183316-1fb3814edf44 h1:vG5QXCUakUhR2CRI44aD3joCWcvb5mfZRxcwVqBVGeU=
 github.com/containerd/ttrpc v0.0.0-20190613183316-1fb3814edf44/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o=
 github.com/containerd/typeurl v0.0.0-20190515163108-7312978f2987 h1:Qaux2AYCIF3t3gxqjFHDJbxWPhMphgBruE8ygIRHtBA=
@@ -286,6 +288,7 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
 github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
 github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
 github.com/stretchr/testify v0.0.0-20151208002404-e3a8ff8ce365/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=

pkg/network/cni/cni.go

@@ -3,174 +3,57 @@ package cni
 import (
 	"context"
 	"fmt"
-	"sort"
-	"strings"
 	"sync"
 
-	"github.com/containernetworking/cni/libcni"
-	cnitypes "github.com/containernetworking/cni/pkg/types"
-	cnicurrentapi "github.com/containernetworking/cni/pkg/types/current"
+	gocni "github.com/containerd/go-cni"
 	log "github.com/sirupsen/logrus"
 	"github.com/weaveworks/ignite/pkg/network"
 	"github.com/weaveworks/ignite/pkg/runtime"
 )
 
-// Disclaimer: This package is heavily influenced by
-// https://github.com/kubernetes/kubernetes/blob/v1.15.0/pkg/kubelet/dockershim/network/cni/cni.go#L49
+const (
+	// TODO: CNIBinDir and CNIConfDir should maybe be globally configurable?
 
-type cniNetworkPlugin struct {
-	sync.RWMutex
-
-	loNetwork      *cniNetwork
-	defaultNetwork *cniNetwork
+	// CNIBinDir describes the directory where the CNI binaries are stored
+	CNIBinDir = "/opt/cni/bin"
+	// CNIConfDir describes the directory where the CNI plugin's configuration is stored
+	CNIConfDir = "/etc/cni/net.d"
+)
 
+type cniNetworkPlugin struct {
+	cni     gocni.CNI
 	runtime runtime.Interface
-	confDir string
-	binDirs []string
-}
-
-type cniNetwork struct {
-	name          string
-	NetworkConfig *libcni.NetworkConfigList
-	CNIConfig     libcni.CNI
+	once    *sync.Once
 }
 
 func GetCNINetworkPlugin(runtime runtime.Interface) (network.Plugin, error) {
 	binDirs := []string{CNIBinDir}
-	plugin := &cniNetworkPlugin{
-		runtime:        runtime,
-		defaultNetwork: nil,
-		loNetwork:      getLoNetwork(binDirs),
-		confDir:        CNIConfDir,
-		binDirs:        binDirs,
-	}
-
-	return plugin, nil
-}
-
-func getLoNetwork(binDirs []string) *cniNetwork {
-	loConfig, err := libcni.ConfListFromBytes([]byte(loopbackCNIConfig))
+	cniInstance, err := gocni.New(gocni.WithMinNetworkCount(2),
+		gocni.WithPluginConfDir(CNIConfDir),
+		gocni.WithPluginDir(binDirs))
 	if err != nil {
-		// The hardcoded config above should always be valid and unit tests will
-		// catch this
-		panic(err)
-	}
-
-	return &cniNetwork{
-		name:          "lo",
-		NetworkConfig: loConfig,
-		CNIConfig:     &libcni.CNIConfig{Path: binDirs},
-	}
-}
-
-func getDefaultCNINetwork(confDir string, binDirs []string) (*cniNetwork, error) {
-	files, err := libcni.ConfFiles(confDir, []string{".conf", ".conflist", ".json"})
-	switch {
-	case err != nil:
 		return nil, err
-	case len(files) == 0:
-		return nil, fmt.Errorf("no networks found in %s", confDir)
-	}
-
-	sort.Strings(files)
-	for _, confFile := range files {
-		var confList *libcni.NetworkConfigList
-		if strings.HasSuffix(confFile, ".conflist") {
-			confList, err = libcni.ConfListFromFile(confFile)
-			if err != nil {
-				log.Infof("Error loading CNI config list file %s: %v", confFile, err)
-				continue
-			}
-		} else {
-			conf, err := libcni.ConfFromFile(confFile)
-			if err != nil {
-				log.Infof("Error loading CNI config file %s: %v", confFile, err)
-				continue
-			}
-
-			// Ensure the config has a "type" so we know what plugin to run.
-			// Also catches the case where somebody put a conflist into a conf file.
-			if conf.Network.Type == "" {
-				log.Infof("Error loading CNI config file %s: no 'type'; perhaps this is a .conflist?", confFile)
-				continue
-			}
-
-			confList, err = libcni.ConfListFromConf(conf)
-			if err != nil {
-				log.Infof("Error converting CNI config file %s to list: %v", confFile, err)
-				continue
-			}
-		}
-
-		if len(confList.Plugins) == 0 {
-			log.Infof("CNI config list %s has no networks, skipping", confFile)
-			continue
-		}
-
-		log.Infof("Using CNI configuration file %s", confFile)
-
-		network := &cniNetwork{
-			name:          confList.Name,
-			NetworkConfig: confList,
-			CNIConfig:     &libcni.CNIConfig{Path: binDirs},
-		}
-
-		return network, nil
 	}
 
-	return nil, fmt.Errorf("no valid networks found in %s", confDir)
-}
-
-func (plugin *cniNetworkPlugin) syncNetworkConfig() error {
-	network, err := getDefaultCNINetwork(plugin.confDir, plugin.binDirs)
-	if err != nil {
-		return fmt.Errorf("unable to get default CNI network: %v", err)
-	}
-
-	plugin.setDefaultNetwork(network)
-	return nil
-}
-
-func (plugin *cniNetworkPlugin) getDefaultNetwork() *cniNetwork {
-	plugin.RLock()
-	defer plugin.RUnlock()
-	return plugin.defaultNetwork
-}
-
-func (plugin *cniNetworkPlugin) setDefaultNetwork(n *cniNetwork) {
-	plugin.Lock()
-	defer plugin.Unlock()
-	plugin.defaultNetwork = n
-}
-
-func (plugin *cniNetworkPlugin) checkInitialized() error {
-	if plugin.getDefaultNetwork() == nil {
-		// Sync the network configuration if the plugin is not initialized
-		if err := plugin.syncNetworkConfig(); err != nil {
-			return err
-		}
-	}
-
-	return nil
+	return &cniNetworkPlugin{
+		runtime: runtime,
+		cni:     cniInstance,
+		once:    &sync.Once{},
+	}, nil
 }
 
 func (plugin *cniNetworkPlugin) Name() network.PluginName {
 	return network.PluginCNI
 }
 
-func (plugin *cniNetworkPlugin) Status() error {
-	// Can't set up pods if we don't have any CNI network configs yet
-	return plugin.checkInitialized()
-}
-
 func (plugin *cniNetworkPlugin) PrepareContainerSpec(container *runtime.ContainerConfig) error {
 	// No need for the container runtime to set up networking, as this plugin will do it
 	container.NetworkMode = "none"
 	return nil
 }
 
 func (plugin *cniNetworkPlugin) SetupContainerNetwork(containerid string) (*network.Result, error) {
-	if err := plugin.checkInitialized(); err != nil {
+	if err := plugin.initialize(); err != nil {
 		return nil, err
 	}
 
@@ -179,34 +62,39 @@ func (plugin *cniNetworkPlugin) SetupContainerNetwork(containerid string) (*netw
 		return nil, fmt.Errorf("CNI failed to retrieve network namespace path: %v", err)
 	}
 
-	if _, err = plugin.addToNetwork(plugin.loNetwork, containerid, netnsPath); err != nil {
-		return nil, err
-	}
-
-	genericResult, err := plugin.addToNetwork(plugin.getDefaultNetwork(), containerid, netnsPath)
-	if err != nil {
-		return nil, err
-	}
-	result, err := cnicurrentapi.NewResultFromResult(genericResult)
+	result, err := plugin.cni.Setup(context.Background(), containerid, netnsPath)
 	if err != nil {
+		log.Errorf("failed to setup network for namespace %q: %v", containerid, err)
 		return nil, err
 	}
+
 	return cniToIgniteResult(result), nil
 }
 
-func cniToIgniteResult(r *cnicurrentapi.Result) *network.Result {
+func (plugin *cniNetworkPlugin) initialize() (err error) {
+	plugin.once.Do(func() {
+		if err = plugin.cni.Load(gocni.WithLoNetwork, gocni.WithDefaultConf); err != nil {
+			log.Errorf("failed to load cni configuration: %v", err)
+		}
+	})
+	return
+}
+
+func cniToIgniteResult(r *gocni.CNIResult) *network.Result {
 	result := &network.Result{}
-	for _, ip := range r.IPs {
-		result.Addresses = append(result.Addresses, network.Address{
-			IPNet:   ip.Address,
-			Gateway: ip.Gateway,
-		})
+	for _, iface := range r.Interfaces {
+		for _, ip := range iface.IPConfigs {
+			result.Addresses = append(result.Addresses, network.Address{
+				IP:      ip.IP,
+				Gateway: ip.Gateway,
+			})
+		}
 	}
 	return result
 }
 
 func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerid string) error {
-	if err := plugin.checkInitialized(); err != nil {
+	if err := plugin.initialize(); err != nil {
 		return err
 	}
 
@@ -216,51 +104,5 @@ func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerid string) error
 		log.Infof("CNI failed to retrieve network namespace path: %v", err)
 	}
 
-	return plugin.deleteFromNetwork(plugin.getDefaultNetwork(), containerid, netnsPath, nil)
-}
-
-func (plugin *cniNetworkPlugin) addToNetwork(network *cniNetwork, containerID string, netnsPath string) (cnitypes.Result, error) {
-	rt, err := plugin.buildCNIRuntimeConf(containerID, netnsPath)
-	if err != nil {
-		return nil, fmt.Errorf("Error adding network when building cni runtime conf: %v", err)
-	}
-
-	netConf, cniNet := network.NetworkConfig, network.CNIConfig
-	log.Debugf("Adding %s to network %s/%s netns %q", containerID, netConf.Plugins[0].Network.Type, netConf.Name, netnsPath)
-	res, err := cniNet.AddNetworkList(context.Background(), netConf, rt)
-	if err != nil {
-		return nil, fmt.Errorf("Error adding %s to network %s/%s: %v", containerID, netConf.Plugins[0].Network.Type, netConf.Name, err)
-	}
-
-	log.Debugf("Added %s to network %s: %v", containerID, netConf.Name, res)
-	return res, nil
-}
-
-func (plugin *cniNetworkPlugin) deleteFromNetwork(network *cniNetwork, containerID string, netnsPath string, annotations map[string]string) error {
-	rt, err := plugin.buildCNIRuntimeConf(containerID, netnsPath)
-	if err != nil {
-		return fmt.Errorf("Error deleting network when building cni runtime conf: %v", err)
-	}
-
-	netConf, cniNet := network.NetworkConfig, network.CNIConfig
-	log.Debugf("Deleting %s from network %s/%s netns %q", containerID, netConf.Plugins[0].Network.Type, netConf.Name, netnsPath)
-	err = cniNet.DelNetworkList(context.Background(), netConf, rt)
-	// The pod may not get deleted successfully at the first time.
-	// Ignore "no such file or directory" error in case the network has already been deleted in previous attempts.
-	if err != nil && !strings.Contains(err.Error(), "no such file or directory") {
-		return fmt.Errorf("Error deleting %s from network %s/%s: %v", containerID, netConf.Plugins[0].Network.Type, netConf.Name, err)
-	}
-
-	log.Debugf("Deleted %s from network %s/%s", containerID, netConf.Plugins[0].Network.Type, netConf.Name)
-	return nil
-}
-
-func (plugin *cniNetworkPlugin) buildCNIRuntimeConf(containerID string, netnsPath string) (*libcni.RuntimeConf, error) {
-	rt := &libcni.RuntimeConf{
-		ContainerID: containerID,
-		NetNS:       netnsPath,
-		IfName:      DefaultInterfaceName,
-	}
-
-	return rt, nil
+	return plugin.cni.Remove(context.Background(), containerid, netnsPath)
 }

pkg/network/docker/docker.go

@@ -35,10 +35,7 @@ func (plugin *dockerNetworkPlugin) SetupContainerNetwork(containerID string) (*n
 	return &network.Result{
 		Addresses: []network.Address{
 			{
-				IPNet: net.IPNet{
-					IP:   result.IPAddress,
-					Mask: net.IPv4Mask(255, 255, 0, 0),
-				},
+				IP: result.IPAddress,
 				// TODO: Make this auto-detect if the gateway is not using the standard setup
 				Gateway: net.IPv4(result.IPAddress[0], result.IPAddress[1], result.IPAddress[2], 1),
 			},
@@ -50,8 +47,3 @@ func (*dockerNetworkPlugin) RemoveContainerNetwork(_ string) error {
 	// no-op for docker, this is handled automatically
 	return nil
 }
-
-func (*dockerNetworkPlugin) Status() error {
-	// no-op, we assume the bridge to be working :)
-	return nil
-}

pkg/network/types.go

@@ -21,17 +21,14 @@ type Plugin interface {
 
 	// RemoveContainerNetwork is the method called before a container using the network plugin can be deleted
 	RemoveContainerNetwork(containerID string) error
-
-	// Status returns error if the network plugin is in error state
-	Status() error
 }
 
 type Result struct {
 	Addresses []Address
 }
 
 type Address struct {
-	net.IPNet
+	IP      net.IP
 	Gateway net.IP
 }