Move remaining pod-namespace operations out of the main process

Author: danwinship

pkg/network/node/cniserver/cniserver.go

@@ -51,6 +51,15 @@ const CNIServerRunDir string = "/var/run/openshift-sdn"
 const CNIServerSocketName string = "cni-server.sock"
 const CNIServerSocketPath string = CNIServerRunDir + "/" + CNIServerSocketName
 
+// Config file containing MTU, and default full path
+const CNIServerConfigFileName string = "config.json"
+const CNIServerConfigFilePath string = CNIServerRunDir + "/" + CNIServerConfigFileName
+
+// Server-to-plugin config data
+type Config struct {
+	MTU uint32 `json:"mtu"`
+}
+
 // Explicit type for CNI commands the server handles
 type CNICommand string
 
@@ -79,6 +88,8 @@ type PodRequest struct {
 	SandboxID string
 	// kernel network namespace path
 	Netns string
+	// for an ADD request, the host side of the created veth
+	HostVeth string
 	// Channel for returning the operation result to the CNIServer
 	Result chan *PodResult
 }
@@ -99,17 +110,19 @@ type CNIServer struct {
 	http.Server
 	requestFunc cniRequestFunc
 	rundir      string
+	config      *Config
 }
 
 // Create and return a new CNIServer object which will listen on a socket in the given path
-func NewCNIServer(rundir string) *CNIServer {
+func NewCNIServer(rundir string, config *Config) *CNIServer {
 	router := mux.NewRouter()
 
 	s := &CNIServer{
 		Server: http.Server{
 			Handler: router,
 		},
 		rundir: rundir,
+		config: config,
 	}
 	router.NotFoundHandler = http.HandlerFunc(http.NotFound)
 	router.HandleFunc("/", s.handleCNIRequest).Methods("POST")
@@ -137,6 +150,17 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
 		return fmt.Errorf("failed to create pod info socket directory: %v", err)
 	}
 
+	// Write config file
+	config, err := json.Marshal(s.config)
+	if err != nil {
+		return fmt.Errorf("could not marshal config data: %v", err)
+	}
+	configPath := filepath.Join(s.rundir, CNIServerConfigFileName)
+	err = ioutil.WriteFile(configPath, config, os.FileMode(0444))
+	if err != nil {
+		return fmt.Errorf("could not write config file %q: %v", configPath, err)
+	}
+
 	// On Linux the socket is created with the permissions of the directory
 	// it is in, so as long as the directory is root-only we can avoid
 	// racy umask manipulation.
@@ -159,6 +183,22 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
 	return nil
 }
 
+func ReadConfig(configPath string) (*Config, error) {
+	bytes, err := ioutil.ReadFile(configPath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, fmt.Errorf("OpenShift SDN network process is not (yet?) available")
+		} else {
+			return nil, fmt.Errorf("could not read config file %q: %v", configPath, err)
+		}
+	}
+	var config Config
+	if err = json.Unmarshal(bytes, &config); err != nil {
+		return nil, fmt.Errorf("could not parse config file %q: %v", configPath, err)
+	}
+	return &config, nil
+}
+
 // Split the "CNI_ARGS" environment variable's value into a map.  CNI_ARGS
 // contains arbitrary key/value pairs separated by ';' and is for runtime or
 // plugin specific uses.  Kubernetes passes the pod namespace and name in
@@ -206,6 +246,11 @@ func cniRequestToPodRequest(r *http.Request) (*PodRequest, error) {
 		return nil, fmt.Errorf("missing CNI_NETNS")
 	}
 
+	req.HostVeth, ok = cr.Env["OSDN_HOSTVETH"]
+	if !ok && req.Command == CNI_ADD {
+		return nil, fmt.Errorf("missing OSDN_HOSTVETH")
+	}
+
 	cniArgs, err := gatherCNIArgs(cr.Env)
 	if err != nil {
 		return nil, err

pkg/network/node/cniserver/cniserver_test.go

@@ -62,7 +62,7 @@ func TestCNIServer(t *testing.T) {
 	defer os.RemoveAll(tmpDir)
 	socketPath := filepath.Join(tmpDir, CNIServerSocketName)
 
-	s := NewCNIServer(tmpDir)
+	s := NewCNIServer(tmpDir, &Config{MTU: 1500})
 	if err := s.Start(serverHandleCNI); err != nil {
 		t.Fatalf("error starting CNI server: %v", err)
 	}
@@ -102,6 +102,7 @@ func TestCNIServer(t *testing.T) {
 					"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
 					"CNI_NETNS":       "/path/to/something",
 					"CNI_ARGS":        "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
+					"OSDN_HOSTVETH":   "vethABC",
 				},
 				Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
 			},
@@ -143,6 +144,7 @@ func TestCNIServer(t *testing.T) {
 					"CNI_COMMAND":     string(CNI_ADD),
 					"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
 					"CNI_NETNS":       "/path/to/something",
+					"OSDN_HOSTVETH":   "vethABC",
 				},
 				Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
 			},
@@ -157,6 +159,7 @@ func TestCNIServer(t *testing.T) {
 					"CNI_COMMAND":     string(CNI_ADD),
 					"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
 					"CNI_ARGS":        "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
+					"OSDN_HOSTVETH":   "vethABC",
 				},
 				Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
 			},
@@ -171,12 +174,28 @@ func TestCNIServer(t *testing.T) {
 					"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
 					"CNI_NETNS":       "/path/to/something",
 					"CNI_ARGS":        "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
+					"OSDN_HOSTVETH":   "vethABC",
 				},
 				Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
 			},
 			result:      nil,
 			errorPrefix: "unexpected or missing CNI_COMMAND",
 		},
