Pass directory rather than socket filename to CNIServer.Start()

Author: danwinship

pkg/network/node/cniserver/cniserver.go

@@ -9,7 +9,7 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path"
+	"path/filepath"
 	"strings"
 
 	"github.com/golang/glog"
@@ -44,9 +44,12 @@ import (
 // removed and re-created with 0700 permissions each time openshift-node is
 // started.
 
-// Default CNIServer unix domain socket path which the OpenShift SDN CNI
-// plugin uses to talk to the CNIServer
-const CNIServerSocketPath string = "/var/run/openshift-sdn/cni-server.sock"
+// Default directory for CNIServer runtime files
+const CNIServerRunDir string = "/var/run/openshift-sdn"
+
+// CNIServer socket name, and default full path
+const CNIServerSocketName string = "cni-server.sock"
+const CNIServerSocketPath string = CNIServerRunDir + "/" + CNIServerSocketName
 
 // Explicit type for CNI commands the server handles
 type CNICommand string
@@ -95,19 +98,18 @@ type cniRequestFunc func(request *PodRequest) ([]byte, error)
 type CNIServer struct {
 	http.Server
 	requestFunc cniRequestFunc
-	path        string
+	rundir      string
 }
 
-// Create and return a new CNIServer object which will listen on the given
-// socket path
-func NewCNIServer(socketPath string) *CNIServer {
+// Create and return a new CNIServer object which will listen on a socket in the given path
+func NewCNIServer(rundir string) *CNIServer {
 	router := mux.NewRouter()
 
 	s := &CNIServer{
 		Server: http.Server{
 			Handler: router,
 		},
-		path: socketPath,
+		rundir: rundir,
 	}
 	router.NotFoundHandler = http.HandlerFunc(http.NotFound)
 	router.HandleFunc("/", s.handleCNIRequest).Methods("POST")
@@ -125,25 +127,25 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
 	s.requestFunc = requestFunc
 
 	// Remove and re-create the socket directory with root-only permissions
-	dirName := path.Dir(s.path)
-	if err := os.RemoveAll(s.path); err != nil && !os.IsNotExist(err) {
+	if err := os.RemoveAll(s.rundir); err != nil && !os.IsNotExist(err) {
 		utilruntime.HandleError(fmt.Errorf("failed to remove old pod info socket: %v", err))
 	}
-	if err := os.RemoveAll(dirName); err != nil && !os.IsNotExist(err) {
+	if err := os.RemoveAll(s.rundir); err != nil && !os.IsNotExist(err) {
 		utilruntime.HandleError(fmt.Errorf("failed to remove contents of socket directory: %v", err))
 	}
-	if err := os.MkdirAll(dirName, 0700); err != nil {
+	if err := os.MkdirAll(s.rundir, 0700); err != nil {
 		return fmt.Errorf("failed to create pod info socket directory: %v", 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.
-	l, err := net.Listen("unix", s.path)
+	socketPath := filepath.Join(s.rundir, CNIServerSocketName)
+	l, err := net.Listen("unix", socketPath)
 	if err != nil {
 		return fmt.Errorf("failed to listen on pod info socket: %v", err)
 	}
-	if err := os.Chmod(s.path, 0600); err != nil {
+	if err := os.Chmod(socketPath, 0600); err != nil {
 		l.Close()
 		return fmt.Errorf("failed to set pod info socket mode: %v", err)
 	}

pkg/network/node/cniserver/cniserver_test.go

@@ -60,17 +60,17 @@ func TestCNIServer(t *testing.T) {
 		t.Fatalf("failed to create temp directory: %v", err)
 	}
 	defer os.RemoveAll(tmpDir)
+	socketPath := filepath.Join(tmpDir, CNIServerSocketName)
 
-	path := filepath.Join(tmpDir, "cni-server.sock")
-	s := NewCNIServer(path)
+	s := NewCNIServer(tmpDir)
 	if err := s.Start(serverHandleCNI); err != nil {
 		t.Fatalf("error starting CNI server: %v", err)
 	}
 
 	client := &http.Client{
 		Transport: &http.Transport{
 			Dial: func(proto, addr string) (net.Conn, error) {
-				return net.Dial("unix", path)
+				return net.Dial("unix", socketPath)
 			},
 		},
 	}

pkg/network/node/node.go

@@ -346,7 +346,7 @@ func (node *OsdnNode) Start() error {
 	}
 
 	glog.V(2).Infof("Starting openshift-sdn pod manager")
-	if err := node.podManager.Start(cniserver.CNIServerSocketPath, node.localSubnetCIDR, node.networkInfo.ClusterNetworks); err != nil {
+	if err := node.podManager.Start(cniserver.CNIServerRunDir, node.localSubnetCIDR, node.networkInfo.ClusterNetworks); err != nil {
 		return err
 	}
 

pkg/network/node/pod.go

@@ -167,7 +167,7 @@ func getIPAMConfig(clusterNetworks []common.ClusterNetwork, localSubnet string)
 }
 
 // Start the CNI server and start processing requests from it
-func (m *podManager) Start(socketPath string, localSubnetCIDR string, clusterNetworks []common.ClusterNetwork) error {
+func (m *podManager) Start(rundir string, localSubnetCIDR string, clusterNetworks []common.ClusterNetwork) error {
 	if m.enableHostports {
 		iptInterface := utiliptables.New(utilexec.New(), utildbus.New(), utiliptables.ProtocolIpv4)
 		m.hostportSyncer = kubehostport.NewHostportSyncer(iptInterface)
@@ -180,7 +180,7 @@ func (m *podManager) Start(socketPath string, localSubnetCIDR string, clusterNet
 
 	go m.processCNIRequests()
 
-	m.cniServer = cniserver.NewCNIServer(socketPath)
+	m.cniServer = cniserver.NewCNIServer(rundir)
 	return m.cniServer.Start(m.handleCNIRequest)
 }
 

pkg/network/node/pod_test.go

@@ -156,7 +156,7 @@ func TestPodManager(t *testing.T) {
 		t.Fatalf("failed to create temp directory: %v", err)
 	}
 	defer os.RemoveAll(tmpDir)
-	socketPath := filepath.Join(tmpDir, "cni-server.sock")
+	socketPath := filepath.Join(tmpDir, cniserver.CNIServerSocketName)
 
 	testcases := map[string]struct {
 		operations []*operation
@@ -318,7 +318,10 @@ func TestPodManager(t *testing.T) {
 		podManager := newDefaultPodManager()
 		podManager.podHandler = podTester
 		_, cidr, _ := net.ParseCIDR("1.2.0.0/16")
-		podManager.Start(socketPath, "1.2.3.0/24", []common.ClusterNetwork{{ClusterCIDR: cidr, HostSubnetLength: 8}})
+		err := podManager.Start(tmpDir, "1.2.3.0/24", []common.ClusterNetwork{{ClusterCIDR: cidr, HostSubnetLength: 8}})
+		if err != nil {
+			t.Fatalf("could not start PodManager: %v", err)
+		}
 
 		// Add pods to our expected pod list before kicking off the
 		// actual pod setup to ensure we don't concurrently access
@@ -408,13 +411,16 @@ func TestDirectPodUpdate(t *testing.T) {
 		t.Fatalf("failed to create temp directory: %v", err)
 	}
 	defer os.RemoveAll(tmpDir)
-	socketPath := filepath.Join(tmpDir, "cni-server.sock")
+	socketPath := filepath.Join(tmpDir, cniserver.CNIServerSocketName)
 
 	podTester := newPodTester(t, "update", socketPath)
 	podManager := newDefaultPodManager()
 	podManager.podHandler = podTester
 	_, cidr, _ := net.ParseCIDR("1.2.0.0/16")
-	podManager.Start(socketPath, "1.2.3.0/24", []common.ClusterNetwork{{ClusterCIDR: cidr, HostSubnetLength: 8}})
+	err = podManager.Start(tmpDir, "1.2.3.0/24", []common.ClusterNetwork{{ClusterCIDR: cidr, HostSubnetLength: 8}})
+	if err != nil {
+		t.Fatalf("could not start PodManager: %v", err)
+	}
 
 	op := &operation{
 		command:   cniserver.CNI_UPDATE,

pkg/network/sdn-cni-plugin/sdn_cni_plugin_test.go

@@ -63,8 +63,8 @@ func TestOpenshiftSdnCNIPlugin(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpDir)
 
-	path := filepath.Join(tmpDir, "cni-server.sock")
-	server := cniserver.NewCNIServer(path)
+	path := filepath.Join(tmpDir, cniserver.CNIServerSocketName)
+	server := cniserver.NewCNIServer(tmpDir)
 	if err := server.Start(serverHandleCNI); err != nil {
 		t.Fatalf("error starting CNI server: %v", err)
 	}