Skip to content

Commit b556452

Browse files
danwinshipdanwinship
committed
Move remaining pod-namespace operations out of the main process
1 parent 3c843d7 commit b556452

File tree

5 files changed

+195
-89
lines changed

5 files changed

+195
-89
lines changed

‎pkg/network/node/cniserver/cniserver.go

+46-1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ const CNIServerRunDir string = "/var/run/openshift-sdn"
5151
const CNIServerSocketName string = "cni-server.sock"
5252
const CNIServerSocketPath string = CNIServerRunDir + "/" + CNIServerSocketName
5353

54+
// Config file containing MTU, and default full path
55+
const CNIServerConfigFileName string = "config.json"
56+
const CNIServerConfigFilePath string = CNIServerRunDir + "/" + CNIServerConfigFileName
57+
58+
// Server-to-plugin config data
59+
type Config struct {
60+
MTU uint32 `json:"mtu"`
61+
}
62+
5463
// Explicit type for CNI commands the server handles
5564
type CNICommand string
5665

@@ -79,6 +88,8 @@ type PodRequest struct {
7988
SandboxID string
8089
// kernel network namespace path
8190
Netns string
91+
// for an ADD request, the host side of the created veth
92+
HostVeth string
8293
// Channel for returning the operation result to the CNIServer
8394
Result chan *PodResult
8495
}
@@ -99,17 +110,19 @@ type CNIServer struct {
99110
http.Server
100111
requestFunc cniRequestFunc
101112
rundir string
113+
config *Config
102114
}
103115

104116
// Create and return a new CNIServer object which will listen on a socket in the given path
105-
func NewCNIServer(rundir string) *CNIServer {
117+
func NewCNIServer(rundir string, config *Config) *CNIServer {
106118
router := mux.NewRouter()
107119

108120
s := &CNIServer{
109121
Server: http.Server{
110122
Handler: router,
111123
},
112124
rundir: rundir,
125+
config: config,
113126
}
114127
router.NotFoundHandler = http.HandlerFunc(http.NotFound)
115128
router.HandleFunc("/", s.handleCNIRequest).Methods("POST")
@@ -137,6 +150,17 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
137150
return fmt.Errorf("failed to create pod info socket directory: %v", err)
138151
}
139152

153+
// Write config file
154+
config, err := json.Marshal(s.config)
155+
if err != nil {
156+
return fmt.Errorf("could not marshal config data: %v", err)
157+
}
158+
configPath := filepath.Join(s.rundir, CNIServerConfigFileName)
159+
err = ioutil.WriteFile(configPath, config, os.FileMode(0444))
160+
if err != nil {
161+
return fmt.Errorf("could not write config file %q: %v", configPath, err)
162+
}
163+
140164
// On Linux the socket is created with the permissions of the directory
141165
// it is in, so as long as the directory is root-only we can avoid
142166
// racy umask manipulation.
@@ -159,6 +183,22 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
159183
return nil
160184
}
161185

186+
func ReadConfig(configPath string) (*Config, error) {
187+
bytes, err := ioutil.ReadFile(configPath)
188+
if err != nil {
189+
if os.IsNotExist(err) {
190+
return nil, fmt.Errorf("OpenShift SDN network process is not (yet?) available")
191+
} else {
192+
return nil, fmt.Errorf("could not read config file %q: %v", configPath, err)
193+
}
194+
}
195+
var config Config
196+
if err = json.Unmarshal(bytes, &config); err != nil {
197+
return nil, fmt.Errorf("could not parse config file %q: %v", configPath, err)
198+
}
199+
return &config, nil
200+
}
201+
162202
// Split the "CNI_ARGS" environment variable's value into a map. CNI_ARGS
163203
// contains arbitrary key/value pairs separated by ';' and is for runtime or
164204
// plugin specific uses. Kubernetes passes the pod namespace and name in
@@ -206,6 +246,11 @@ func cniRequestToPodRequest(r *http.Request) (*PodRequest, error) {
206246
return nil, fmt.Errorf("missing CNI_NETNS")
207247
}
208248