+		// Missing OSDN_HOSTVETH
+		{
+			name: "ARGS4",
+			request: &CNIRequest{
+				Env: map[string]string{
+					"CNI_COMMAND":     string(CNI_ADD),
+					"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
+					"CNI_NETNS":       "/path/to/something",
+					"CNI_ARGS":        "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
+				},
+				Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
+			},
+			result:      nil,
+			errorPrefix: "missing OSDN_HOSTVETH",
+		},
 	}
 
 	for _, tc := range testcases {

pkg/network/node/pod.go

@@ -35,9 +35,6 @@ import (
 	"github.com/containernetworking/cni/pkg/invoke"
 	cnitypes "github.com/containernetworking/cni/pkg/types"
 	cni020 "github.com/containernetworking/cni/pkg/types/020"
-	cnicurrent "github.com/containernetworking/cni/pkg/types/current"
-	"github.com/containernetworking/plugins/pkg/ip"
-	"github.com/containernetworking/plugins/pkg/ipam"
 	"github.com/containernetworking/plugins/pkg/ns"
 
 	"github.com/vishvananda/netlink"
@@ -180,7 +177,7 @@ func (m *podManager) Start(rundir string, localSubnetCIDR string, clusterNetwork
 
 	go m.processCNIRequests()
 
-	m.cniServer = cniserver.NewCNIServer(rundir)
+	m.cniServer = cniserver.NewCNIServer(rundir, &cniserver.Config{MTU: m.mtu})
 	return m.cniServer.Start(m.handleCNIRequest)
 }
 
@@ -384,6 +381,8 @@ func maybeAddMacvlan(pod *kapi.Pod, netns string) error {
 		}
 	}
 
+	// Note that this use of ns is safe because it doesn't call Do() or WithNetNSPath()
+
 	podNs, err := ns.GetNS(netns)
 	if err != nil {
 		return fmt.Errorf("could not open netns %q", netns)
@@ -402,17 +401,7 @@ func maybeAddMacvlan(pod *kapi.Pod, netns string) error {
 	if err != nil {
 		return fmt.Errorf("failed to create macvlan interface: %v", err)
 	}
-	return podNs.Do(func(netns ns.NetNS) error {
-		l, err := netlink.LinkByName("macvlan0")
-		if err != nil {
-			return fmt.Errorf("failed to find macvlan interface: %v", err)
-		}
-		err = netlink.LinkSetUp(l)
-		if err != nil {
-			return fmt.Errorf("failed to set macvlan interface up: %v", err)
-		}
-		return nil
-	})
+	return nil
 }
 
 func createIPAMArgs(netnsPath string, action cniserver.CNICommand, id string) *invoke.Args {
@@ -539,61 +528,6 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running
 		}
 	}
 
-	var hostVethName string
-	err = ns.WithNetNSPath(req.Netns, func(hostNS ns.NetNS) error {
-		hostVeth, contVeth, err := ip.SetupVeth(podInterfaceName, int(m.mtu), hostNS)
-		if err != nil {
-			return fmt.Errorf("failed to create container veth: %v", err)
-		}
-		// Force a consistent MAC address based on the IP address
-		if err := ip.SetHWAddrByIP(podInterfaceName, podIP, nil); err != nil {
-			return fmt.Errorf("failed to set pod interface MAC address: %v", err)
-		}
-		// refetch to get hardware address and other properties
-		tmp, err := net.InterfaceByIndex(contVeth.Index)
-		if err != nil {
-			return fmt.Errorf("failed to fetch container veth: %v", err)
-		}
-		contVeth = *tmp
-
-		// Clear out gateway to prevent ConfigureIface from adding the cluster
-		// subnet via the gateway
-		ipamResult.IP4.Gateway = nil
-		result030, err := cnicurrent.NewResultFromResult(ipamResult)
-		if err != nil {
-			return fmt.Errorf("failed to convert IPAM: %v", err)
-		}
-		// Add a sandbox interface record which ConfigureInterface expects.
-		// The only interface we report is the pod interface.
-		result030.Interfaces = []*cnicurrent.Interface{
-			{
-				Name:    podInterfaceName,
-				Mac:     contVeth.HardwareAddr.String(),
-				Sandbox: req.Netns,
-			},
-		}
-		intPtr := 0
-		result030.IPs[0].Interface = &intPtr
-
-		if err = ipam.ConfigureIface(podInterfaceName, result030); err != nil {
-			return fmt.Errorf("failed to configure container IPAM: %v", err)
-		}
-
-		lo, err := netlink.LinkByName("lo")
-		if err == nil {
-			err = netlink.LinkSetUp(lo)
-		}
-		if err != nil {
-			return fmt.Errorf("failed to configure container loopback: %v", err)
-		}
-
-		hostVethName = hostVeth.Name
-		return nil
-	})
-	if err != nil {
-		return nil, nil, err
-	}
-
 	vnid, err := m.policy.GetVNID(req.PodNamespace)
 	if err != nil {
 		return nil, nil, err
@@ -603,11 +537,11 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running
 		return nil, nil, err
 	}
 
-	ofport, err := m.ovs.SetUpPod(req.SandboxID, hostVethName, podIP, vnid)
+	ofport, err := m.ovs.SetUpPod(req.SandboxID, req.HostVeth, podIP, vnid)
 	if err != nil {
 		return nil, nil, err
 	}
-	if err := setupPodBandwidth(m.ovs, pod, hostVethName, req.SandboxID); err != nil {
+	if err := setupPodBandwidth(m.ovs, pod, req.HostVeth, req.SandboxID); err != nil {
 		return nil, nil, err
 	}