From 95c6a488d82465ff00ef3ee0cfadbc91a91de4b5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Feb 2021 12:33:18 -0500 Subject: [PATCH] Make kube-proxy check if IPv6 is really supported before assuming dual-stack --- cmd/kube-proxy/app/server_others.go | 68 ++++++++++--------- .../network/hostport/fake_iptables.go | 8 +++ pkg/util/iptables/iptables.go | 14 ++-- pkg/util/iptables/testing/fake.go | 5 ++ 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index b44dbb54b2f..bcbf7392f06 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -134,15 +134,7 @@ func newProxyServer( } nodeIP := detectNodeIP(client, hostname, config.BindAddress) - protocol := utiliptables.ProtocolIPv4 - if utilsnet.IsIPv6(nodeIP) { - klog.V(0).Infof("kube-proxy node IP is an IPv6 address (%s), assume IPv6 operation", nodeIP.String()) - protocol = utiliptables.ProtocolIPv6 - } else { - klog.V(0).Infof("kube-proxy node IP is an IPv4 address (%s), assume IPv4 operation", nodeIP.String()) - } - - iptInterface = utiliptables.New(execer, protocol) + klog.Infof("Detected node IP %s", nodeIP.String()) // Create event recorder eventBroadcaster := record.NewBroadcaster() @@ -181,6 +173,38 @@ func newProxyServer( klog.V(2).Info("DetectLocalMode: '", string(detectLocalMode), "'") + primaryProtocol := utiliptables.ProtocolIPv4 + if utilsnet.IsIPv6(nodeIP) { + primaryProtocol = utiliptables.ProtocolIPv6 + } + iptInterface = utiliptables.New(execer, primaryProtocol) + + var ipt [2]utiliptables.Interface + dualStack := utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) && proxyMode != proxyModeUserspace + if dualStack { + // Create iptables handlers for both families, one is already created + // Always ordered as IPv4, IPv6 + if primaryProtocol == utiliptables.ProtocolIPv4 { + ipt[0] = iptInterface + ipt[1] = utiliptables.New(execer, utiliptables.ProtocolIPv6) + + // Just because the feature gate is enabled doesn't mean the node + // actually supports dual-stack + if _, err := ipt[1].ChainExists(utiliptables.TableNAT, utiliptables.ChainPostrouting); err != nil { + klog.Warningf("No iptables support for IPv6: %v", err) + dualStack = false + } + } else { + ipt[0] = utiliptables.New(execer, utiliptables.ProtocolIPv4) + ipt[1] = iptInterface + } + } + if dualStack { + klog.V(0).Infof("kube-proxy running in dual-stack mode, %s-primary", iptInterface.Protocol()) + } else { + klog.V(0).Infof("kube-proxy running in single-stack %s mode", iptInterface.Protocol()) + } + if proxyMode == proxyModeIPTables { klog.V(0).Info("Using iptables Proxier.") if config.IPTables.MasqueradeBit == nil { @@ -188,20 +212,9 @@ func newProxyServer( return nil, fmt.Errorf("unable to read IPTables MasqueradeBit from config") } - if utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) { + if dualStack { klog.V(0).Info("creating dualStackProxier for iptables.") - // Create iptables handlers for both families, one is already created - // Always ordered as IPv4, IPv6 - var ipt [2]utiliptables.Interface - if iptInterface.IsIPv6() { - ipt[1] = iptInterface - ipt[0] = utiliptables.New(execer, utiliptables.ProtocolIPv4) - } else { - ipt[0] = iptInterface - ipt[1] = utiliptables.New(execer, utiliptables.ProtocolIPv6) - } - // Always ordered to match []ipt var localDetectors [2]proxyutiliptables.LocalTrafficDetector localDetectors, err = getDualStackLocalDetectorTuple(detectLocalMode, config, ipt, nodeInfo) @@ -256,20 +269,9 @@ func newProxyServer( proxymetrics.RegisterMetrics() } else if proxyMode == proxyModeIPVS { klog.V(0).Info("Using ipvs Proxier.") - if utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) { + if dualStack { klog.V(0).Info("creating dualStackProxier for ipvs.") - // Create iptables handlers for both families, one is already created - // Always ordered as IPv4, IPv6 - var ipt [2]utiliptables.Interface - if iptInterface.IsIPv6() { - ipt[1] = iptInterface - ipt[0] = utiliptables.New(execer, utiliptables.ProtocolIPv4) - } else { - ipt[0] = iptInterface - ipt[1] = utiliptables.New(execer, utiliptables.ProtocolIPv6) - } - nodeIPs := nodeIPTuple(config.BindAddress) // Always ordered to match []ipt diff --git a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go index 9d19e90e05e..e4715c2be58 100644 --- a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go +++ b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go @@ -123,6 +123,14 @@ func (f *fakeIPTables) DeleteChain(tableName utiliptables.Table, chainName utili return nil } +func (f *fakeIPTables) ChainExists(tableName utiliptables.Table, chainName utiliptables.Chain) (bool, error) { + _, _, err := f.getChain(tableName, chainName) + if err != nil { + return false, err + } + return true, nil +} + // Returns index of rule in array; < 0 if rule is not found func findRule(chain *fakeChain, rule string) int { for i, candidate := range chain.rules { diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 69c262bb345..d8c5cc67054 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -51,6 +51,9 @@ type Interface interface { FlushChain(table Table, chain Chain) error // DeleteChain deletes the specified chain. If the chain did not exist, return error. DeleteChain(table Table, chain Chain) error + // ChainExists tests whether the specified chain exists, returning an error if it + // does not, or if it is unable to check. + ChainExists(table Table, chain Chain) (bool, error) // EnsureRule checks if the specified rule is present and, if not, creates it. If the rule existed, return true. EnsureRule(position RulePosition, table Table, chain Chain, args ...string) (bool, error) // DeleteRule checks if the specified rule is present and, if so, deletes it. @@ -570,7 +573,7 @@ func (runner *runner) Monitor(canary Chain, tables []Table, reloadFunc func(), i // Poll until stopCh is closed or iptables is flushed err := utilwait.PollUntil(interval, func() (bool, error) { - if exists, err := runner.chainExists(tables[0], canary); exists { + if exists, err := runner.ChainExists(tables[0], canary); exists { return false, nil } else if isResourceError(err) { klog.Warningf("Could not check for iptables canary %s/%s: %v", string(tables[0]), string(canary), err) @@ -582,7 +585,7 @@ func (runner *runner) Monitor(canary Chain, tables []Table, reloadFunc func(), i // 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 := runner.chainExists(tables[i], canary); exists || isResourceError(err) { + if exists, err := runner.ChainExists(tables[i], canary); exists || isResourceError(err) { return false, nil } } @@ -607,15 +610,14 @@ func (runner *runner) Monitor(canary Chain, tables []Table, reloadFunc func(), i } } -// chainExists is used internally by Monitor; none of the public Interface methods can be -// used to distinguish "chain exists" from "chain does not exist" with no side effects -func (runner *runner) chainExists(table Table, chain Chain) (bool, error) { +// ChainExists is part of Interface +func (runner *runner) ChainExists(table Table, chain Chain) (bool, error) { fullArgs := makeFullArgs(table, chain) runner.mu.Lock() defer runner.mu.Unlock() - trace := utiltrace.New("iptables Monitor CANARY check") + trace := utiltrace.New("iptables ChainExists") defer trace.LogIfLong(2 * time.Second) _, err := runner.run(opListChain, fullArgs) diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index b0ac31e7109..cff190630cd 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -93,6 +93,11 @@ func (*FakeIPTables) DeleteChain(table iptables.Table, chain iptables.Chain) err return nil } +// ChainExists is part of iptables.Interface +func (*FakeIPTables) ChainExists(table iptables.Table, chain iptables.Chain) (bool, error) { + return true, nil +} + // EnsureRule is part of iptables.Interface func (*FakeIPTables) EnsureRule(position iptables.RulePosition, table iptables.Table, chain iptables.Chain, args ...string) (bool, error) { return true, nil