249+
req.HostVeth, ok = cr.Env["OSDN_HOSTVETH"]
250+
if !ok && req.Command == CNI_ADD {
251+
return nil, fmt.Errorf("missing OSDN_HOSTVETH")
252+
}
253+
209254
cniArgs, err := gatherCNIArgs(cr.Env)
210255
if err != nil {
211256
return nil, err

‎pkg/network/node/cniserver/cniserver_test.go

+20-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestCNIServer(t *testing.T) {
6262
defer os.RemoveAll(tmpDir)
6363
socketPath := filepath.Join(tmpDir, CNIServerSocketName)
6464

65-
s := NewCNIServer(tmpDir)
65+
s := NewCNIServer(tmpDir, &Config{MTU: 1500})
6666
if err := s.Start(serverHandleCNI); err != nil {
6767
t.Fatalf("error starting CNI server: %v", err)
6868
}
@@ -102,6 +102,7 @@ func TestCNIServer(t *testing.T) {
102102
"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
103103
"CNI_NETNS": "/path/to/something",
104104
"CNI_ARGS": "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
105+
"OSDN_HOSTVETH": "vethABC",
105106
},
106107
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
107108
},
@@ -143,6 +144,7 @@ func TestCNIServer(t *testing.T) {
143144
"CNI_COMMAND": string(CNI_ADD),
144145
"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
145146
"CNI_NETNS": "/path/to/something",
147+
"OSDN_HOSTVETH": "vethABC",
146148
},
147149
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
148150
},
@@ -157,6 +159,7 @@ func TestCNIServer(t *testing.T) {
157159
"CNI_COMMAND": string(CNI_ADD),
158160
"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
159161
"CNI_ARGS": "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
162+
"OSDN_HOSTVETH": "vethABC",
160163
},
161164
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
162165
},
@@ -171,12 +174,28 @@ func TestCNIServer(t *testing.T) {
171174
"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
172175
"CNI_NETNS": "/path/to/something",
173176
"CNI_ARGS": "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
177+
"OSDN_HOSTVETH": "vethABC",
174178
},
175179
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
176180
},
177181
result: nil,
178182
errorPrefix: "unexpected or missing CNI_COMMAND",
179183
},
184+
// Missing OSDN_HOSTVETH
185+
{
186+
name: "ARGS4",
187+
request: &CNIRequest{
188+
Env: map[string]string{
189+
"CNI_COMMAND": string(CNI_ADD),
190+
"CNI_CONTAINERID": "adsfadsfasfdasdfasf",
191+
"CNI_NETNS": "/path/to/something",
192+
"CNI_ARGS": "K8S_POD_NAMESPACE=awesome-namespace;K8S_POD_NAME=awesome-name",
193+
},
194+
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
195+
},
196+
result: nil,
197+
errorPrefix: "missing OSDN_HOSTVETH",
198+
},
180199
}
181200

182201
for _, tc := range testcases {

‎pkg/network/node/pod.go

+6-72
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ import (
3535
"github.com/containernetworking/cni/pkg/invoke"
3636
cnitypes "github.com/containernetworking/cni/pkg/types"
3737
cni020 "github.com/containernetworking/cni/pkg/types/020"
38-
cnicurrent "github.com/containernetworking/cni/pkg/types/current"
39-
"github.com/containernetworking/plugins/pkg/ip"
40-
"github.com/containernetworking/plugins/pkg/ipam"
4138
"github.com/containernetworking/plugins/pkg/ns"
4239

4340
"github.com/vishvananda/netlink"
@@ -180,7 +177,7 @@ func (m *podManager) Start(rundir string, localSubnetCIDR string, clusterNetwork
180177

181178
go m.processCNIRequests()
182179

183-
m.cniServer = cniserver.NewCNIServer(rundir)
180+
m.cniServer = cniserver.NewCNIServer(rundir, &cniserver.Config{MTU: m.mtu})
184181
return m.cniServer.Start(m.handleCNIRequest)
185182
}
186183

@@ -384,6 +381,8 @@ func maybeAddMacvlan(pod *kapi.Pod, netns string) error {
384381
}
385382
}
386383

384+
// Note that this use of ns is safe because it doesn't call Do() or WithNetNSPath()
385+
387386
podNs, err := ns.GetNS(netns)
388387
if err != nil {
389388
return fmt.Errorf("could not open netns %q", netns)
@@ -402,17 +401,7 @@ func maybeAddMacvlan(pod *kapi.Pod, netns string) error {
402401
if err != nil {
403402
return fmt.Errorf("failed to create macvlan interface: %v", err)
404403
}
405-
return podNs.Do(func(netns ns.NetNS) error {
406-
l, err := netlink.LinkByName("macvlan0")
407-
if err != nil {
408-
return fmt.Errorf("failed to find macvlan interface: %v", err)
409-
}
410-
err = netlink.LinkSetUp(l)
411-
if err != nil {
412-
return fmt.Errorf("failed to set macvlan interface up: %v", err)
413-
}
414-
return nil
415-
})
404+
return nil
416405
}
417406

418407
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
539528
}
540529
}
541530

