Merge pull request #95378 from danwinship/fix-dual-stack-canaries

fix kubelet iptables monitoring on dual-stack

pkg/kubelet/kubelet_network_linux.go

@@ -40,21 +40,29 @@ func (kl *Kubelet) initNetworkUtil() {
 	ipv6Primary := kl.nodeIPs != nil && utilnet.IsIPv6(kl.nodeIPs[0])
 
 	var iptClients []utiliptables.Interface
+	var protocols []utiliptables.Protocol
 	if maybeDualStack || !ipv6Primary {
+		protocols = append(protocols, utiliptables.ProtocolIPv4)
 		iptClients = append(iptClients, utiliptables.New(exec, utiliptables.ProtocolIPv4))
 	}
 	if maybeDualStack || ipv6Primary {
+		protocols = append(protocols, utiliptables.ProtocolIPv6)
 		iptClients = append(iptClients, utiliptables.New(exec, utiliptables.ProtocolIPv6))
 	}
 
-	for _, iptClient := range iptClients {
-		kl.syncNetworkUtil(iptClient)
+	for i := range iptClients {
+		iptClient := iptClients[i]
+		if kl.syncNetworkUtil(iptClient) {
+			klog.Infof("Initialized %s iptables rules.", protocols[i])
 			go iptClient.Monitor(
 				utiliptables.Chain("KUBE-KUBELET-CANARY"),
 				[]utiliptables.Table{utiliptables.TableMangle, utiliptables.TableNAT, utiliptables.TableFilter},
 				func() { kl.syncNetworkUtil(iptClient) },
 				1*time.Minute, wait.NeverStop,
 			)
+		} else {
+			klog.Warningf("Failed to initialize %s iptables rules; some functionality may be missing.", protocols[i])
+		}
 	}
 }
 
@@ -64,27 +72,27 @@ func (kl *Kubelet) initNetworkUtil() {
 // 	Marked connection will be drop on INPUT/OUTPUT Chain in filter table
 // 2. 	In nat table, KUBE-MARK-MASQ rule to mark connections for SNAT
 // 	Marked connection will get SNAT on POSTROUTING Chain in nat table
-func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) {
+func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) bool {
 	// Setup KUBE-MARK-DROP rules
 	dropMark := getIPTablesMark(kl.iptablesDropBit)
 	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubeMarkDropChain); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s exists: %v", utiliptables.TableNAT, KubeMarkDropChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubeMarkDropChain, "-j", "MARK", "--or-mark", dropMark); err != nil {
 		klog.Errorf("Failed to ensure marking rule for %v: %v", KubeMarkDropChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureChain(utiliptables.TableFilter, KubeFirewallChain); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s exists: %v", utiliptables.TableFilter, KubeFirewallChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableFilter, KubeFirewallChain,
 		"-m", "comment", "--comment", "kubernetes firewall for dropping marked packets",
 		"-m", "mark", "--mark", fmt.Sprintf("%s/%s", dropMark, dropMark),
 		"-j", "DROP"); err != nil {
 		klog.Errorf("Failed to ensure rule to drop packet marked by %v in %v chain %v: %v", KubeMarkDropChain, utiliptables.TableFilter, KubeFirewallChain, err)
-		return
+		return false
 	}
 
 	// drop all non-local packets to localhost if they're not part of an existing
@@ -98,37 +106,37 @@ func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) {
 			"!", "--ctstate", "RELATED,ESTABLISHED,DNAT",
 			"-j", "DROP"); err != nil {
 			klog.Errorf("Failed to ensure rule to drop invalid localhost packets in %v chain %v: %v", utiliptables.TableFilter, KubeFirewallChain, err)
-			return
+			return false
 		}
 	}
 
 	if _, err := iptClient.EnsureRule(utiliptables.Prepend, utiliptables.TableFilter, utiliptables.ChainOutput, "-j", string(KubeFirewallChain)); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s jumps to %s: %v", utiliptables.TableFilter, utiliptables.ChainOutput, KubeFirewallChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Prepend, utiliptables.TableFilter, utiliptables.ChainInput, "-j", string(KubeFirewallChain)); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s jumps to %s: %v", utiliptables.TableFilter, utiliptables.ChainInput, KubeFirewallChain, err)
