fix: [NPM] [LINUX] iptables-nft segfault at high scale

Signed-off-by: Hunter Gregory <[email protected]>

Author: huntergregory

npm/examples/azure-npm-test-deployment.yaml

@@ -0,0 +1,88 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: azure-npm
+  namespace: kube-system
+  labels:
+    app: azure-npm
+    addonmanager.kubernetes.io/mode: EnsureExists
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      k8s-app: azure-npm
+  template:
+    metadata:
+      labels:
+        k8s-app: azure-npm
+      annotations:
+        scheduler.alpha.kubernetes.io/critical-pod: ""
+        azure.npm/scrapeable: ""
+    spec:
+      priorityClassName: system-node-critical
+      tolerations:
+        - operator: "Exists"
+          effect: NoExecute
+        - operator: "Exists"
+          effect: NoSchedule
+        - key: CriticalAddonsOnly
+          operator: Exists
+      containers:
+        - name: azure-npm
+          image: acnpublic.azurecr.io/azure-npm:11-14-2024-test-nft-commands-only
+          resources:
+            limits:
+              cpu: 250m
+              memory: 400Mi
+            requests:
+              cpu: 50m
+              memory: 300Mi
+          securityContext:
+            privileged: false
+            capabilities:
+              add:
+              - NET_ADMIN
+            readOnlyRootFilesystem: true
+          env:
+            - name: HOSTNAME
+              valueFrom:
+                fieldRef:
+                  apiVersion: v1
+                  fieldPath: spec.nodeName
+            - name: NPM_CONFIG
+              value: /etc/azure-npm/azure-npm.json
+          volumeMounts:
+            - name: log
+              mountPath: /var/log
+            - name: xtables-lock
+              mountPath: /run/xtables.lock
+            - name: protocols
+              mountPath: /etc/protocols
+            - name: azure-npm-config
+              mountPath: /etc/azure-npm
+            - name: tmp
+              mountPath: /tmp
+      hostNetwork: true
+      hostUsers: false
+      nodeSelector:
+        kubernetes.io/os: linux
+        azure-npm-debug: "true"
+      volumes:
+        - name: log
+          hostPath:
+            path: /var/log
+            type: Directory
+        - name: xtables-lock
+          hostPath:
+            path: /run/xtables.lock
+            type: File
+        - name: protocols
+          hostPath:
+            path: /etc/protocols
+            type: File
+        - name: azure-npm-config
+          configMap:
+            name: azure-npm-config
+        - name: tmp
+          emptyDir: {}
+      serviceAccountName: azure-npm

npm/util/const.go

@@ -4,10 +4,13 @@ package util
 
 import (
 	"bytes"
-	"fmt"
-	"strings"
+	"os"
+	"time"
 
 	"github.com/Azure/azure-container-networking/common"
+
+	"k8s.io/klog"
+	utilexec "k8s.io/utils/exec"
 )
 
 // kubernetes related constants.
