From 48e0c1ea8795fb6495a24b0e4dc4441c6f85091f Mon Sep 17 00:00:00 2001 From: Kay Yan Date: Tue, 20 Dec 2022 11:29:24 +0800 Subject: [PATCH] fix-the-auto-iptables-detection --- felix/environment/feature_detect.go | 60 +++++++++++----- felix/environment/feature_detect_test.go | 88 ++++++++++++++++++++---- 2 files changed, 114 insertions(+), 34 deletions(-) diff --git a/felix/environment/feature_detect.go b/felix/environment/feature_detect.go index 797707cc1e2..5d9ce1b745c 100644 --- a/felix/environment/feature_detect.go +++ b/felix/environment/feature_detect.go @@ -291,34 +291,56 @@ func countRulesInIptableOutput(in []byte) int { return count } +// hasKubernetesChains tries to find in the output of the binary if the Kubernetes +// chains exists +func hasKubernetesChains(output []byte) bool { + return strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") +} + // GetIptablesBackend attempts to detect the iptables backend being used where Felix is running. // This code is duplicating the detection method found at -// https://github.com/kubernetes/kubernetes/blob/623b6978866b5d3790d17ff13601ef9e7e4f4bf0/build/debian-iptables/iptables-wrapper#L28 +// https://github.com/kubernetes-sigs/iptables-wrappers/blob/master/iptables-wrapper-installer.sh#L107 // If there is a specifiedBackend then it is used but if it does not match the detected // backend then a warning is logged. func DetectBackend(lookPath func(file string) (string, error), newCmd cmdshim.CmdFactory, specifiedBackend string) string { - ip6LgcySave := FindBestBinary(lookPath, 6, "legacy", "save") - ip4LgcySave := FindBestBinary(lookPath, 4, "legacy", "save") - ip6l, _ := newCmd(ip6LgcySave).Output() - ip4l, _ := newCmd(ip4LgcySave).Output() - log.WithField("ip6l", string(ip6l)).Debug("Ip6tables legacy save out") - log.WithField("ip4l", string(ip4l)).Debug("Iptables legacy save out") - legacyLines := countRulesInIptableOutput(ip6l) + countRulesInIptableOutput(ip4l) + ip6NftSave := FindBestBinary(lookPath, 6, "nft", "save") + ip4NftSave := FindBestBinary(lookPath, 4, "nft", "save") + + ip6nm, _ := newCmd(ip6NftSave, "-t", "mangle").Output() + log.WithField("ip6n", string(ip6nm)).Debug("Ip6tables save out") + ip4nm, _ := newCmd(ip4NftSave, "-t", "mangle").Output() + log.WithField("ip4n", string(ip4nm)).Debug("Iptables save out") + var detectedBackend string - if legacyLines >= 10 { - detectedBackend = "legacy" + if hasKubernetesChains(ip6nm) || hasKubernetesChains(ip4nm) { + detectedBackend = "nft" } else { - ip6NftSave := FindBestBinary(lookPath, 6, "nft", "save") - ip4NftSave := FindBestBinary(lookPath, 4, "nft", "save") - ip6n, _ := newCmd(ip6NftSave).Output() - log.WithField("ip6n", string(ip6n)).Debug("Ip6tables save out") - ip4n, _ := newCmd(ip4NftSave).Output() - log.WithField("ip4n", string(ip4n)).Debug("Iptables save out") - nftLines := countRulesInIptableOutput(ip6n) + countRulesInIptableOutput(ip4n) - if legacyLines >= nftLines { + ip6LgcySave := FindBestBinary(lookPath, 6, "legacy", "save") + ip4LgcySave := FindBestBinary(lookPath, 4, "legacy", "save") + ip6lm, _ := newCmd(ip6LgcySave, "-t", "mangle").Output() + log.WithField("ip6l", string(ip6lm)).Debug("Ip6tables legacy save -t mangle out") + ip4lm, _ := newCmd(ip4LgcySave, "-t", "mangle").Output() + log.WithField("ip4l", string(ip4lm)).Debug("Iptables legacy save -t mangle out") + + if hasKubernetesChains(ip6lm) || hasKubernetesChains(ip4lm) { detectedBackend = "legacy" } else { - detectedBackend = "nft" + ip6l, _ := newCmd(ip6LgcySave).Output() + log.WithField("ip6l", string(ip6l)).Debug("Ip6tables legacy save out") + ip4l, _ := newCmd(ip4LgcySave).Output() + log.WithField("ip4l", string(ip4l)).Debug("Iptables legacy save out") + legacyLines := countRulesInIptableOutput(ip6l) + countRulesInIptableOutput(ip4l) + + ip6n, _ := newCmd(ip6NftSave).Output() + log.WithField("ip6n", string(ip6n)).Debug("Ip6tables save out") + ip4n, _ := newCmd(ip4NftSave).Output() + log.WithField("ip4n", string(ip4n)).Debug("Iptables save out") + nftLines := countRulesInIptableOutput(ip6n) + countRulesInIptableOutput(ip4n) + if legacyLines >= nftLines { + detectedBackend = "legacy" // default to legacy mode + } else { + detectedBackend = "nft" + } } } log.WithField("detectedBackend", detectedBackend).Debug("Detected Iptables backend") diff --git a/felix/environment/feature_detect_test.go b/felix/environment/feature_detect_test.go index 2f72cd2b23e..ac843070357 100644 --- a/felix/environment/feature_detect_test.go +++ b/felix/environment/feature_detect_test.go @@ -242,43 +242,43 @@ func TestIptablesBackendDetection(t *testing.T) { { "No output from cmds", "auto", - ipOutputFactory{0, 0, 0, 0}, + ipOutputFactory{0, 0, 0, 0, 0, 0, 0, 0}, "legacy", }, { "Output from legacy cmds", "auto", - ipOutputFactory{10, 10, 0, 0}, + ipOutputFactory{10, 10, 0, 0, 0, 0, 0, 0}, "legacy", }, { "Output from nft cmds", "auto", - ipOutputFactory{0, 0, 10, 10}, + ipOutputFactory{0, 0, 10, 10, 0, 0, 0, 0}, "nft", }, { "Detected and Specified backend of nft match", "nft", - ipOutputFactory{0, 0, 10, 10}, + ipOutputFactory{0, 0, 10, 10, 0, 0, 0, 0}, "nft", }, { "Detected and Specified backend of legacy match", "legacy", - ipOutputFactory{10, 10, 0, 0}, + ipOutputFactory{10, 10, 0, 0, 0, 0, 0, 0}, "legacy", }, { "Backend detected as nft does not match Specified legacy", "legacy", - ipOutputFactory{0, 0, 10, 10}, + ipOutputFactory{0, 0, 10, 10, 0, 0, 0, 0}, "legacy", }, { "Backend detected as legacy does not match Specified nft", "nft", - ipOutputFactory{10, 10, 0, 0}, + ipOutputFactory{10, 10, 0, 0, 0, 0, 0, 0}, "nft", }, { @@ -301,7 +301,7 @@ func TestIptablesBackendDetection(t *testing.T) { Ip6Nft: 10, Ip4Nft: 10, }, - "legacy", + "nft", }, { "Only ipv6 output from legacy cmds", @@ -312,7 +312,7 @@ func TestIptablesBackendDetection(t *testing.T) { Ip6Nft: 10, Ip4Nft: 10, }, - "legacy", + "nft", }, { "Only ipv6 output from nft cmds still detects nft", @@ -325,6 +325,50 @@ func TestIptablesBackendDetection(t *testing.T) { }, "nft", }, + { + "Output from nft with kube chains", + "auto", + ipOutputFactory{ + Ip6legacy: 0, + Ip4legacy: 0, + Ip6Nft: 64, + Ip4Nft: 123, + Ip6legacyKube: 0, + Ip4legacyKube: 0, + Ip6NftKube: 2, + Ip4NftKube: 2, + }, + "nft", + }, + { + "Output from nft with kube chains and has legacy chains", + "auto", + ipOutputFactory{ + Ip6legacy: 20, + Ip4legacy: 20, + Ip6Nft: 2, + Ip4Nft: 2, + Ip6legacyKube: 0, + Ip4legacyKube: 0, + Ip6NftKube: 2, + Ip4NftKube: 2, + }, + "nft", + }, { + "Output from legacy with kube chains and has nft chains", + "auto", + ipOutputFactory{ + Ip6legacy: 20, + Ip4legacy: 20, + Ip6Nft: 30, + Ip4Nft: 30, + Ip6legacyKube: 2, + Ip4legacyKube: 2, + Ip6NftKube: 0, + Ip4NftKube: 0, + }, + "legacy", + }, } { tst := tst t.Run("DetectingBackend, testing "+tst.name, func(t *testing.T) { @@ -341,24 +385,30 @@ type ipOutputFactory struct { Ip4legacy int Ip6Nft int Ip4Nft int + + Ip6legacyKube int + Ip4legacyKube int + Ip6NftKube int + Ip4NftKube int } func (f *ipOutputFactory) NewCmd(name string, arg ...string) cmdshim.CmdIface { switch name { case "iptables-legacy-save": - return &ipOutputCmd{out: f.Ip4legacy} + return &ipOutputCmd{out: f.Ip4legacy, outKube: f.Ip4legacyKube} case "ip6tables-legacy-save": - return &ipOutputCmd{out: f.Ip6legacy} + return &ipOutputCmd{out: f.Ip6legacy, outKube: f.Ip6legacyKube} case "iptables-nft-save": - return &ipOutputCmd{out: f.Ip4Nft} + return &ipOutputCmd{out: f.Ip4Nft, outKube: f.Ip4NftKube} case "ip6tables-nft-save": - return &ipOutputCmd{out: f.Ip6Nft} + return &ipOutputCmd{out: f.Ip6Nft, outKube: f.Ip6NftKube} } return nil } type ipOutputCmd struct { - out int + out int + outKube int } func (d *ipOutputCmd) String() string { @@ -393,10 +443,18 @@ func (d *ipOutputCmd) Output() ([]byte, error) { if d.out < 0 { return nil, errors.New("iptables command failed") } + if d.outKube > d.out { + return nil, errors.New("iptables command failed") + } + out := []byte{} - for i := 0; i < d.out; i++ { + for i := 0; i < d.outKube; i++ { + out = append(out, []byte(fmt.Sprintf("KUBE-IPTABLES-HINT - [0:0] %d\n", i))...) + } + for i := 0; i < d.out-d.outKube; i++ { out = append(out, []byte(fmt.Sprintf("-Output line %d\n", i))...) } + return out, nil }