diff --git a/net/bridge.go b/net/bridge.go index f882ebb1..268e70c1 100644 --- a/net/bridge.go +++ b/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 in case 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,26 +560,22 @@ 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 } // For the cases where the weave bridge is the default gateway for - // containers (e.g. Kubernetes): create the ipset to store CIDRs allocated - // by IPAM for local containers. In the case of Kubernetes, external traffic - // sent to these CIDRs avoids SNAT'ing so that NodePort with - // `"externalTrafficPolicy":"Local"` would receive packets with correct - // src IP addr. + // containers (e.g. Kubernetes): In `ResetIPTables` (which we assume + // to have been called at this point) we create an ipset to store CIDRs + // allocated by IPAM for local containers. + // In the case of Kubernetes, external traffic sent to these CIDRs + // avoids SNAT'ing so that NodePort with `"externalTrafficPolicy":"Local"` + // would receive packets with correct 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 - } - 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 +633,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. // diff --git a/net/expose.go b/net/expose.go index 76ba41a1..d35f781e 100644 --- a/net/expose.go +++ b/net/expose.go @@ -103,13 +103,8 @@ func addNatRule(ipt *iptables.IPTables, rulespec ...string) error { // Loop until we get an exit code other than "temporarily unavailable" for { if err := ipt.AppendUnique("nat", "WEAVE", rulespec...); err != nil { - if ierr, ok := err.(*iptables.Error); ok { - if status, ok := ierr.ExitError.Sys().(syscall.WaitStatus); ok { - // (magic exit code 4 found in iptables source code; undocumented) - if status.ExitStatus() == 4 { - continue - } - } + if isResourceError(err) { + continue } return err } diff --git a/net/ipset/ipset.go b/net/ipset/ipset.go index b538a6c9..4c0b7481 100644 --- a/net/ipset/ipset.go +++ b/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)) diff --git a/net/iptables.go b/net/iptables.go index a11ca533..cda4655a 100644 --- a/net/iptables.go +++ b/net/iptables.go @@ -2,9 +2,13 @@ package net import ( "strings" + "syscall" + "time" "github.com/coreos/go-iptables/iptables" "github.com/pkg/errors" + "github.com/sirupsen/logrus" + utilwait "k8s.io/apimachinery/pkg/util/wait" ) // AddChainWithRules creates a chain and appends given rules to it. @@ -104,3 +108,119 @@ func ensureRulesAtTop(table, chain string, rulespecs [][]string, ipt *iptables.I return nil } + +func chainExists(ipt *iptables.IPTables, table string, chain string) (bool, error) { + existingChains, err := ipt.ListChains(table) + if err != nil { + return false, errors.Wrapf(err, "ipt.ListChains(%s)", table) + } + chainMap := make(map[string]struct{}) + for _, c := range existingChains { + chainMap[c] = struct{}{} + } + + _, found := chainMap[chain] + return found, nil +} + +const ( + // Max time we wait for an iptables flush to complete after we notice it has started + iptablesFlushTimeout = 5 * time.Second + // How often we poll while waiting for an iptables flush to complete + iptablesFlushPollTime = 100 * time.Millisecond +) + +// MonitorForIptablesFlush periodically checks for a canary chain in iptables. If this canary chain goes missing it calls the reloadFunc. +// This is a more efficient way of detecting whether firewalld or another process has been removing rules that we rely on. +// The reloadFunc can then check whether other chains that should exist are still there, fix things and restore the canary. +func MonitorForIptablesFlush(log *logrus.Logger, canary string, tables []string, reloadFunc func(), interval time.Duration, stopCh <-chan struct{}) { + ipt, err := iptables.New() + if err != nil { + log.Errorf("creating iptables object while initializing iptable Monitoring %s", err) + return + } + + for { + _ = PollImmediateUntil(interval, func() (bool, error) { + for _, table := range tables { + if err := ensureChains(ipt, table, canary); err != nil { + log.Warningf("Could not set up iptables canary %s/%s: %v", table, canary, err) + return false, nil + } + } + return true, nil + }, stopCh) + + // Poll until stopCh is closed or iptables is flushed + err = utilwait.PollUntil(interval, func() (bool, error) { + if exists, err := chainExists(ipt, tables[0], canary); exists { + return false, nil + } else if isResourceError(err) { + log.Warningf("Could not check for iptables canary %s/%s: %v", tables[0], canary, err) + return false, nil + } + log.Infof("iptables canary %s/%s deleted", tables[0], canary) + + // Wait for the other canaries to be deleted too before returning + // so we don't start reloading too soon. + err := utilwait.PollImmediate(iptablesFlushPollTime, iptablesFlushTimeout, func() (bool, error) { + for i := 1; i < len(tables); i++ { + if exists, err := chainExists(ipt, tables[i], canary); exists || isResourceError(err) { + return false, nil + } + } + return true, nil + }) + if err != nil { + log.Warning("Inconsistent iptables state detected.") + } + return true, nil + }, stopCh) + + if err != nil { + // stopCh was closed + for _, table := range tables { + _ = ipt.DeleteChain(table, canary) + } + return + } + + log.Infof("Reloading after iptables flush") + reloadFunc() + } +} + +const iptablesStatusResourceProblem = 4 + +// isResourceError returns true if the error indicates that iptables ran into a "resource +// problem" and was unable to attempt the request. In particular, this will be true if it +// times out trying to get the iptables lock. +func isResourceError(err error) bool { + if ierr, ok := err.(*iptables.Error); ok { + if status, ok := ierr.ExitError.Sys().(syscall.WaitStatus); ok { + return status.ExitStatus() == iptablesStatusResourceProblem + } + } + + return false +} + +// PollImmediateUntil tries a condition func until it returns true, an error or stopCh is closed. +// +// PollImmediateUntil runs the 'condition' before waiting for the interval. +// 'condition' will always be invoked at least once. +func PollImmediateUntil(interval time.Duration, condition utilwait.ConditionFunc, stopCh <-chan struct{}) error { + done, err := condition() + if err != nil { + return err + } + if done { + return nil + } + select { + case <-stopCh: + return utilwait.ErrWaitTimeout + default: + return utilwait.PollUntil(interval, condition, stopCh) + } +} diff --git a/npc/controller_test.go b/npc/controller_test.go index 860b5e5c..618e453c 100644 --- a/npc/controller_test.go +++ b/npc/controller_test.go @@ -72,12 +72,17 @@ func (i *mockIPSet) DelEntry(user types.UID, ipsetName ipset.Name, entry string) return nil } -func (i *mockIPSet) Exist(user types.UID, ipsetName ipset.Name, entry string) bool { +func (i *mockIPSet) EntryExists(user types.UID, ipsetName ipset.Name, entry string) bool { _, ok := i.sets[string(ipsetName)].subSets[entry][user] return ok } -func (i *mockIPSet) entryExists(ipsetName ipset.Name, entry string) bool { +func (i *mockIPSet) Exists(ipsetName ipset.Name) (bool, error) { + _, ok := i.sets[string(ipsetName)] + return ok, nil +} + +func (i *mockIPSet) entriesExist(ipsetName ipset.Name, entry string) bool { return len(i.sets[string(ipsetName)].subSets[entry]) > 0 } @@ -262,8 +267,8 @@ func TestDefaultAllow(t *testing.T) { controller.AddPod(podFoo) // Should add the foo pod to default-allow - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) podBar := &coreapi.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -278,8 +283,8 @@ func TestDefaultAllow(t *testing.T) { controller.UpdatePod(podBarNoIP, podBar) // Should add the bar pod to default-allow - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, barPodIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, barPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, barPodIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, barPodIP)) // Allow access from the bar pod to the foo pod netpol := &networkingv1.NetworkPolicy{ @@ -310,48 +315,48 @@ func TestDefaultAllow(t *testing.T) { controller.AddNetworkPolicy(netpol) // Should remove the foo pod from default-allow as the netpol selects it - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.False(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, barPodIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, barPodIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.False(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, barPodIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, barPodIP)) podBarWithNewIP := *podBar podBarWithNewIP.Status.PodIP = barPodNewIP controller.UpdatePod(podBar, &podBarWithNewIP) // Should update IP addr of the bar pod in default-allow - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, barPodIP)) - require.False(t, m.entryExists(egressDefaultAllowIPSetName, barPodIP)) - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, barPodNewIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, barPodNewIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, barPodIP)) + require.False(t, m.entriesExist(egressDefaultAllowIPSetName, barPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, barPodNewIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, barPodNewIP)) controller.UpdatePod(&podBarWithNewIP, podBarNoIP) // Should remove the bar pod from default-allow as it does not have any IP addr - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, barPodNewIP)) - require.False(t, m.entryExists(egressDefaultAllowIPSetName, barPodNewIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, barPodNewIP)) + require.False(t, m.entriesExist(egressDefaultAllowIPSetName, barPodNewIP)) podFooWithNewLabel := *podFoo podFooWithNewLabel.ObjectMeta.Labels = map[string]string{"run": "new-foo"} controller.UpdatePod(podFoo, &podFooWithNewLabel) // Should bring back the foo pod to default-allow as it does not match dst of any netpol - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) controller.UpdatePod(&podFooWithNewLabel, podFoo) // Should remove from default-allow as it matches the netpol after the update - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.False(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.False(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) controller.DeleteNetworkPolicy(netpol) // Should bring back the foo pod to default-allow as no netpol selects it - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.True(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.True(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) controller.DeletePod(podFoo) // Should remove foo pod from default-allow - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, fooPodIP)) - require.False(t, m.entryExists(egressDefaultAllowIPSetName, fooPodIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, fooPodIP)) + require.False(t, m.entriesExist(egressDefaultAllowIPSetName, fooPodIP)) controller.DeleteNamespace(defaultNamespace) // Should remove default ipset @@ -388,7 +393,7 @@ func TestOutOfOrderPodEvents(t *testing.T) { controller.AddPod(podFoo) // Should be in default-allow as no netpol selects podFoo - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) netpol := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -410,7 +415,7 @@ func TestOutOfOrderPodEvents(t *testing.T) { controller.AddNetworkPolicy(netpol) // Shouldn't be in default-allow as netpol above selects podFoo - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) podBar := &coreapi.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -422,10 +427,10 @@ func TestOutOfOrderPodEvents(t *testing.T) { controller.AddPod(podBar) // Should be in default-allow as no netpol selects podBar - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) - require.True(t, m.Exist(podBar.ObjectMeta.UID, ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.EntryExists(podBar.ObjectMeta.UID, ingressDefaultAllowIPSetName, podIP)) // Should be in run=bar ipset - require.True(t, m.entryExists(runBarIPSetName, podIP)) + require.True(t, m.entriesExist(runBarIPSetName, podIP)) controller.DeletePod(podFoo) // Multiple duplicate events should not affect npc state @@ -434,13 +439,13 @@ func TestOutOfOrderPodEvents(t *testing.T) { // Should be in default-allow as no netpol selects podBar and podFoo removal // should not affect podBar in default-allow - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) controller.DeletePod(podBar) // Should remove from default-allow and run=bar ipsets require.Equal(t, 0, len(m.sets[ingressDefaultAllowIPSetName].subSets)) - require.False(t, m.entryExists(runBarIPSetName, podIP)) + require.False(t, m.entriesExist(runBarIPSetName, podIP)) } // Test case for https://github.com/weaveworks/weave/issues/3222 @@ -471,7 +476,7 @@ func TestNewTargetSelector(t *testing.T) { Status: coreapi.PodStatus{PodIP: podIP}} controller.AddPod(podFoo) - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) netpolBar := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -492,7 +497,7 @@ func TestNewTargetSelector(t *testing.T) { controller.AddNetworkPolicy(netpolBar) // netpolBar target selector selects podFoo - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) netpolFoo := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -514,10 +519,10 @@ func TestNewTargetSelector(t *testing.T) { controller.DeleteNetworkPolicy(netpolBar) // netpolFoo target-selects podFoo - require.False(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.False(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) controller.DeleteNetworkPolicy(netpolFoo) // No netpol target-selects podFoo - require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP)) + require.True(t, m.entriesExist(ingressDefaultAllowIPSetName, podIP)) } func TestEgressPolicyWithIPBlock(t *testing.T) { @@ -573,8 +578,8 @@ func TestEgressPolicyWithIPBlock(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(m.sets[exceptIPSetName].subSets)) - require.True(t, m.entryExists(exceptIPSetName, "192.168.48.1/32")) - require.True(t, m.entryExists(exceptIPSetName, "192.168.48.2/32")) + require.True(t, m.entriesExist(exceptIPSetName, "192.168.48.1/32")) + require.True(t, m.entriesExist(exceptIPSetName, "192.168.48.2/32")) // Each egress rule is represented as two iptables rules (-J MARK and -J RETURN). require.Equal(t, 2, len(ipt.rules[EgressCustomChain])) @@ -683,7 +688,7 @@ func TestIngressPolicyWithIPBlockAndPortSpecified(t *testing.T) { defer controller.DeleteNetworkPolicy(netpolicty) require.Equal(t, 1, len(m.sets[runBarIPSetName].subSets)) - require.True(t, m.entryExists(runBarIPSetName, barPodIP)) + require.True(t, m.entriesExist(runBarIPSetName, barPodIP)) require.Equal(t, 1, len(ipt.rules[IngressChain])) for rule := range ipt.rules[IngressChain] { diff --git a/npc/namespace.go b/npc/namespace.go index 3c061111..6423b4cd 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -441,7 +441,7 @@ func (ns *ns) updateDefaultAllowIPSetEntry(oldObj, newObj *coreapi.Pod, ipsetNam // Instead of iterating over all selectors we check whether old pod IP // has been inserted into default-allow ipset to decide whether the IP // in the ipset has to be updated. - if ns.ips.Exist(oldObj.ObjectMeta.UID, ipsetName, oldObj.Status.PodIP) { + if ns.ips.EntryExists(oldObj.ObjectMeta.UID, ipsetName, oldObj.Status.PodIP) { if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ipsetName, oldObj.Status.PodIP); err != nil { return err diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 4da4a1de..22e4e556 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -196,7 +196,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { } // delete `weave-local-pods` ipset which is no longer used by weave-npc - weaveLocalPodExist, err := ipsetExist(ips, npc.LocalIpset) + weaveLocalPodExist, err := ips.Exists(npc.LocalIpset) if err != nil { common.Log.Errorf("Failed to look if ipset '%s' exists", npc.LocalIpset) } else if weaveLocalPodExist { @@ -209,22 +209,6 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { return nil } -// Dummy way to check whether a given ipset exists. -// TODO(brb) Use "ipset -exist create <..>" for our purpose instead (for some reasons -// creating an ipset with -exist fails). -func ipsetExist(ips ipset.Interface, name ipset.Name) (bool, error) { - sets, err := ips.List(string(name)) - if err != nil { - return false, err - } - for _, s := range sets { - if s == name { - return true, nil - } - } - return false, nil -} - func root(cmd *cobra.Command, args []string) { var npController cache.Controller diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 3edaed16..811fc5d9 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -313,8 +313,11 @@ func main() { } } ips := ipset.New(common.LogLogger(), 0) + err = weavenet.ResetIPTables(&bridgeConfig, ips) + checkFatal(err) bridgeType, err := weavenet.EnsureBridge(procPath, &bridgeConfig, Log, ips) checkFatal(err) + Log.Println("Bridge type is", bridgeType) config.Password = determinePassword(password) @@ -512,6 +515,18 @@ func main() { go exposeForAWSVPC(allocator, defaultSubnet, bridgeConfig.WeaveBridgeName, waitReady.Add()) } + applyIPTables := func() { + Log.Info("Re-configuring iptables") + err := weavenet.ConfigureIPTables(&bridgeConfig, ips) + if err != nil { + Log.Errorf("Error configuring iptables: %s", err) + } + weavenet.Reexpose(&bridgeConfig, Log) + } + stopChan := make(chan struct{}) + go weavenet.MonitorForIptablesFlush(Log, "WEAVE-CANARY", []string{"mangle", "nat", "filter"}, applyIPTables, 10*time.Second, stopChan) + defer close(stopChan) + signals.SignalHandlerLoop(common.Log, router) } diff --git a/test/135_expose_iptables_refresh.sh b/test/135_expose_iptables_refresh.sh new file mode 100755 index 00000000..a844e7cc --- /dev/null +++ b/test/135_expose_iptables_refresh.sh @@ -0,0 +1,47 @@ +#!/bin/bash + +. "$(dirname "$0")/config.sh" + +run_on1() { + assert_raises "run_on $HOST1 $@" +} + +weave_on1() { + assert_raises "weave_on $HOST1 $@" +} + +stop_weave_on1() { + assert_raises "stop_weave_on $HOST1 $@" +} + +get_weave_iptable_rules() { + get_command_output_on $HOST1 "sudo iptables-save | grep -i weave" +} + +wait_for_iptable_refresh() { + # Canary is checked every 10s + sleep 12 +} + +start_suite "exposing weave network to host" + +# Launch +weave_on1 "launch" +IPT_BEFORE=$(get_weave_iptable_rules) +run_on1 "sudo iptables -t nat -X WEAVE-CANARY" +wait_for_iptable_refresh +IPT_AFTER=$(get_weave_iptable_rules) +assert_raises "diff <(echo $IPT_BEFORE) <(echo $IPT_AFTER)" 0 +stop_weave_on1 + +# After exposing +weave_on1 "launch" +weave_on1 "expose" +IPT_BEFORE=$(get_weave_iptable_rules) +run_on1 "sudo iptables -t nat -X WEAVE-CANARY" +wait_for_iptable_refresh +IPT_AFTER=$(get_weave_iptable_rules) +assert_raises "diff <(echo $IPT_BEFORE) <(echo $IPT_AFTER)" 0 +stop_weave_on1 + +end_suite