Add test for isCNIResultForPID

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>

pkg/cninetwork/cni_network.go

@@ -1,6 +1,7 @@
 package cninetwork
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"io"
@@ -10,6 +11,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 
 	"github.com/containerd/containerd"
 	gocni "github.com/containerd/go-cni"
@@ -19,21 +21,31 @@ import (
 const (
 	// 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"
+
 	// NetNSPathFmt gives the path to the a process network namespace, given the pid
 	NetNSPathFmt = "/proc/%d/ns/net"
-	// CNIResultsDir is the directory CNI stores allocated IP for containers
-	CNIResultsDir = "/var/lib/cni/results"
+
+	// CNIDataDir is the directory CNI stores allocated IP for containers
+	CNIDataDir = "/var/run/cni"
+
 	// defaultCNIConfFilename is the vanity filename of default CNI configuration file
 	defaultCNIConfFilename = "10-openfaas.conflist"
+
 	// defaultNetworkName names the "docker-bridge"-like CNI plugin-chain installed when no other CNI configuration is present.
 	// This value appears in iptables comments created by CNI.
 	defaultNetworkName = "openfaas-cni-bridge"
+
 	// defaultBridgeName is the default bridge device name used in the defaultCNIConf
 	defaultBridgeName = "openfaas0"
+
 	// defaultSubnet is the default subnet used in the defaultCNIConf -- this value is set to not collide with common container networking subnets:
 	defaultSubnet = "10.62.0.0/16"
+
+	// defaultIfPrefix is the interface name to be created in the container
+	defaultIfPrefix = "eth"
 )
 
 // defaultCNIConf is a CNI configuration that enables network access to containers (docker-bridge style)
@@ -50,6 +62,7 @@ var defaultCNIConf = fmt.Sprintf(`
         "ipam": {
             "type": "host-local",
             "subnet": "%s",
+            "dataDir": "%s",
             "routes": [
                 { "dst": "0.0.0.0/0" }
             ]
@@ -60,7 +73,7 @@ var defaultCNIConf = fmt.Sprintf(`
       }
     ]
 }
