From b95d2e431280323842b01f2059d41f51a19ba78a Mon Sep 17 00:00:00 2001 From: Qingchuan Hao Date: Tue, 29 Oct 2024 09:43:02 +0000 Subject: [PATCH 1/3] ignore know copy failure and fix iptables issue Signed-off-by: Qingchuan Hao --- pkg/capture/provider/network_capture_unix.go | 65 +++++++++++++++----- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/pkg/capture/provider/network_capture_unix.go b/pkg/capture/provider/network_capture_unix.go index b0522e0926..406539012b 100644 --- a/pkg/capture/provider/network_capture_unix.go +++ b/pkg/capture/provider/network_capture_unix.go @@ -188,15 +188,16 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(filter string, duration, } type command struct { - name string - args []string - description string + name string + args []string + description string + ignoreFailure bool } func (ncp *NetworkCaptureProvider) CollectMetadata() error { ncp.l.Info("Start to collect network metadata") - iptablesMode := obtainIptablesMode() + iptablesMode := obtainIptablesMode(ncp.l) ncp.l.Info(fmt.Sprintf("Iptables mode %s is used", iptablesMode)) iptablesSaveCmdName := fmt.Sprintf("iptables-%s-save", iptablesMode) iptablesCmdName := fmt.Sprintf("iptables-%s", iptablesMode) @@ -282,9 +283,10 @@ func (ncp *NetworkCaptureProvider) CollectMetadata() error { description: "networking stats", }, { - name: "cp", - args: []string{"-r", "/proc/sys/net", filepath.Join(ncp.TmpCaptureDir, "proc-sys-net")}, - description: "kernel networking configuration", + name: "cp", + args: []string{"-r", "/proc/sys/net", filepath.Join(ncp.TmpCaptureDir, "proc-sys-net")}, + description: "kernel networking configuration", + ignoreFailure: true, }, }, }, @@ -320,14 +322,15 @@ func (ncp *NetworkCaptureProvider) CollectMetadata() error { } // Write command stdout and stderr to output file - for _, cmd := range cmds { + for _, command := range metadata.commands { + cmd := exec.Command(command.name, command.args...) if _, err := outfile.WriteString(fmt.Sprintf("%s\n\n", cmd.String())); err != nil { ncp.l.Error("Failed to write string to file", zap.String("file", outfile.Name()), zap.Error(err)) } cmd.Stdout = outfile cmd.Stderr = outfile - if err := cmd.Run(); err != nil { + if err := cmd.Run(); err != nil && !command.ignoreFailure { // Don't return for error to continue capturing following network metadata. ncp.l.Error("Failed to execute command", zap.String("command", cmd.String()), zap.Error(err)) // Log the error in output file because this error does not stop capture job pod from finishing, @@ -342,7 +345,7 @@ func (ncp *NetworkCaptureProvider) CollectMetadata() error { cmd := exec.Command(command.name, command.args...) // Errors will when copying kernel networking configuration for not all files under /proc/sys/net are // readable, like '/proc/sys/net/ipv4/route/flush', which doesn't implement the read function. - if output, err := cmd.CombinedOutput(); err != nil { + if output, err := cmd.CombinedOutput(); err != nil && !command.ignoreFailure { // Don't return for error to continue capturing following network metadata. ncp.l.Error("Failed to execute command", zap.String("command", cmd.String()), zap.String("output", string(output)), zap.Error(err)) } @@ -361,16 +364,44 @@ func (ncp *NetworkCaptureProvider) Cleanup() error { return nil } -func obtainIptablesMode() string { +type iptablesMode string + +const ( + legacyIptablesMode iptablesMode = "legacy" + nftIptablesMode iptablesMode = "nft" +) + +func obtainIptablesMode(l *log.ZapLogger) iptablesMode { // Since iptables v1.8, nf_tables are introduced as an improvement of legacy iptables, but provides the same user // interface as legacy iptables through iptables-nft command. // based on: https://github.com/kubernetes-sigs/iptables-wrappers/blob/97b01f43a8e8db07840fc4b95e833a37c0d36b12/iptables-wrapper-installer.sh - legacySaveOut, _ := exec.Command("iptables-legacy-save").CombinedOutput() + + // when both iptables modes available, we choose the one with more rules. + nftIptablesModeAvaiable := true + legacyIptablesModeAvaiable := true + legacySaveOut, err := exec.Command("iptables-legacy-save").CombinedOutput() + if err != nil && strings.Contains(err.Error(), "command not found") { + legacyIptablesModeAvaiable = false + } + legacySaveLineNum := len(strings.Split(string(legacySaveOut), "\n")) - nftSaveOut, _ := exec.Command("iptables-nft-save").CombinedOutput() - nftSaveLineNum := len(strings.Split(string(nftSaveOut), "\n")) - if legacySaveLineNum > nftSaveLineNum { - return "legacy" + nftSaveOut, err := exec.Command("iptables-nft-save").CombinedOutput() + if err != nil && strings.Contains(err.Error(), "command not found") { + nftIptablesModeAvaiable = false + } + + if nftIptablesModeAvaiable && legacyIptablesModeAvaiable { + nftSaveLineNum := len(strings.Split(string(nftSaveOut), "\n")) + if legacySaveLineNum > nftSaveLineNum { + return legacyIptablesMode + } + return "nft" + } + + // when only one iptables mode available, we choose the available one when the other one is not available. + // when both iptables modes are not available, we choose the nft mode and let the iptables command fail. + if !nftIptablesModeAvaiable { + return legacyIptablesMode } - return "nft" + return nftIptablesMode } From fac120036537051a0752586b254dd75eed4dc7cd Mon Sep 17 00:00:00 2001 From: Qingchuan Hao Date: Wed, 30 Oct 2024 01:49:51 +0000 Subject: [PATCH 2/3] return defined constant --- pkg/capture/provider/network_capture_unix.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/capture/provider/network_capture_unix.go b/pkg/capture/provider/network_capture_unix.go index 406539012b..dabba243a5 100644 --- a/pkg/capture/provider/network_capture_unix.go +++ b/pkg/capture/provider/network_capture_unix.go @@ -197,7 +197,7 @@ type command struct { func (ncp *NetworkCaptureProvider) CollectMetadata() error { ncp.l.Info("Start to collect network metadata") - iptablesMode := obtainIptablesMode(ncp.l) + iptablesMode := obtainIptablesMode() ncp.l.Info(fmt.Sprintf("Iptables mode %s is used", iptablesMode)) iptablesSaveCmdName := fmt.Sprintf("iptables-%s-save", iptablesMode) iptablesCmdName := fmt.Sprintf("iptables-%s", iptablesMode) @@ -371,7 +371,7 @@ const ( nftIptablesMode iptablesMode = "nft" ) -func obtainIptablesMode(l *log.ZapLogger) iptablesMode { +func obtainIptablesMode() iptablesMode { // Since iptables v1.8, nf_tables are introduced as an improvement of legacy iptables, but provides the same user // interface as legacy iptables through iptables-nft command. // based on: https://github.com/kubernetes-sigs/iptables-wrappers/blob/97b01f43a8e8db07840fc4b95e833a37c0d36b12/iptables-wrapper-installer.sh @@ -395,7 +395,7 @@ func obtainIptablesMode(l *log.ZapLogger) iptablesMode { if legacySaveLineNum > nftSaveLineNum { return legacyIptablesMode } - return "nft" + return nftIptablesMode } // when only one iptables mode available, we choose the available one when the other one is not available. From b173ff76508d28194fc674e7ff190b85114f1eff Mon Sep 17 00:00:00 2001 From: Qingchuan Hao Date: Wed, 30 Oct 2024 03:22:34 +0000 Subject: [PATCH 3/3] log iptables command error Signed-off-by: Qingchuan Hao --- pkg/capture/provider/network_capture_unix.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/capture/provider/network_capture_unix.go b/pkg/capture/provider/network_capture_unix.go index dabba243a5..46747256da 100644 --- a/pkg/capture/provider/network_capture_unix.go +++ b/pkg/capture/provider/network_capture_unix.go @@ -197,7 +197,7 @@ type command struct { func (ncp *NetworkCaptureProvider) CollectMetadata() error { ncp.l.Info("Start to collect network metadata") - iptablesMode := obtainIptablesMode() + iptablesMode := obtainIptablesMode(ncp.l) ncp.l.Info(fmt.Sprintf("Iptables mode %s is used", iptablesMode)) iptablesSaveCmdName := fmt.Sprintf("iptables-%s-save", iptablesMode) iptablesCmdName := fmt.Sprintf("iptables-%s", iptablesMode) @@ -371,27 +371,29 @@ const ( nftIptablesMode iptablesMode = "nft" ) -func obtainIptablesMode() iptablesMode { +func obtainIptablesMode(logger *log.ZapLogger) iptablesMode { // Since iptables v1.8, nf_tables are introduced as an improvement of legacy iptables, but provides the same user // interface as legacy iptables through iptables-nft command. // based on: https://github.com/kubernetes-sigs/iptables-wrappers/blob/97b01f43a8e8db07840fc4b95e833a37c0d36b12/iptables-wrapper-installer.sh - // when both iptables modes available, we choose the one with more rules. + // When both iptables modes available, we choose the one with more rules, because the other one normally outputs empty rules. nftIptablesModeAvaiable := true legacyIptablesModeAvaiable := true legacySaveOut, err := exec.Command("iptables-legacy-save").CombinedOutput() - if err != nil && strings.Contains(err.Error(), "command not found") { - legacyIptablesModeAvaiable = false + if err != nil { + nftIptablesModeAvaiable = false + logger.Error("Failed to run iptables-legacy-save", zap.Error(err)) } - legacySaveLineNum := len(strings.Split(string(legacySaveOut), "\n")) + nftSaveOut, err := exec.Command("iptables-nft-save").CombinedOutput() - if err != nil && strings.Contains(err.Error(), "command not found") { + if err != nil { nftIptablesModeAvaiable = false + logger.Error("Failed to run iptables-nft-save", zap.Error(err)) } + nftSaveLineNum := len(strings.Split(string(nftSaveOut), "\n")) if nftIptablesModeAvaiable && legacyIptablesModeAvaiable { - nftSaveLineNum := len(strings.Split(string(nftSaveOut), "\n")) if legacySaveLineNum > nftSaveLineNum { return legacyIptablesMode }