diff --git a/daemon/libnetwork/iptables/iptables.go b/daemon/libnetwork/iptables/iptables.go index 3d11cf07df..581327dc99 100644 --- a/daemon/libnetwork/iptables/iptables.go +++ b/daemon/libnetwork/iptables/iptables.go @@ -440,7 +440,11 @@ func (iptable IPTable) DeleteJumpRule(table Table, fromChain, toChain string, ru rule = append(rule, "-j", toChain) if iptable.Exists(table, fromChain, rule...) { if err := iptable.RawCombinedOutput(append([]string{"-t", string(table), "-D", fromChain}, rule...)...); err != nil { - return fmt.Errorf("unable to remove jump to %s rule in %s chain: %v", toChain, fromChain, err) + // Another process may have removed the rule between Exists and Delete. + if isRuleNotFoundError(err) { + return nil + } + return fmt.Errorf("unable to remove jump to %s rule in %s chain: %w", toChain, fromChain, err) } } return nil @@ -500,7 +504,14 @@ func (r Rule) Delete() error { if !r.Exists() { return nil } - return r.exec(Delete) + if err := r.exec(Delete); err != nil { + // Another process may have removed the rule between Exists and Delete. + if isRuleNotFoundError(err) { + return nil + } + return err + } + return nil } func (r Rule) String() string { @@ -510,3 +521,18 @@ func (r Rule) String() string { } return strings.Join(cmd, " ") } + +// isRuleNotFoundError reports whether err indicates that an iptables +// delete could not find a matching rule. +// +// The iptables-nft backend maps ENOENT from nft_rule_delete to this +// diagnostic: +// +// iptables: Bad rule (does a matching rule exist in that chain?). +// +// see: https://git.netfilter.org/iptables/tree/iptables/nft.c?id=384958620abab397062b67fb2763e813b63f74f0#n2745 +// ref: https://git.netfilter.org/iptables/commit/?id=384958620abab397062b67fb2763e813b63f74f0 +func isRuleNotFoundError(err error) bool { + return err != nil && + strings.Contains(err.Error(), "Bad rule (does a matching rule exist in that chain?)") +} diff --git a/daemon/libnetwork/iptables/iptables_test.go b/daemon/libnetwork/iptables/iptables_test.go index da74efb440..ed3f14b0fd 100644 --- a/daemon/libnetwork/iptables/iptables_test.go +++ b/daemon/libnetwork/iptables/iptables_test.go @@ -336,3 +336,33 @@ func TestFlushChain(t *testing.T) { // Cleanup _ = iptable.RemoveExistingChain(chain, table) } + +// TestDeleteNonExistingRule verifies the diagnostic returned by iptables when +// deleting a rule that does not exist. This is used to handle the race where a +// rule is removed by another process between checking for it and deleting it. +// +// Reproducing that race directly would be timing-dependent, so exercise the +// delete error deterministically with a rule in a freshly-created chain. +func TestDeleteNonExistingRule(t *testing.T) { + if UsingFirewalld() { + t.Skip("firewalld in host netns cannot create rules in the test's netns") + } + defer netnsutils.SetupTestOSContext(t)() + iptable := GetIptable(IPv4) + + const chain = "TEST-DELETEMISSING" + _, err := iptable.NewChain(chain, Filter) + assert.NilError(t, err) + defer func() { _ = iptable.RemoveExistingChain(chain, Filter) }() + + // Use TEST-NET-1 to make the rule clearly test-only. The freshly-created + // chain guarantees that the rule does not already exist. + err = iptable.RawCombinedOutput( + "-t", string(Filter), + "-D", chain, + "-s", "192.0.2.1", + "-j", "ACCEPT", + ) + assert.Assert(t, err != nil) + assert.Assert(t, isRuleNotFoundError(err), err) +}