-		return
+		return false
 	}
 
 	// Setup KUBE-MARK-MASQ rules
 	masqueradeMark := getIPTablesMark(kl.iptablesMasqueradeBit)
 	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubeMarkMasqChain); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s exists: %v", utiliptables.TableNAT, KubeMarkMasqChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubePostroutingChain); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s exists: %v", utiliptables.TableNAT, KubePostroutingChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubeMarkMasqChain, "-j", "MARK", "--or-mark", masqueradeMark); err != nil {
 		klog.Errorf("Failed to ensure marking rule for %v: %v", KubeMarkMasqChain, err)
-		return
+		return false
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Prepend, utiliptables.TableNAT, utiliptables.ChainPostrouting,
 		"-m", "comment", "--comment", "kubernetes postrouting rules", "-j", string(KubePostroutingChain)); err != nil {
 		klog.Errorf("Failed to ensure that %s chain %s jumps to %s: %v", utiliptables.TableNAT, utiliptables.ChainPostrouting, KubePostroutingChain, err)
-		return
+		return false
 	}
 
 	// Set up KUBE-POSTROUTING to unmark and masquerade marked packets
@@ -138,7 +146,7 @@ func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) {
 		"-m", "mark", "!", "--mark", fmt.Sprintf("%s/%s", masqueradeMark, masqueradeMark),
 		"-j", "RETURN"); err != nil {
 		klog.Errorf("Failed to ensure filtering rule for %v: %v", KubePostroutingChain, err)
-		return
+		return false
 	}
 	// Clear the mark to avoid re-masquerading if the packet re-traverses the network stack.
 	// We know the mark bit is currently set so we can use --xor-mark to clear it (without needing
@@ -146,7 +154,7 @@ func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) {
 	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain,
 		"-j", "MARK", "--xor-mark", masqueradeMark); err != nil {
 		klog.Errorf("Failed to ensure unmarking rule for %v: %v", KubePostroutingChain, err)
-		return
+		return false
 	}
 	masqRule := []string{
 		"-m", "comment", "--comment", "kubernetes service traffic requiring SNAT",
@@ -157,8 +165,10 @@ func (kl *Kubelet) syncNetworkUtil(iptClient utiliptables.Interface) {
 	}
 	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain, masqRule...); err != nil {
 		klog.Errorf("Failed to ensure SNAT rule for packets marked by %v in %v chain %v: %v", KubeMarkMasqChain, utiliptables.TableNAT, KubePostroutingChain, err)
-		return
+		return false
 	}
+
+	return true
 }
 
 // getIPTablesMark returns the fwmark given the bit

test/e2e/network/networking.go

@@ -477,10 +477,10 @@ var _ = SIGDescribe("Networking", func() {
 		// restart iptables"?). So instead we just manually delete all "KUBE-"
 		// chains.
 
-		ginkgo.By("dumping iptables rules on a node")
+		ginkgo.By("dumping iptables rules on node " + host)
 		result, err := e2essh.SSH("sudo iptables-save", host, framework.TestContext.Provider)
-		if err != nil || result.Code != 0 {
 		e2essh.LogResult(result)
+		if err != nil || result.Code != 0 {
 			framework.Failf("couldn't dump iptable rules: %v", err)
 		}
 
@@ -532,6 +532,9 @@ var _ = SIGDescribe("Networking", func() {
 			}
 			return false, nil
 		})
+		if err != nil {
+			e2essh.LogResult(result)
+		}
 		framework.ExpectNoError(err, "kubelet did not recreate its iptables rules")
 	})
 })