Skip to content

Commit 053be10

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 containernetworking#810 Signed-off-by: hyphen.wang <[email protected]>
1 parent 5188dc8 commit 053be10

File tree

1 file changed

+93
-17
lines changed

1 file changed

+93
-17
lines changed

plugins/main/bridge/bridge.go

+93-17
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"
@@ -41,7 +42,10 @@ import (
4142
)
4243

4344
// For testcases to force an error after IPAM has been performed
44-
var debugPostIPAMError error
45+
var (
46+
debugPostIPAMError error
47+
logger = log.New(os.Stdout, "", log.Ldate|log.Ltime)
48+
)
4549

4650
const defaultBrName = "cni0"
4751

@@ -774,10 +778,6 @@ func cmdDel(args *skel.CmdArgs) error {
774778
return nil
775779
}
776780

777-
if args.Netns == "" {
778-
return ipamDel()
779-
}
780-
781781
// There is a netns so try to clean up. Delete can be called multiple times
782782
// so don't return an error if the device is already removed.
783783
// If the device isn't there then don't try to clean up IP masq either.
@@ -791,28 +791,35 @@ func cmdDel(args *skel.CmdArgs) error {
791791
return err
792792
})
793793
if err != nil {
794-
// if NetNs is passed down by the Cloud Orchestration Engine, or if it called multiple times
794+
// if NetNs is passed down by the Cloud Orchestration Engine, or if it called multiple times
795795
// so don't return an error if the device is already removed.
796796
// https://github.com/kubernetes/kubernetes/issues/43014#issuecomment-287164444
797797
_, ok := err.(ns.NSPathNotExistErr)
798-
if ok {
799-
return ipamDel()
798+
if !ok {
799+
return err
800800
}
801-
return err
802801
}
803802

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

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)
810+
if prevResult != nil {
811+
ipCfgs, err := getIPCfgs(args.IfName, prevResult)
812+
if err != nil {
813+
return err
814+
}
815+
816+
for _, ip := range ipCfgs {
817+
ipnets = append(ipnets, &ip.Address)
818+
}
813819
}
814820
}
815821

822+
// clean up IP masq first
816823
if isLayer3 && n.IPMasq {
817824
chain := utils.FormatChainName(n.Name, args.ContainerID)
818825
comment := utils.FormatComment(n.Name, args.ContainerID)
@@ -823,6 +830,18 @@ func cmdDel(args *skel.CmdArgs) error {
823830
}
824831
}
825832

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

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

0 commit comments

Comments
 (0)