Make ConfigureIPTables idempotent and poll it

- Instead of EnsureBridge
- Adds ipset.ExistEntry method
- Refactor ipset method names Exist <-> EntryExists
- Turn on iptable-refresh by default (10s)
- Adds a test that checks iptables are refreshed, check EXPOSE recovers

net/bridge.go

@@ -262,7 +262,7 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger, ips
 		break
 	}
 
-	if err := configureIPTables(config, ips); err != nil {
+	if err := ConfigureIPTables(config, ips); err != nil {
 		return bridgeType, errors.Wrap(err, "configuring iptables")
 	}
 
@@ -297,9 +297,7 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger, ips
 		return bridgeType, errors.Wrapf(err, "configuring ARP cache on bridge %q", config.WeaveBridgeName)
 	}
 
-	// NB: No concurrent call to Expose is possible, as EnsureBridge is called
-	// before any service has been started.
-	if err := reexpose(config, log); err != nil {
+	if err := Reexpose(config, log); err != nil {
 		return bridgeType, err
 	}
 
@@ -431,10 +429,40 @@ func (f fastdpImpl) attach(veth *netlink.Veth) error {
 	return odp.AddDatapathInterfaceIfNotExist(f.datapathName, veth.Attrs().Name)
 }
 
-func configureIPTables(config *BridgeConfig, ips ipset.Interface) error {
+// ResetIPTables resets IPTables if they're in a strange state from a previous run.
+func ResetIPTables(config *BridgeConfig, ips ipset.Interface) error {
 	ipt, err := iptables.New()
 	if err != nil {
-		return errors.Wrap(err, "creating iptables object")
+		return errors.Wrap(err, "creating iptables object while resetting")
+	}
+
+	if !config.NPC {
+		// Create/Flush a chain for allowing ingress traffic when the bridge is exposed
+		if err := ipt.ClearChain("filter", "WEAVE-EXPOSE"); err != nil {
+			return errors.Wrap(err, "failed to clear/create filter/WEAVE-EXPOSE chain")
+		}
+	}
+
+	if err := ipt.ClearChain("nat", "WEAVE"); err != nil {
+		return errors.Wrap(err, "failed to clear/create nat/WEAVE chain")
+	}
+
+	if config.NoMasqLocal {
+		ips := ipset.New(common.LogLogger(), 0)
+		_ = ips.Destroy(NoMasqLocalIpset)
+		if err := ips.Create(NoMasqLocalIpset, ipset.HashNet); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// ConfigureIPTables idempotently configures all the iptables!
+func ConfigureIPTables(config *BridgeConfig, ips ipset.Interface) error {
+	ipt, err := iptables.New()
+	if err != nil {
+		return errors.Wrap(err, "creating iptables object while configuring")
 	}
 
 	// The order among weave filter/FORWARD rules is important!
@@ -515,8 +543,8 @@ func configureIPTables(config *BridgeConfig, ips ipset.Interface) error {
 
 	if !config.NPC {
 		// Create/Flush a chain for allowing ingress traffic when the bridge is exposed
-		if err := ipt.ClearChain("filter", "WEAVE-EXPOSE"); err != nil {
-			return errors.Wrap(err, "failed to clear/create filter/WEAVE-EXPOSE chain")
+		if err := ensureChains(ipt, "filter", "WEAVE-EXPOSE"); err != nil {
+			return errors.Wrap(err, "failed to ensure existence of filter/WEAVE-EXPOSE chain")
 		}
 
 		fwdRules = append(fwdRules, []string{"-o", config.WeaveBridgeName, "-j", "WEAVE-EXPOSE"})
@@ -532,8 +560,8 @@ func configureIPTables(config *BridgeConfig, ips ipset.Interface) error {
 	}
 
 	// Create a chain for masquerading
-	if err := ipt.ClearChain("nat", "WEAVE"); err != nil {
-		return errors.Wrap(err, "failed to clear/create nat/WEAVE chain")
+	if err := ensureChains(ipt, "nat", "WEAVE"); err != nil {
+		return errors.Wrap(err, "failed to ensure existence of nat/WEAVE chain")
 	}
 	if err := ipt.AppendUnique("nat", "POSTROUTING", "-j", "WEAVE"); err != nil {
 		return err
@@ -547,11 +575,17 @@ func configureIPTables(config *BridgeConfig, ips ipset.Interface) error {
 	// src IP addr.
 	if config.NoMasqLocal {
 		ips := ipset.New(common.LogLogger(), 0)
-		_ = ips.Destroy(NoMasqLocalIpset)
-		if err := ips.Create(NoMasqLocalIpset, ipset.HashNet); err != nil {
-			return err
+
+		noMasqLocalExists, err := ips.Exists(NoMasqLocalIpset)
+		if err != nil {
+			common.Log.Errorf("Failed to look if ipset '%s' exists: %s", NoMasqLocalIpset, err)
+		} else if !noMasqLocalExists {
+			if err := ips.Create(NoMasqLocalIpset, ipset.HashNet); err != nil {
+				return err
+			}
 		}
-		if err := ipt.Insert("nat", "WEAVE", 1,
+
+		if err := ipt.AppendUnique("nat", "WEAVE",
 			"-m", "set", "--match-set", string(NoMasqLocalIpset), "dst",
 			"-m", "comment", "--comment", "Prevent SNAT to locally running containers",
 			"-j", "RETURN"); err != nil {
@@ -609,7 +643,7 @@ func linkSetUpByName(linkName string) error {
 	return netlink.LinkSetUp(link)
 }
 
-func reexpose(config *BridgeConfig, log *logrus.Logger) error {
+func Reexpose(config *BridgeConfig, log *logrus.Logger) error {
 	// Get existing IP addrs of the weave bridge.
 	// If the bridge hasn't been exposed, then this functions does nothing.
 	//

net/ipset/ipset.go

@@ -26,7 +26,8 @@ type Interface interface {
 	Create(ipsetName Name, ipsetType Type) error
 	AddEntry(user types.UID, ipsetName Name, entry string, comment string) error
 	DelEntry(user types.UID, ipsetName Name, entry string) error
-	Exist(user types.UID, ipsetName Name, entry string) bool
+	EntryExists(user types.UID, ipsetName Name, entry string) bool
+	Exists(ipsetName Name) (bool, error)
 	Flush(ipsetName Name) error
 	Destroy(ipsetName Name) error
 
@@ -121,10 +122,24 @@ func (i *ipset) DelEntry(user types.UID, ipsetName Name, entry string) error {
 	return doExec("del", string(ipsetName), entry)
 }
 
-func (i *ipset) Exist(user types.UID, ipsetName Name, entry string) bool {
+func (i *ipset) EntryExists(user types.UID, ipsetName Name, entry string) bool {
 	return i.existUser(user, ipsetName, entry)
 }
 
+// Dummy way to check whether a given ipset exists.
+func (i *ipset) Exists(name Name) (bool, error) {
+	sets, err := i.List(string(name))
+	if err != nil {
+		return false, err
+	}
+	for _, s := range sets {
+		if s == name {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func (i *ipset) Flush(ipsetName Name) error {
 	i.removeSetFromUsers(ipsetName)
 	return doExec("flush", string(ipsetName))