-`, defaultNetworkName, defaultBridgeName, defaultSubnet)
+`, defaultNetworkName, defaultBridgeName, defaultSubnet, CNIDataDir)
 
 // InitNetwork writes configlist file and initializes CNI network
 func InitNetwork() (gocni.CNI, error) {
@@ -75,11 +88,14 @@ func InitNetwork() (gocni.CNI, error) {
 	netConfig := path.Join(CNIConfDir, defaultCNIConfFilename)
 	if err := ioutil.WriteFile(netConfig, []byte(defaultCNIConf), 644); err != nil {
 		return nil, fmt.Errorf("cannot write network config: %s", defaultCNIConfFilename)
-
 	}
+
 	// Initialize CNI library
-	cni, err := gocni.New(gocni.WithPluginConfDir(CNIConfDir),
-		gocni.WithPluginDir([]string{CNIBinDir}))
+	cni, err := gocni.New(
+		gocni.WithPluginConfDir(CNIConfDir),
+		gocni.WithPluginDir([]string{CNIBinDir}),
+		gocni.WithInterfacePrefix(defaultIfPrefix),
+	)
 
 	if err != nil {
 		return nil, fmt.Errorf("error initializing cni: %s", err)
@@ -131,43 +147,61 @@ func DeleteCNINetwork(ctx context.Context, cni gocni.CNI, client *containerd.Cli
 	return errors.Wrapf(containerErr, "Unable to find container: %s, error: %s", name, containerErr)
 }
 
-// GetIPAddress returns the IP address of the created container
-func GetIPAddress(result *gocni.CNIResult, task containerd.Task) (net.IP, error) {
-	// Get the IP of the created interface
-	var ip net.IP
-	for ifName, config := range result.Interfaces {
-		if config.Sandbox == netNamespace(task) {
-			for _, ipConfig := range config.IPConfigs {
-				if ifName != "lo" && ipConfig.IP.To4() != nil {
-					ip = ipConfig.IP
-				}
-			}
+// GetIPAddress returns the IP address from container based on container name and PID
+func GetIPAddress(container string, PID uint32) (string, error) {
+	CNIDir := path.Join(CNIDataDir, defaultNetworkName)
+
+	files, err := ioutil.ReadDir(CNIDir)
+	if err != nil {
+		return "", fmt.Errorf("failed to read CNI dir for container %s: %v", container, err)
+	}
+
+	for _, file := range files {
+		// each fileName is an IP address
+		fileName := file.Name()
+
+		resultsFile := filepath.Join(CNIDir, fileName)
+		found, err := isCNIResultForPID(resultsFile, container, PID)
+		if err != nil {
+			return "", err
+		}
+
+		if found {
+			return fileName, nil
 		}
 	}
-	if ip == nil {
-		return nil, fmt.Errorf("unable to get IP address for: %s", task.ID())
-	}
-	return ip, nil
+
+	return "", fmt.Errorf("unable to get IP address for container: %s", container)
 }
 
-func GetIPfromPID(pid int) (*net.IP, error) {
-	// https://github.com/weaveworks/weave/blob/master/net/netdev.go
+// isCNIResultForPID confirms if the CNI result file contains the
+// process name, PID and interface name
+//
+// Example:
+//
+// /var/run/cni/openfaas-cni-bridge/10.62.0.2
+//
+// nats-621
+// eth1
+func isCNIResultForPID(fileName, container string, PID uint32) (bool, error) {
+	found := false
 
-	peerIDs, err := ConnectedToBridgeVethPeerIds(defaultBridgeName)
+	f, err := os.Open(fileName)
 	if err != nil {
-		return nil, fmt.Errorf("unable to find peers on: %s %s", defaultBridgeName, err)
+		return false, fmt.Errorf("failed to open CNI IP file for %s: %v", fileName, err)
+	}
+	defer f.Close()
+
+	reader := bufio.NewReader(f)
+	processLine, _ := reader.ReadString('\n')
+	if strings.Contains(processLine, fmt.Sprintf("%s-%d", container, PID)) {
+		ethNameLine, _ := reader.ReadString('\n')
+		if strings.Contains(ethNameLine, defaultIfPrefix) {
+			found = true
+		}
 	}
 
-	addrs, addrsErr := GetNetDevsByVethPeerIds(pid, peerIDs)
-	if addrsErr != nil {
-		return nil, fmt.Errorf("unable to find address for veth pair using: %v %s", peerIDs, addrsErr)
-	}
-
-	if len(addrs) > 0 && len(addrs[0].CIDRs) > 0 {
-		return &addrs[0].CIDRs[0].IP, nil
-	}
-
-	return nil, fmt.Errorf("no IP found for function")
+	return found, nil
 }
 
 // CNIGateway returns the gateway for default subnet

pkg/cninetwork/cni_network_test.go

@@ -0,0 +1,63 @@
+package cninetwork
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func Test_isCNIResultForPID_Found(t *testing.T) {
+	body := `nats-621
+eth1`
+	fileName := `10.62.0.2`
+	container := "nats"
+	PID := uint32(621)
+	fullPath := filepath.Join(os.TempDir(), fileName)
+
+	err := ioutil.WriteFile(fullPath, []byte(body), 0700)
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	defer func() {
+		os.Remove(fullPath)
+	}()
+
+	got, err := isCNIResultForPID(fullPath, container, PID)
+
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	want := true
+	if got != want {
+		t.Fatalf("want %v, but got %v", want, got)
+	}
+}
+
+func Test_isCNIResultForPID_NoMatch(t *testing.T) {
+	body := `nats-621
+eth1`
+	fileName := `10.62.0.3`
+	container := "gateway"
+	PID := uint32(621)
+	fullPath := filepath.Join(os.TempDir(), fileName)
+
+	err := ioutil.WriteFile(fullPath, []byte(body), 0700)
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	defer func() {
+		os.Remove(fullPath)
+	}()
+
+	got, err := isCNIResultForPID(fullPath, container, PID)
+
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	want := false
+	if got != want {
+		t.Fatalf("want %v, but got %v", want, got)
+	}
+}

pkg/cninetwork/weave.go

@@ -1,118 +0,0 @@
-// Copyright Weaveworks
-// github.com/weaveworks/weave/net
-
-package cninetwork
-
-import (
-	"fmt"
-	"net"
-	"os"
-
-	"github.com/vishvananda/netlink"
-	"github.com/vishvananda/netns"
-)
-
-type Dev struct {
-	Name  string           `json:"Name,omitempty"`
-	MAC   net.HardwareAddr `json:"MAC,omitempty"`
-	CIDRs []*net.IPNet     `json:"CIDRs,omitempty"`
-}
-
-// ConnectedToBridgeVethPeerIds returns peer indexes of veth links connected to
-// the given bridge. The peer index is used to query from a container netns
-// whether the container is connected to the bridge.
-func ConnectedToBridgeVethPeerIds(bridgeName string) ([]int, error) {
-	var ids []int
-
-	br, err := netlink.LinkByName(bridgeName)
-	if err != nil {
-		return nil, err
-	}
-	links, err := netlink.LinkList()
-	if err != nil {
-		return nil, err
-	}
-
-	for _, link := range links {
-		if _, isveth := link.(*netlink.Veth); isveth && link.Attrs().MasterIndex == br.Attrs().Index {
-			peerID := link.Attrs().ParentIndex
-			if peerID == 0 {
-				// perhaps running on an older kernel where ParentIndex doesn't work.
-				// as fall-back, assume the peers are consecutive
-				peerID = link.Attrs().Index - 1
-			}
-			ids = append(ids, peerID)
-		}
-	}
-
-	return ids, nil
-}
-
-// Lookup the weave interface of a container
-func GetWeaveNetDevs(processID int) ([]Dev, error) {
-	peerIDs, err := ConnectedToBridgeVethPeerIds("weave")
-	if err != nil {
-		return nil, err
-	}
-
-	return GetNetDevsByVethPeerIds(processID, peerIDs)
-}
-
-func GetNetDevsByVethPeerIds(processID int, peerIDs []int) ([]Dev, error) {
-	// Bail out if this container is running in the root namespace
-	netnsRoot, err := netns.GetFromPid(1)
-	if err != nil {
-		return nil, fmt.Errorf("unable to open root namespace: %s", err)
-	}
-	defer netnsRoot.Close()
-	netnsContainer, err := netns.GetFromPid(processID)
-	if err != nil {
-		// Unable to find a namespace for this process - just return nothing
-		if os.IsNotExist(err) {
-			return nil, nil
-		}
-		return nil, fmt.Errorf("unable to open process %d namespace: %s", processID, err)
-	}
-	defer netnsContainer.Close()
-	if netnsRoot.Equal(netnsContainer) {
-		return nil, nil
-	}
-
-	// convert list of peerIDs into a map for faster lookup
-	indexes := make(map[int]struct{})
-	for _, id := range peerIDs {
-		indexes[id] = struct{}{}
-	}
-
-	var netdevs []Dev
-	err = WithNetNS(netnsContainer, func() error {
-		links, err := netlink.LinkList()
-		if err != nil {
-			return err
-		}
-
-		for _, link := range links {
-			if _, found := indexes[link.Attrs().Index]; found {
-				netdev, err := linkToNetDev(link)
-				if err != nil {
-					return err
-				}
-				netdevs = append(netdevs, netdev)
-			}
-		}
-		return nil
-	})
-
-	return netdevs, err
-}
-
-// Get the weave bridge interface.
-// NB: Should be called from the root network namespace.
-func GetBridgeNetDev(bridgeName string) (Dev, error) {
-	link, err := netlink.LinkByName(bridgeName)
-	if err != nil {
-		return Dev{}, err
-	}
-
-	return linkToNetDev(link)
-}

pkg/cninetwork/weave_darwin.go

@@ -1,10 +0,0 @@
-// +build darwin
-
-package cninetwork
-
-import "github.com/vishvananda/netlink"
-
-func linkToNetDev(link netlink.Link) (Dev, error) {
-
-	return Dev{}, nil
-}

pkg/cninetwork/weave_linux.go

@@ -1,19 +0,0 @@
-// +build linux
-
-package cninetwork
-
-import "github.com/vishvananda/netlink"
-
-func linkToNetDev(link netlink.Link) (Dev, error) {
-
-	addrs, err := netlink.AddrList(link, netlink.FAMILY_V4)
-	if err != nil {
-		return Dev{}, err
-	}
-
-	netDev := Dev{Name: link.Attrs().Name, MAC: link.Attrs().HardwareAddr}
-	for _, addr := range addrs {
-		netDev.CIDRs = append(netDev.CIDRs, addr.IPNet)
-	}
-	return netDev, nil
-}

pkg/provider/handlers/deploy.go

@@ -199,17 +199,18 @@ func createTask(ctx context.Context, client *containerd.Client, container contai
 	log.Printf("Container ID: %s\tTask ID %s:\tTask PID: %d\t\n", name, task.ID(), task.Pid())
 
 	labels := map[string]string{}
-	network, err := cninetwork.CreateCNINetwork(ctx, cni, task, labels)
+	_, err := cninetwork.CreateCNINetwork(ctx, cni, task, labels)
 
 	if err != nil {
 		return err
 	}
 
-	ip, err := cninetwork.GetIPAddress(network, task)
+	ip, err := cninetwork.GetIPAddress(name, task.Pid())
 	if err != nil {
 		return err
 	}
-	log.Printf("%s has IP: %s.\n", name, ip.String())
+
+	log.Printf("%s has IP: %s.\n", name, ip)
 
 	_, waitErr := task.Wait(ctx)
 	if waitErr != nil {

pkg/provider/handlers/functions.go

@@ -3,10 +3,11 @@ package handlers
 import (
 	"context"
 	"fmt"
-	"github.com/opencontainers/runtime-spec/specs-go"
 	"log"
 	"strings"
 
+	"github.com/opencontainers/runtime-spec/specs-go"
+
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/openfaas/faasd/pkg/cninetwork"
@@ -106,11 +107,11 @@ func GetFunction(client *containerd.Client, name string) (Function, error) {
 			fn.pid = task.Pid()
 
 			// Get container IP address
-			ip, err := cninetwork.GetIPfromPID(int(task.Pid()))
+			ip, err := cninetwork.GetIPAddress(name, task.Pid())
 			if err != nil {
 				return Function{}, err
 			}
-			fn.IP = ip.String()
+			fn.IP = ip
 		}
 	} else {
 		replicas = 0

pkg/supervisor.go

@@ -193,19 +193,19 @@ func (s *Supervisor) Start(svcs []Service) error {
 		}
 
 		labels := map[string]string{}
-		network, err := cninetwork.CreateCNINetwork(ctx, s.cni, task, labels)
+		_, err = cninetwork.CreateCNINetwork(ctx, s.cni, task, labels)
 		if err != nil {
 			log.Printf("Error creating CNI for %s: %s", svc.Name, err)
 			return err
 		}
 
-		ip, err := cninetwork.GetIPAddress(network, task)
+		ip, err := cninetwork.GetIPAddress(svc.Name, task.Pid())
 		if err != nil {
 			log.Printf("Error getting IP for %s: %s", svc.Name, err)
 			return err
 		}
 
-		log.Printf("%s has IP: %s\n", newContainer.ID(), ip.String())
+		log.Printf("%s has IP: %s\n", newContainer.ID(), ip)
 
 		hosts, err := ioutil.ReadFile("hosts")
 		if err != nil {