Skip to content

Commit

Permalink
iptables: add transient rules removal failure to badLogMessages
Browse files Browse the repository at this point in the history 
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 <quentin@isovalent.com>
  • Loading branch information
@qmonnet
qmonnet committed Jun 10, 2020
1 parent 6dae0d7 commit f106702
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 37 deletions.
18 changes: 9 additions & 9 deletions pkg/datapath/iptables/iptables.go
View file
Expand Up @@ -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:"
)
Expand Down Expand Up @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -339,7 +339,7 @@ var ciliumChains = []customChain{
}

var transientChain = customChain{
name: ciliumTransientForwardChain,
name: CiliumTransientForwardChain,
table: "filter",
hook: "FORWARD",
feederArgs: []string{""},
Expand Down Expand Up @@ -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)"
}

Expand Down Expand Up @@ -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)
}
}
Expand Down
60 changes: 32 additions & 28 deletions test/helpers/cons.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit f106702

Please sign in to comment.