Skip to content

Commit

Permalink
iptables: add transient rules removal failure to badLogMessages
Browse files Browse the repository at this point in the history
[ upstream commit e78405a ]

[ Backporter's notes: Minor conflicts against PR #10639 and CI failure
                      log messages. ]

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>
Signed-off-by: Joe Stringer <joe@cilium.io>
  • Loading branch information
qmonnet authored and joestringer committed Jun 11, 2020
1 parent 073d5a5 commit 8f4dd3c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 25 deletions.
14 changes: 7 additions & 7 deletions pkg/datapath/iptables/iptables.go
Expand Up @@ -49,7 +49,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 @@ -164,7 +164,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 Down Expand Up @@ -200,11 +200,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 @@ -322,7 +322,7 @@ var ciliumChains = []customChain{
}

var transientChain = customChain{
name: ciliumTransientForwardChain,
name: CiliumTransientForwardChain,
table: "filter",
hook: "FORWARD",
feederArgs: []string{""},
Expand Down Expand Up @@ -721,7 +721,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 @@ -833,7 +833,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
40 changes: 22 additions & 18 deletions test/helpers/cons.go
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 @@ -210,15 +211,17 @@ const (
PrivateIface = "enp0s8"

// 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
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
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 @@ -265,15 +268,16 @@ const CiliumConfigMapPatchKvstoreAllocator = "cilium-cm-kvstore-allocator-patch.
// 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,
panicMessage: nil,
deadLockHeader: nil,
segmentationFault: nil,
NACKreceived: nil,
RunInitFailed: {"signal: terminated", "signal: killed"},
sizeMismatch: nil,
emptyBPFInitArg: nil,
RemovingMapMsg: nil,
logBufferMessage: nil,
removeTransientRule: nil,
}

var ciliumCLICommands = map[string]string{
Expand Down

0 comments on commit 8f4dd3c

Please sign in to comment.