Address the PR comments

- Track only local changes.
- Add comment to the iptables rule which prevents SNAT'ing.
- Rename the ipset to weaver-no-masq-local to prevent NPC from destroying it.
- Fix comment about create/flush WEAVE-EXPOSE.
- Add comment about reexpose being no-op if bridge hasn't been exposed.
- Add comment that the feature is not only for Kubernetes.
- Reuse the ipset instance.
This commit is contained in:
Martynas Pumputis committed 2018-06-04 19:22:48 +02:00
1 parent 7fdf8fee13
commit b6115f0147
4 files changed
+29 -24

No files matched your search

+22 -12
View File
@@ -57,7 +57,7 @@ const (
DatapathIfName = "vethwe-datapath"
BridgeIfName = "vethwe-bridge"
PcapIfName = "vethwe-pcap"
NoMasqLocalIpset = ipset.Name("weave-no-masq-local")
NoMasqLocalIpset = ipset.Name("weaver-no-masq-local")
)
type Bridge interface {
@@ -233,7 +233,7 @@ func (config *BridgeConfig) configuredBridgeType() Bridge {
}
}
func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger) (Bridge, error) {
func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger, ips ipset.Interface) (Bridge, error) {
existingBridgeType, err := ExistingBridgeType(config.WeaveBridgeName, config.DatapathName)
if err != nil {
return nil, err
@@ -259,7 +259,7 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger) (Br
break
}
if err := configureIPTables(config); err != nil {
if err := configureIPTables(config, ips); err != nil {
return bridgeType, errors.Wrap(err, "configuring iptables")
}
@@ -417,7 +417,7 @@ func (f fastdpImpl) attach(veth *netlink.Veth) error {
return odp.AddDatapathInterfaceIfNotExist(f.datapathName, veth.Attrs().Name)
}
func configureIPTables(config *BridgeConfig) error {
func configureIPTables(config *BridgeConfig, ips ipset.Interface) error {
ipt, err := iptables.New()
if err != nil {
return errors.Wrap(err, "creating iptables object")
@@ -475,7 +475,7 @@ func configureIPTables(config *BridgeConfig) error {
}
if !config.NPC {
// Create a chain for allowing ingress traffic when the bridge is exposed
// 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")
}
@@ -500,9 +500,11 @@ func configureIPTables(config *BridgeConfig) error {
return err
}
// k8s-only: Create the ipset to store CIDRs allocated by IPAM for local pods.
// External traffic sent to these CIDRs avoids SNAT'ing so that NodePort
// with `"externalTrafficPolicy":"Local"` would receive packets with correct
// 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.
if config.NoMasqLocal {
ips := ipset.New(common.LogLogger())
@@ -510,7 +512,10 @@ func configureIPTables(config *BridgeConfig) error {
if err := ips.Create(NoMasqLocalIpset, ipset.HashNet); err != nil {
return err
}
if err := ipt.Insert("nat", "WEAVE", 1, "-m", "set", "--match-set", string(NoMasqLocalIpset), "dst", "-j", "RETURN"); err != nil {
if err := ipt.Insert("nat", "WEAVE", 1,
"-m", "set", "--match-set", string(NoMasqLocalIpset), "dst",
"-m", "comment", "--comment", "Prevent SNAT to locally running containers",
"-j", "RETURN"); err != nil {
return err
}
}
@@ -523,10 +528,9 @@ type NoMasqLocalTracker struct {
owner types.UID
}
func NewNoMasqLocalTracker() *NoMasqLocalTracker {
func NewNoMasqLocalTracker(ips ipset.Interface) *NoMasqLocalTracker {
return &NoMasqLocalTracker{
// TODO(mp) Reuse ipset object as each time calling New creates a test set.
ips: ipset.New(common.LogLogger()),
ips: ips,
owner: types.UID(0), // dummy ipset owner
}
}
@@ -536,6 +540,10 @@ func (t *NoMasqLocalTracker) String() string {
}
func (t *NoMasqLocalTracker) HandleUpdate(prevRanges, currRanges []address.Range, local bool) error {
if !local {
return nil
}
prev, curr := tracker.RemoveCommon(
address.NewCIDRs(tracker.Merge(prevRanges)),
address.NewCIDRs(tracker.Merge(currRanges)))
@@ -601,6 +609,8 @@ func ensureRulesAtTop(table, chain string, rulespecs [][]string, ipt *iptables.I
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.
//
// Ideally, we should consult IPAM for IP addrs allocated to "weave:expose",
// but we don't want to introduce dependency on IPAM, as weave should be able
// to run w/o IPAM.
+1 -8
View File
@@ -18,7 +18,6 @@ import (
"k8s.io/client-go/tools/cache"
"github.com/weaveworks/weave/common"
"github.com/weaveworks/weave/net"
"github.com/weaveworks/weave/net/ipset"
"github.com/weaveworks/weave/npc"
"github.com/weaveworks/weave/npc/metrics"
@@ -57,7 +56,7 @@ func resetIPTables(ipt *iptables.IPTables) error {
}
func resetIPSets(ips ipset.Interface) error {
// Remove ipsets prefixed `weave-` excluding weave-no-masq-local (used by weaver).
// Remove ipsets prefixed `weave-` only.
sets, err := ips.List(npc.IpsetNamePrefix)
if err != nil {
@@ -69,9 +68,6 @@ func resetIPSets(ips ipset.Interface) error {
// Must remove references to ipsets by other ipsets before they're destroyed
for _, s := range sets {
if s == ipset.Name(net.NoMasqLocalIpset) {
continue
}
common.Log.Debugf("Flushing ipset '%s'", string(s))
if err := ips.Flush(s); err != nil {
common.Log.Errorf("Failed to flush ipset '%s'", string(s))
@@ -80,9 +76,6 @@ func resetIPSets(ips ipset.Interface) error {
}
for _, s := range sets {
if s == ipset.Name(net.NoMasqLocalIpset) {
continue
}
common.Log.Debugf("Destroying ipset '%s'", string(s))
if err := ips.Destroy(s); err != nil {
common.Log.Errorf("Failed to destroy ipset '%s'", string(s))
+5 -3
View File
@@ -26,6 +26,7 @@ import (
"github.com/weaveworks/weave/nameserver"
weavenet "github.com/weaveworks/weave/net"
"github.com/weaveworks/weave/net/address"
"github.com/weaveworks/weave/net/ipset"
"github.com/weaveworks/weave/plugin"
weaveproxy "github.com/weaveworks/weave/proxy"
weave "github.com/weaveworks/weave/router"
@@ -221,7 +222,7 @@ func main() {
mflag.BoolVar(&pluginConfig.EnableV2Multicast, []string{"-plugin-v2-multicast"}, false, "enable multicast for Docker plugin (v2)")
mflag.StringVar(&pluginConfig.Socket, []string{"-plugin-socket"}, "/run/docker/plugins/weave.sock", "plugin socket on which to listen")
mflag.StringVar(&pluginConfig.MeshSocket, []string{"-plugin-mesh-socket"}, "/run/docker/plugins/weavemesh.sock", "plugin socket on which to listen in mesh mode")
mflag.BoolVar(&bridgeConfig.NoMasqLocal, []string{"-no-masq-local"}, false, "do not SNAT external traffic sent to pods running on this node (Kubernetes only)")
mflag.BoolVar(&bridgeConfig.NoMasqLocal, []string{"-no-masq-local"}, false, "do not SNAT external traffic sent to containers running on this node")
proxyConfig := newProxyConfig()
@@ -306,7 +307,8 @@ func main() {
bridgeConfig.Mac = name.String()
bridgeConfig.Port = config.Port
bridgeType, err := weavenet.EnsureBridge(procPath, &bridgeConfig, Log)
ips := ipset.New(common.LogLogger())
bridgeType, err := weavenet.EnsureBridge(procPath, &bridgeConfig, Log, ips)
checkFatal(err)
Log.Println("Bridge type is", bridgeType)
@@ -408,7 +410,7 @@ func main() {
Log.Fatalf("Cannot create AWSVPC LocalRangeTracker: %s", err)
}
} else if bridgeConfig.NoMasqLocal {
t = weavenet.NewNoMasqLocalTracker()
t = weavenet.NewNoMasqLocalTracker(ips)
}
if t != nil {
Log.Infof("Using %q LocalRangeTracker", t)
+1 -1
View File
@@ -506,7 +506,7 @@ destroy_bridge() {
run_iptables -t nat -D POSTROUTING -o $BRIDGE -j ACCEPT >/dev/null 2>&1 || true
run_iptables -t nat -X WEAVE >/dev/null 2>&1 || true
ipset destroy weave-no-masq-local >/dev/null 2>&1 || true
ipset destroy weaver-no-masq-local >/dev/null 2>&1 || true
}
add_iface_fastdp() {