Skip to content

Commit e58ec8a

Browse files
author
hyphen.wang
committed
bridge: clean ip masq if netns is empty
In the Del command it didn't clean ip masq when netns is empty. Add the clean-up code if ip address can fetch from prevResult in StdinData. Fix #810 Signed-off-by: hyphen.wang <[email protected]>
1 parent 5188dc8 commit e58ec8a

File tree

1 file changed

+90
-16
lines changed

1 file changed

+90
-16
lines changed

plugins/main/bridge/bridge.go

Lines changed: 90 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"encoding/json"
1919
"errors"
2020
"fmt"
21+
"log"
2122
"net"
2223
"os"
2324
"runtime"
@@ -42,6 +43,7 @@ import (
4243

4344
// For testcases to force an error after IPAM has been performed
4445
var debugPostIPAMError error
46+
var logger = log.New(os.Stdout, "", log.Ldate|log.Ltime)
4547

4648
const defaultBrName = "cni0"
4749

@@ -774,10 +776,6 @@ func cmdDel(args *skel.CmdArgs) error {
774776
return nil
775777
}
776778

777-
if args.Netns == "" {
778-
return ipamDel()
779-
}
780-
781779
// There is a netns so try to clean up. Delete can be called multiple times
782780
// so don't return an error if the device is already removed.
783781
// If the device isn't there then don't try to clean up IP masq either.
@@ -791,28 +789,35 @@ func cmdDel(args *skel.CmdArgs) error {
791789
return err
792790
})
793791
if err != nil {
794-
// if NetNs is passed down by the Cloud Orchestration Engine, or if it called multiple times
792+
// if NetNs is passed down by the Cloud Orchestration Engine, or if it called multiple times
795793
// so don't return an error if the device is already removed.
796794
// https://github.com/kubernetes/kubernetes/issues/43014#issuecomment-287164444
797795
_, ok := err.(ns.NSPathNotExistErr)
798-
if ok {
799-
return ipamDel()
796+
if !ok {
797+
return err
800798
}
801-
return err
802799
}
803800

804-
// call ipam.ExecDel after clean up device in netns
805-
if err := ipamDel(); err != nil {
806-
return err
807-
}
801+
if len(ipnets) == 0 {
802+
// could not get ip address within netns, so try to get it from prevResult
803+
prevResult, err := parsePrevResult(n)
804+
if err != nil {
805+
return err
806+
}
808807

809-
if n.MacSpoofChk {
810-
sc := link.NewSpoofChecker("", "", uniqueID(args.ContainerID, args.IfName))
811-
if err := sc.Teardown(); err != nil {
812-
fmt.Fprintf(os.Stderr, "%v", err)
808+
if prevResult != nil {
809+
ipCfgs, err := getIPCfgs(args.IfName, prevResult)
810+
if err != nil {
811+
return err
812+
}
813+
814+
for _, ip := range ipCfgs {
815+
ipnets = append(ipnets, &ip.Address)
816+
}
813817
}
814818
}
815819

820+
// clean up IP masq first
816821
if isLayer3 && n.IPMasq {
817822
chain := utils.FormatChainName(n.Name, args.ContainerID)
818823
comment := utils.FormatComment(n.Name, args.ContainerID)
@@ -823,6 +828,18 @@ func cmdDel(args *skel.CmdArgs) error {
823828
}
824829
}
825830

831+
// call ipam.ExecDel after clean up device in netns
832+
if err := ipamDel(); err != nil {
833+
return err
834+
}
835+
836+
if n.MacSpoofChk {
837+
sc := link.NewSpoofChecker("", "", uniqueID(args.ContainerID, args.IfName))
838+
if err := sc.Teardown(); err != nil {
839+
fmt.Fprintf(os.Stderr, "%v", err)
840+
}
841+
}
842+
826843
return err
827844
}
828845

@@ -1088,3 +1105,60 @@ func cmdCheck(args *skel.CmdArgs) error {
10881105
func uniqueID(containerID, cniIface string) string {
10891106
return containerID + "-" + cniIface
10901107
}
1108+
1109+
// parsePrevResult parse previous result
1110+
func parsePrevResult(conf *NetConf) (*current.Result, error) {
1111+
var prevResult *current.Result
1112+
if conf.RawPrevResult != nil {
1113+
resultBytes, err := json.Marshal(conf.RawPrevResult)
1114+
if err != nil {
1115+
return nil, fmt.Errorf("could not serialize prevResult: %#v", err)
1116+
}
1117+
res, err := version.NewResult(conf.CNIVersion, resultBytes)
1118+
if err != nil {
1119+
return nil, fmt.Errorf("could not parse prevResult: %v", err)
1120+
}
1121+
conf.RawPrevResult = nil
1122+
prevResult, err = current.NewResultFromResult(res)
1123+
if err != nil {
1124+
return nil, fmt.Errorf("could not convert result to current version: %v", err)
1125+
}
1126+
}
1127+
return prevResult, nil
1128+
}
1129+
1130+
// getIPCfgs finds the IPs on the supplied interface, returning as IPConfig structures
1131+
func getIPCfgs(iface string, prevResult *current.Result) ([]*current.IPConfig, error) {
1132+
if len(prevResult.IPs) == 0 {
1133+
// No IP addresses; that makes no sense. Pack it in.
1134+
return nil, fmt.Errorf("No IP addresses supplied on interface: %s", iface)
1135+
}
1136+
1137+
// We do a single interface name, stored in args.IfName
1138+
logger.Printf("Checking for relevant interface: %s", iface)
1139+
1140+
// ips contains the IPConfig structures that were passed, filtered somewhat
1141+
ipCfgs := make([]*current.IPConfig, 0, len(prevResult.IPs))
1142+
1143+
for _, ipCfg := range prevResult.IPs {
1144+
// IPs have an interface that is an index into the interfaces array.
1145+
// We assume a match if this index is missing.
1146+
if ipCfg.Interface == nil {
1147+
logger.Printf("No interface for IP address %s", ipCfg.Address.IP)
1148+
ipCfgs = append(ipCfgs, ipCfg)
1149+
continue
1150+
}
1151+
1152+
// Skip all IPs we know belong to an interface with the wrong name.
1153+
intIdx := *ipCfg.Interface
1154+
if intIdx >= 0 && intIdx < len(prevResult.Interfaces) && prevResult.Interfaces[intIdx].Name != iface {
1155+
logger.Printf("Incorrect interface for IP address %s", ipCfg.Address.IP)
1156+
continue
1157+
}
1158+
1159+
logger.Printf("Found IP address %s", ipCfg.Address.IP.String())
1160+
ipCfgs = append(ipCfgs, ipCfg)
1161+
}
1162+
1163+
return ipCfgs, nil
1164+
}

0 commit comments

Comments
 (0)