@@ -273,61 +276,42 @@ const (
 )
 
 func DetectIptablesVersion(ioShim *common.IOShim) {
-	cmd := ioShim.Exec.Command(IptablesSaveNft, "-t", "mangle")
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		fmt.Printf("Error running iptables-nft-save: %s", err)
+	listChainArgs := []string{"-w", "60", "-t", "mangle", "-n", "-L"}
+	hintArgs := make([]string, 0, len(listChainArgs)+1)
+	hintArgs = append(hintArgs, listChainArgs...)
+	hintArgs = append(hintArgs, "KUBE-IPTABLES-HINT")
+	canaryArgs := make([]string, 0, len(listChainArgs)+1)
+	canaryArgs = append(canaryArgs, listChainArgs...)
+	canaryArgs = append(canaryArgs, "KUBE-KUBELET-CANARY")
+
+	cmdHint := ioShim.Exec.Command(IptablesNft, hintArgs...)
+	cmdCanary := ioShim.Exec.Command(IptablesNft, canaryArgs...)
+
+	hintOutput, hintErr := cmdHint.CombinedOutput()
+	canaryOutput, canaryErr := cmdCanary.CombinedOutput()
+	if hintErr != nil && canaryErr != nil {
+		klog.Infof("DEBUGME: iptables-nft hint failed. hintErr: %s. canaryErr: %s", hintErr.Error(), canaryErr.Error())
 		return
 	}
 
-	if strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") {
-		Iptables = IptablesNft
-		IptablesSave = IptablesSaveNft
-		IptablesRestore = IptablesRestoreNft
-	} else {
-		lCmd := ioShim.Exec.Command(IptablesSaveLegacy, "-t", "mangle")
-
-		loutput, err := lCmd.CombinedOutput()
-		if err != nil {
-			fmt.Printf("Error running iptables-legacy-save: %s", err)
-			return
-		}
-
-		if strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") {
-			Iptables = IptablesLegacy
-			IptablesSave = IptablesSaveLegacy
-			IptablesRestore = IptablesRestoreLegacy
-		} else {
-			lsavecmd := ioShim.Exec.Command(IptablesSaveNft)
-			lsaveoutput, err := lsavecmd.CombinedOutput()
-			if err != nil {
-				fmt.Printf("Error running iptables-nft-save: %s", err)
-				return
-			}
-
-			lcount := countLines(lsaveoutput)
-
-			savecmd := ioShim.Exec.Command(IptablesSaveLegacy)
-			saveoutput, err := savecmd.CombinedOutput()
-			if err != nil {
-				fmt.Printf("Error running iptables-legacy-save: %s", err)
-				return
-			}
-
-			count := countLines(saveoutput)
-
-			if lcount > count {
-				Iptables = IptablesLegacy
-				IptablesSave = IptablesSaveLegacy
-				IptablesRestore = IptablesRestoreLegacy
-			} else {
-				Iptables = IptablesNft
-				IptablesSave = IptablesSaveNft
-				IptablesRestore = IptablesRestoreNft
-			}
-		}
+	if hintErr != nil {
+		klog.Infof("DEBUGME: iptables-nft hint failed: %v", hintErr)
+	} else if canaryErr != nil {
+		klog.Infof("DEBUGME: iptables-nft canary failed: %v", canaryErr)
 	}
+
+	klog.Info("DEBUGME: found matches")
+
+	klog.Info("DEBUGME: line count (hint): %d", len(bytes.Split(hintOutput, []byte("\n"))))
+	klog.Info("DEBUGME: line count (canary): %d", len(bytes.Split(canaryOutput, []byte("\n"))))
+
+	Iptables = IptablesNft
+	IptablesSave = IptablesSaveNft
+	IptablesRestore = IptablesRestoreNft
+
+	time.Sleep(30 * time.Second)
+	klog.Info("DEBUGME: crashing for test image after running iptables-nft commands")
+	os.Exit(1)
 }
 
 func countLines(output []byte) int {
@@ -339,3 +323,32 @@ func countLines(output []byte) int {
 	}
 	return count
 }
+
+func pipeCommandToGrep(command, grepCommand utilexec.Cmd) (searchResults []byte, gotMatches bool, commandError error) {
+	pipe, commandError := command.StdoutPipe()
+	if commandError != nil {
+		return
+	}
+	closePipe := func() { _ = pipe.Close() } // appease go lint
+	defer closePipe()
+
+	grepCommand.SetStdin(pipe)
+	commandError = command.Start()
+	if commandError != nil {
+		return
+	}
+
+	// Without this wait, defunct iptable child process are created
+	wait := func() { _ = command.Wait() } // appease go lint
+	defer wait()
+
+	output, err := grepCommand.CombinedOutput()
+	if err != nil {
+		// grep returns err status 1 if nothing is found
+		// but the other command's exit status gets propagated through this CombinedOutput, so we might have errors undetected
+		return
+	}
+	searchResults = output
+	gotMatches = true
+	return
+}