From f106702c85f1ab4b3c2d3e4e048e3482404e8702 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 10 Jun 2020 17:04:05 +0100 Subject: [PATCH] iptables: add transient rules removal failure to badLogMessages In an attempt to catch #11276 in CI, let's add any message related to a failure to flush or delete the chain related to iptables transient rules to the list of badLogMessages we want to catch. We need to filter on the name of the chain for transient rules to avoid false positives, which requires exporting that name. We also need to modify the log message error, to avoid adding four disting logs to the list (combinations for iptables/ip6tables, flush/delete). Signed-off-by: Quentin Monnet --- pkg/datapath/iptables/iptables.go | 18 +++++----- test/helpers/cons.go | 60 ++++++++++++++++--------------- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/pkg/datapath/iptables/iptables.go b/pkg/datapath/iptables/iptables.go index 1c29ed3daa8a..9caefd699884 100644 --- a/pkg/datapath/iptables/iptables.go +++ b/pkg/datapath/iptables/iptables.go @@ -50,7 +50,7 @@ const ( ciliumPreMangleChain = "CILIUM_PRE_mangle" ciliumPreRawChain = "CILIUM_PRE_raw" ciliumForwardChain = "CILIUM_FORWARD" - ciliumTransientForwardChain = "CILIUM_TRANSIENT_FORWARD" + CiliumTransientForwardChain = "CILIUM_TRANSIENT_FORWARD" feederDescription = "cilium-feeder:" xfrmDescription = "cilium-xfrm-notrack:" ) @@ -165,7 +165,7 @@ func (m *IptablesManager) removeCiliumRules(table, prog, match string) { for scanner.Scan() { rule := scanner.Text() log.WithField(logfields.Object, logfields.Repr(rule)).Debugf("Considering removing %s rule", prog) - if match != ciliumTransientForwardChain && strings.Contains(rule, ciliumTransientForwardChain) { + if match != CiliumTransientForwardChain && strings.Contains(rule, CiliumTransientForwardChain) { continue } @@ -179,8 +179,8 @@ func (m *IptablesManager) removeCiliumRules(table, prog, match string) { // disabled chains skipFeeder := false for _, disabledChain := range option.Config.DisableIptablesFeederRules { - // we skip if the match is ciliumTransientForwardChain since we don't want to touch it - if match != ciliumTransientForwardChain && strings.Contains(rule, " "+strings.ToUpper(disabledChain)+" ") { + // we skip if the match is CiliumTransientForwardChain since we don't want to touch it + if match != CiliumTransientForwardChain && strings.Contains(rule, " "+strings.ToUpper(disabledChain)+" ") { log.WithField("chain", disabledChain).Info("Skipping the removal of feeder chain") skipFeeder = true break @@ -217,11 +217,11 @@ func (c *customChain) remove(waitArgs []string, quiet bool) { // present, log the error. // This is to help debug #11276. msgChainNotFound := ": No chain/target/match by that name.\n" - debugTransientRules := c.name == ciliumTransientForwardChain && + debugTransientRules := c.name == CiliumTransientForwardChain && string(combinedOutput) != prog+msgChainNotFound if !quiet || debugTransientRules { log.Warnf(string(combinedOutput)) - log.WithError(err).WithField(logfields.Object, args).Warnf("Unable to %s Cilium %s chain", operation, prog) + log.WithError(err).WithField(logfields.Object, args).Warnf("Unable to process chain %s with %s (%s)", c.name, prog, operation) } } } @@ -339,7 +339,7 @@ var ciliumChains = []customChain{ } var transientChain = customChain{ - name: ciliumTransientForwardChain, + name: CiliumTransientForwardChain, table: "filter", hook: "FORWARD", feederArgs: []string{""}, @@ -717,7 +717,7 @@ func getDeliveryInterface(ifName string) string { func (m *IptablesManager) installForwardChainRules(ifName, localDeliveryInterface, forwardChain string) error { transient := "" - if forwardChain == ciliumTransientForwardChain { + if forwardChain == CiliumTransientForwardChain { transient = " (transient)" } @@ -829,7 +829,7 @@ func (m *IptablesManager) TransientRulesStart(ifName string) error { // TransientRulesEnd removes Cilium related rules installed from TransientRulesStart. func (m *IptablesManager) TransientRulesEnd(quiet bool) { if option.Config.EnableIPv4 { - m.removeCiliumRules("filter", "iptables", ciliumTransientForwardChain) + m.removeCiliumRules("filter", "iptables", CiliumTransientForwardChain) transientChain.remove(m.waitArgs, quiet) } } diff --git a/test/helpers/cons.go b/test/helpers/cons.go index 62540fa0ca2c..3c07dd8e2794 100644 --- a/test/helpers/cons.go +++ b/test/helpers/cons.go @@ -20,6 +20,7 @@ import ( "os" "time" + "github.com/cilium/cilium/pkg/datapath/iptables" k8sConst "github.com/cilium/cilium/pkg/k8s/apis/cilium.io" "github.com/cilium/cilium/pkg/versioncheck" "github.com/cilium/cilium/test/ginkgo-ext" @@ -222,20 +223,22 @@ const ( SecondaryIface = "enp0s9" // Logs messages that should not be in the cilium logs. - panicMessage = "panic:" - deadLockHeader = "POTENTIAL DEADLOCK:" // from github.com/sasha-s/go-deadlock/deadlock.go:header - segmentationFault = "segmentation fault" // from https://github.com/cilium/cilium/issues/3233 - NACKreceived = "NACK received for version" // from https://github.com/cilium/cilium/issues/4003 - RunInitFailed = "JoinEP: " // from https://github.com/cilium/cilium/pull/5052 - sizeMismatch = "size mismatch for BPF map" // from https://github.com/cilium/cilium/issues/7851 - emptyBPFInitArg = "empty argument passed to bpf/init.sh" // from https://github.com/cilium/cilium/issues/10228 - RemovingMapMsg = "Removing map to allow for property upgrade" // from https://github.com/cilium/cilium/pull/10626 - logBufferMessage = "Log buffer too small to dump verifier log" // from https://github.com/cilium/cilium/issues/10517 - ClangErrorsMsg = " errors generated." // from https://github.com/cilium/cilium/issues/10857 - ClangErrorMsg = "1 error generated." // from https://github.com/cilium/cilium/issues/10857 - symbolSubstitution = "Skipping symbol substitution" // - uninitializedRegen = "Uninitialized regeneration level" // from https://github.com/cilium/cilium/pull/10949 - unstableStat = "BUG: stat() has unstable behavior" // from https://github.com/cilium/cilium/pull/11028 + panicMessage = "panic:" + deadLockHeader = "POTENTIAL DEADLOCK:" // from github.com/sasha-s/go-deadlock/deadlock.go:header + segmentationFault = "segmentation fault" // from https://github.com/cilium/cilium/issues/3233 + NACKreceived = "NACK received for version" // from https://github.com/cilium/cilium/issues/4003 + RunInitFailed = "JoinEP: " // from https://github.com/cilium/cilium/pull/5052 + sizeMismatch = "size mismatch for BPF map" // from https://github.com/cilium/cilium/issues/7851 + emptyBPFInitArg = "empty argument passed to bpf/init.sh" // from https://github.com/cilium/cilium/issues/10228 + RemovingMapMsg = "Removing map to allow for property upgrade" // from https://github.com/cilium/cilium/pull/10626 + logBufferMessage = "Log buffer too small to dump verifier log" // from https://github.com/cilium/cilium/issues/10517 + ClangErrorsMsg = " errors generated." // from https://github.com/cilium/cilium/issues/10857 + ClangErrorMsg = "1 error generated." // from https://github.com/cilium/cilium/issues/10857 + symbolSubstitution = "Skipping symbol substitution" // + uninitializedRegen = "Uninitialized regeneration level" // from https://github.com/cilium/cilium/pull/10949 + unstableStat = "BUG: stat() has unstable behavior" // from https://github.com/cilium/cilium/pull/11028 + removeTransientRule = "Unable to process chain " + + iptables.CiliumTransientForwardChain + " with ip" // from https://github.com/cilium/cilium/issues/11276 // HelmTemplate is the location of the Helm templates to install Cilium HelmTemplate = "../install/kubernetes/cilium" @@ -275,20 +278,21 @@ var ( // badLogMessages is a map which key is a part of a log message which indicates // a failure if the message does not contain any part from value list. var badLogMessages = map[string][]string{ - panicMessage: nil, - deadLockHeader: nil, - segmentationFault: nil, - NACKreceived: nil, - RunInitFailed: {"signal: terminated", "signal: killed"}, - sizeMismatch: nil, - emptyBPFInitArg: nil, - RemovingMapMsg: nil, - logBufferMessage: nil, - ClangErrorsMsg: nil, - ClangErrorMsg: nil, - symbolSubstitution: nil, - uninitializedRegen: nil, - unstableStat: nil, + panicMessage: nil, + deadLockHeader: nil, + segmentationFault: nil, + NACKreceived: nil, + RunInitFailed: {"signal: terminated", "signal: killed"}, + sizeMismatch: nil, + emptyBPFInitArg: nil, + RemovingMapMsg: nil, + logBufferMessage: nil, + ClangErrorsMsg: nil, + ClangErrorMsg: nil, + symbolSubstitution: nil, + uninitializedRegen: nil, + unstableStat: nil, + removeTransientRule: nil, } var ciliumCLICommands = map[string]string{