542-
var hostVethName string
543-
err = ns.WithNetNSPath(req.Netns, func(hostNS ns.NetNS) error {
544-
hostVeth, contVeth, err := ip.SetupVeth(podInterfaceName, int(m.mtu), hostNS)
545-
if err != nil {
546-
return fmt.Errorf("failed to create container veth: %v", err)
547-
}
548-
// Force a consistent MAC address based on the IP address
549-
if err := ip.SetHWAddrByIP(podInterfaceName, podIP, nil); err != nil {
550-
return fmt.Errorf("failed to set pod interface MAC address: %v", err)
551-
}
552-
// refetch to get hardware address and other properties
553-
tmp, err := net.InterfaceByIndex(contVeth.Index)
554-
if err != nil {
555-
return fmt.Errorf("failed to fetch container veth: %v", err)
556-
}
557-
contVeth = *tmp
558-
559-
// Clear out gateway to prevent ConfigureIface from adding the cluster
560-
// subnet via the gateway
561-
ipamResult.IP4.Gateway = nil
562-
result030, err := cnicurrent.NewResultFromResult(ipamResult)
563-
if err != nil {
564-
return fmt.Errorf("failed to convert IPAM: %v", err)
565-
}
566-
// Add a sandbox interface record which ConfigureInterface expects.
567-
// The only interface we report is the pod interface.
568-
result030.Interfaces = []*cnicurrent.Interface{
569-
{
570-
Name: podInterfaceName,
571-
Mac: contVeth.HardwareAddr.String(),
572-
Sandbox: req.Netns,
573-
},
574-
}
575-
intPtr := 0
576-
result030.IPs[0].Interface = &intPtr
577-
578-
if err = ipam.ConfigureIface(podInterfaceName, result030); err != nil {
579-
return fmt.Errorf("failed to configure container IPAM: %v", err)
580-
}
581-
582-
lo, err := netlink.LinkByName("lo")
583-
if err == nil {
584-
err = netlink.LinkSetUp(lo)
585-
}
586-
if err != nil {
587-
return fmt.Errorf("failed to configure container loopback: %v", err)
588-
}
589-
590-
hostVethName = hostVeth.Name
591-
return nil
592-
})
593-
if err != nil {
594-
return nil, nil, err
595-
}
596-
597531
vnid, err := m.policy.GetVNID(req.PodNamespace)
598532
if err != nil {
599533
return nil, nil, err
@@ -603,11 +537,11 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running
603537
return nil, nil, err
604538
}
605539

606-
ofport, err := m.ovs.SetUpPod(req.SandboxID, hostVethName, podIP, vnid)
540+
ofport, err := m.ovs.SetUpPod(req.SandboxID, req.HostVeth, podIP, vnid)
607541
if err != nil {
608542
return nil, nil, err
609543
}
610-
if err := setupPodBandwidth(m.ovs, pod, hostVethName, req.SandboxID); err != nil {
544+
if err := setupPodBandwidth(m.ovs, pod, req.HostVeth, req.SandboxID); err != nil {
611545
return nil, nil, err
612546
}
613547

0 commit comments

Comments
 (0)