Skip to content

Commit 22f3e02

Browse files
dcbwdcbw
committed
sdn: clean up ports and QoS by matching on sandbox ID
Store the sandbox ID in the OVSDB as an external-id and use that to find things we need to clean up. Previously these were leaked if the network namespace was removed underneath kubernetes.
1 parent 99cfb31 commit 22f3e02

File tree

5 files changed

+83
-42
lines changed

5 files changed

+83
-42
lines changed

‎pkg/network/node/ovscontroller.go

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const (
3434
Vxlan0 = "vxlan0"
3535

3636
// rule versioning; increment each time flow rules change
37-
ruleVersion = 5
37+
ruleVersion = 6
3838

3939
ruleVersionTable = 253
4040
)
@@ -228,8 +228,8 @@ func (oc *ovsController) NewTransaction() ovs.Transaction {
228228
return oc.ovs.NewTransaction()
229229
}
230230

231-
func (oc *ovsController) ensureOvsPort(hostVeth string) (int, error) {
232-
return oc.ovs.AddPort(hostVeth, -1)
231+
func (oc *ovsController) ensureOvsPort(hostVeth, sandboxID string) (int, error) {
232+
return oc.ovs.AddPort(hostVeth, -1, "external-ids=sandbox="+sandboxID)
233233
}
234234

235235
func (oc *ovsController) setupPodFlows(ofport int, podIP, podMAC, note string, vnid uint32) error {
@@ -283,33 +283,54 @@ func (oc *ovsController) SetUpPod(hostVeth, podIP, podMAC, sandboxID string, vni
283283
if err != nil {
284284
return -1, err
285285
}
286-
ofport, err := oc.ensureOvsPort(hostVeth)
286+
ofport, err := oc.ensureOvsPort(hostVeth, sandboxID)
287287
if err != nil {
288288
return -1, err
289289
}
290290
return ofport, oc.setupPodFlows(ofport, podIP, podMAC, note, vnid)
291291
}
292292

293-
func (oc *ovsController) SetPodBandwidth(hostVeth string, ingressBPS, egressBPS int64) error {
294-
// note pod ingress == OVS egress and vice versa
293+
// Returned list can also be used for port names
294+
func (oc *ovsController) getInterfacesForSandbox(sandboxID string) ([]string, error) {
295+
return oc.ovs.Find("interface", "name", "external-ids:sandbox="+sandboxID)
296+
}
297+
298+
func (oc *ovsController) ClearPodBandwidth(portList []string, sandboxID string) error {
299+
// Clear the QoS for any ports of this sandbox
300+
for _, port := range portList {
301+
if err := oc.ovs.Clear("port", port, "qos"); err != nil {
302+
return err
303+
}
304+
}
295305

296-
qos, err := oc.ovs.Get("port", hostVeth, "qos")
306+
// Now that the QoS is unused remove it
307+
qosList, err := oc.ovs.Find("qos", "_uuid", "external-ids:sandbox="+sandboxID)
297308
if err != nil {
298309
return err
299310
}
300-
if qos != "[]" {
301-
err = oc.ovs.Clear("port", hostVeth, "qos")
302-
if err != nil {
303-
return err
304-
}
305-
err = oc.ovs.Destroy("qos", qos)
306-
if err != nil {
311+
for _, qos := range qosList {
312+
if err := oc.ovs.Destroy("qos", qos); err != nil {
307313
return err
308314
}
309315
}
310316

317+
return nil
318+
}
319+
320+
func (oc *ovsController) SetPodBandwidth(hostVeth, sandboxID string, ingressBPS, egressBPS int64) error {
321+
// note pod ingress == OVS egress and vice versa
322+
323+
ports, err := oc.getInterfacesForSandbox(sandboxID)
324+
if err != nil {
325+
return err
326+
}
327+
328+
if err := oc.ClearPodBandwidth(ports, sandboxID); err != nil {
329+
return err
330+
}
331+
311332
if ingressBPS > 0 {
312-
qos, err := oc.ovs.Create("qos", "type=linux-htb", fmt.Sprintf("other-config:max-rate=%d", ingressBPS))
333+
qos, err := oc.ovs.Create("qos", "type=linux-htb", fmt.Sprintf("other-config:max-rate=%d", ingressBPS), "external-ids=sandbox="+sandboxID)
313334
if err != nil {
314335
return err
315336
}
@@ -387,7 +408,7 @@ func (oc *ovsController) UpdatePod(sandboxID string, vnid uint32) error {
387408
return oc.setupPodFlows(ofport, podIP, podMAC, note, vnid)
388409
}
389410

390-
func (oc *ovsController) TearDownPod(hostVeth, podIP, sandboxID string) error {
411+
func (oc *ovsController) TearDownPod(podIP, sandboxID string) error {
391412
if podIP == "" {
392413
flows, err := oc.ovs.DumpFlows()
393414
if err != nil {
@@ -401,17 +422,26 @@ func (oc *ovsController) TearDownPod(hostVeth, podIP, sandboxID string) error {
401422
}
402423
}
403424

404-
err := oc.cleanupPodFlows(podIP)
425+
if err := oc.cleanupPodFlows(podIP); err != nil {
426+
return err
427+
}
428+
429+
ports, err := oc.getInterfacesForSandbox(sandboxID)
405430
if err != nil {
406431
return err
407432
}
408433

409-
// veth may have already been destroyed if the container was deleted out-of-band
410-
if hostVeth != "" {
411-
_ = oc.SetPodBandwidth(hostVeth, -1, -1)
412-
err = oc.ovs.DeletePort(hostVeth)
434+
if err := oc.ClearPodBandwidth(ports, sandboxID); err != nil {
435+
return err
436+
}
437+
438+
for _, port := range ports {
439+
if err := oc.ovs.DeletePort(port); err != nil {
440+
return err
441+
}
413442
}
414-
return err
443+
444+
return nil
415445
}
416446

417447
func policyNames(policies []networkapi.EgressNetworkPolicy) string {

‎pkg/network/node/ovscontroller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ func TestOVSPod(t *testing.T) {
303303
}
304304

305305
// Delete
306-
err = oc.TearDownPod("veth1", "10.128.0.2", sandboxID)
306+
err = oc.TearDownPod("10.128.0.2", sandboxID)
307307
if err != nil {
308308
t.Fatalf("Unexpected error deleting pod rules: %v", err)
309309
}

‎pkg/network/node/pod.go

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ func (m *podManager) ipamDel(id string) error {
503503
return nil
504504
}
505505

506-
func setupPodBandwidth(ovs *ovsController, pod *kapi.Pod, hostVeth string) error {
506+
func setupPodBandwidth(ovs *ovsController, pod *kapi.Pod, hostVeth, sandboxID string) error {
507507
ingressVal, egressVal, err := kbandwidth.ExtractPodBandwidthResources(pod.Annotations)
508508
if err != nil {
509509
return fmt.Errorf("failed to parse pod bandwidth: %v", err)
@@ -527,7 +527,7 @@ func setupPodBandwidth(ovs *ovsController, pod *kapi.Pod, hostVeth string) error
527527
egressBPS = egressVal.Value()
528528
}
529529

530-
return ovs.SetPodBandwidth(hostVeth, ingressBPS, egressBPS)
530+
return ovs.SetPodBandwidth(hostVeth, sandboxID, ingressBPS, egressBPS)
531531
}
532532

533533
func vnidToString(vnid uint32) string {
@@ -652,7 +652,7 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running
652652
if err != nil {
653653
return nil, nil, err
654654
}
655-
if err := setupPodBandwidth(m.ovs, pod, hostVethName); err != nil {
655+
if err := setupPodBandwidth(m.ovs, pod, hostVethName, req.SandboxID); err != nil {
656656
return nil, nil, err
657657
}
658658

@@ -678,26 +678,20 @@ func (m *podManager) update(req *cniserver.PodRequest) (uint32, error) {
678678
func (m *podManager) teardown(req *cniserver.PodRequest) error {
679679
defer PodOperationsLatency.WithLabelValues(PodOperationTeardown).Observe(sinceInMicroseconds(time.Now()))
680680

681-
netnsValid := true
682-
err := ns.IsNSorErr(req.Netns)
683-
if err != nil {
684-
if _, ok := err.(ns.NSPathNotExistErr); ok {
685-
glog.V(3).Infof("teardown called on already-destroyed pod %s/%s; only cleaning up IPAM", req.PodNamespace, req.PodName)
686-
netnsValid = false
687-
}
688-
}
689-
690-
var hostVethName string
691681
var podIP string
692-
if netnsValid {
693-
hostVethName, _, podIP, err = getVethInfo(req.Netns, podInterfaceName)
694-
if err != nil {
695-
return err
682+
errList := []error{}
683+
684+
if err := ns.IsNSorErr(req.Netns); err != nil {
685+
if _, ok := err.(ns.NSPathNotExistErr); !ok {
686+
// Namespace still exists, get pod IP from the veth
687+
_, _, podIP, err = getVethInfo(req.Netns, podInterfaceName)
688+
if err != nil {
689+
errList = append(errList, err)
690+
}
696691
}
697692
}
698693

699-
errList := []error{}
700-
if err := m.ovs.TearDownPod(hostVethName, podIP, req.SandboxID); err != nil {
694+
if err := m.ovs.TearDownPod(podIP, req.SandboxID); err != nil {
701695
errList = append(errList, err)
702696
}
703697

‎pkg/util/ovs/fake_ovs.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ func (fake *ovsFake) Set(table, record string, values ...string) error {
113113
return nil
114114
}
115115

116+
func (fake *ovsFake) Find(table, column, condition string) ([]string, error) {
117+
return make([]string, 0), nil
118+
}
119+
116120
func (fake *ovsFake) Clear(table, record string, columns ...string) error {
117121
return nil
118122
}

‎pkg/util/ovs/ovs.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ type Interface interface {
6262
// the value is already unset
6363
Clear(table, record string, columns ...string) error
6464

65+
// Find finds records in the OVS database that match the given condition.
66+
// It returns the value of the given column of matching records.
67+
Find(table, column, condition string) ([]string, error)
68+
6569
// DumpFlows dumps the flow table for the bridge and returns it as an array of
6670
// strings, one per flow.
6771
DumpFlows() ([]string, error)
@@ -244,6 +248,15 @@ func (ovsif *ovsExec) Set(table, record string, values ...string) error {
244248
return err
245249
}
246250

251+
// Returns the given column of records that match the condition
252+
func (ovsif *ovsExec) Find(table, column, condition string) ([]string, error) {
253+
output, err := ovsif.exec(OVS_VSCTL, "--no-heading", "--data=bare", "--columns="+column, "find", table, condition)
254+
if err != nil {
255+
return nil, err
256+
}
257+
return strings.Fields(output), nil
258+
}
259+
247260
func (ovsif *ovsExec) Clear(table, record string, columns ...string) error {
248261
args := append([]string{"--if-exists", "clear", table, record}, columns...)
249262
_, err := ovsif.exec(OVS_VSCTL, args...)

0 commit comments

Comments
 (0)