fix: [NPM] [Linux] improve iptables version detection and cleanup

go.mod

@@ -130,6 +130,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor v0.11.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v5 v5.2.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
+	github.com/zcalusic/sysinfo v1.1.2
 	golang.org/x/sync v0.8.0
 	gotest.tools/v3 v3.5.1
 	k8s.io/kubectl v0.28.5

go.sum

@@ -294,6 +294,8 @@ github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZla
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
+github.com/zcalusic/sysinfo v1.1.2 h1:38KUgZQmCxlN9vUTt4miis4rU5ISJXGXOJ2rY7bMC8g=
+github.com/zcalusic/sysinfo v1.1.2/go.mod h1:NX+qYnWGtJVPV0yWldff9uppNKU4h40hJIRPf/pGLv4=
 go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
 go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=

npm/pkg/dataplane/dataplane_linux.go

@@ -1,11 +1,25 @@
 package dataplane
 
 import (
+	"errors"
+	"fmt"
+	"strconv"
+	"strings"
+
+	"github.com/Azure/azure-container-networking/common"
+	"github.com/Azure/azure-container-networking/npm/metrics"
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
 	"github.com/Azure/azure-container-networking/npm/util"
 	npmerrors "github.com/Azure/azure-container-networking/npm/util/errors"
+
+	"github.com/zcalusic/sysinfo"
+	"k8s.io/klog"
 )
 
+const detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry"
+
+var errDetectingIptablesVersion = errors.New(detectingErrMsg)
+
 func (dp *DataPlane) getEndpointsToApplyPolicies(_ []*policies.NPMNetworkPolicy) (map[string]string, error) {
 	// NOOP in Linux
 	return nil, nil
@@ -21,7 +35,9 @@
 }
 
 func (dp *DataPlane) bootupDataPlane() error {
-	util.DetectIptablesVersion(dp.ioShim)
+	if err := detectIptablesVersion(dp.ioShim); err != nil {
+		return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to detect iptables version", err)
+	}
 
 	// It is important to keep order to clean-up ACLs before ipsets. Otherwise we won't be able to delete ipsets referenced by ACLs
 	if err := dp.policyMgr.Bootup(nil); err != nil {
@@ -37,3 +53,81 @@
 	// NOOP in Linux
 	return nil
 }
+
+// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected.
+// NPM will crash if it fails to detect either.
+// This global variable is referenced in all iptables related functions.
+func detectIptablesVersion(ioShim *common.IOShim) error {
+	klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle")
+	cmd := ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle")
+	output, err := cmd.CombinedOutput()
+	if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") {
+		msg := "detected iptables version on first attempt. found KUBE chains in nft tables. NPM will use iptables-nft"
+		klog.Info(msg)
+		metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
+		util.Iptables = util.IptablesNft
+		util.IptablesSave = util.IptablesSaveNft
+		util.IptablesRestore = util.IptablesRestoreNft
+		return nil
+	}
+
+	if err != nil {
+		msg := fmt.Sprintf("failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %w", err)
+		klog.Info(msg)
+		metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
+	}
+
+	klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle")
+	lCmd := ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle")
+	loutput, err := lCmd.CombinedOutput()
+	if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") {
+		msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy"
+		klog.Info(msg)
+		metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
+		util.Iptables = util.IptablesLegacy
+		util.IptablesSave = util.IptablesSaveLegacy
+		util.IptablesRestore = util.IptablesRestoreLegacy
+		return nil
+	}
+
+	if err != nil {
+		msg := fmt.Sprintf("failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %w", err)
+		klog.Info(msg)
+		metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
+	}
+
+	klog.Info("third attempt detecting iptables version. getting kernel version")
+	var si sysinfo.SysInfo
+	si.GetSysInfo()
+	kernelVersion := strings.Split(si.Kernel.Release, ".")
+	if kernelVersion[0] == "" {
+		msg := fmt.Sprintf("failed to detect iptables version on third attempt. error getting kernel version. err: %w", err)
+		klog.Info(msg)
+		metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
+		return errDetectingIptablesVersion
+	}
+
+	majorVersion, err := strconv.Atoi(kernelVersion[0])
+	if err != nil {
+		msg := fmt.Sprintf("failed to detect iptables version on third attempt. error converting kernel version to int. err: %w", err)
+		klog.Info(msg)
+		metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
+		return errDetectingIptablesVersion
+	}
+
+	if majorVersion >= 5 {
+		msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft"
+		klog.Info(msg)
+		metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
+		util.Iptables = util.IptablesNft
+		util.IptablesSave = util.IptablesSaveNft
+		util.IptablesRestore = util.IptablesRestoreNft
+		return nil
+	}
+
+	msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy"
+	klog.Info(msg)
+	metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
+
+	return nil
+}

npm/util/const.go

@@ -2,14 +2,6 @@
 // MIT License
 package util
 
-import (
-	"bytes"
-	"fmt"
-	"strings"
-
-	"github.com/Azure/azure-container-networking/common"
-)
-
 // kubernetes related constants.
 const (
 	KubeSystemFlag             string = "kube-system"
@@ -271,71 +263,3 @@ const (
 	DaemonDataplaneID // for v2
 	FanOutServerID    // for v2
 )
-
-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)
-		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
-			}
-		}
-	}
-}
-
-func countLines(output []byte) int {
-	count := 0
-	for _, x := range bytes.Split(output, []byte("\n")) {
-		if len(x) >= 1 && x[0] == '-' {
-			count++
-		}
-	}
-	return count
-}