From d8e28fc900cb26e70ba97e48eaba8d8d41971480 Mon Sep 17 00:00:00 2001
From: Antonio Ojea <antonio.ojea.garcia@gmail.com>
Date: Mon, 9 Dec 2019 15:35:32 +0100
Subject: [PATCH] Portmap doesn't fail if chain doesn't exist

It turns out that the portmap plugin is not idempotent if its
executed in parallel.
The errors are caused due to a race of different instantiations
deleting the chains.
This patch does that the portmap plugin doesn't fail if the
errors are because the chain doesn't exist on teardown.

xref: https://github.com/containernetworking/plugins/issues/418

Signed-off-by: Antonio Ojea <antonio.ojea.garcia@gmail.com>
---
 plugins/meta/portmap/chain.go      | 14 ++++++++++---
 plugins/meta/portmap/chain_test.go | 32 ++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/plugins/meta/portmap/chain.go b/plugins/meta/portmap/chain.go
index 8e8dbe3fa..23fa6cce8 100644
--- a/plugins/meta/portmap/chain.go
+++ b/plugins/meta/portmap/chain.go
@@ -70,7 +70,10 @@ func (c *chain) teardown(ipt *iptables.IPTables) error {
 	// flush the chain
 	// This will succeed *and create the chain* if it does not exist.
 	// If the chain doesn't exist, the next checks will fail.
-	if err := ipt.ClearChain(c.table, c.name); err != nil {
+	err := ipt.ClearChain(c.table, c.name)
+	eerr, eok := err.(*iptables.Error)
+	// #418 don't fail if the error is because the chain doesn't exist
+	if err != nil && !eerr.IsNotExist() {
 		return err
 	}
 
@@ -97,10 +100,15 @@ func (c *chain) teardown(ipt *iptables.IPTables) error {
 		}
 	}
 
-	if err := ipt.DeleteChain(c.table, c.name); err != nil {
+	err = ipt.DeleteChain(c.table, c.name)
+	eerr, eok = err.(*iptables.Error)
+	switch {
+	case eok && eerr.IsNotExist():
+		// #418 don't fail if the chain doesn't exist
+		return nil
+	default:
 		return err
 	}
-	return nil
 }
 
 // insertUnique will add a rule to a chain if it does not already exist.
diff --git a/plugins/meta/portmap/chain_test.go b/plugins/meta/portmap/chain_test.go
index 93e4be13e..a4053a4d0 100644
--- a/plugins/meta/portmap/chain_test.go
+++ b/plugins/meta/portmap/chain_test.go
@@ -195,4 +195,36 @@ var _ = Describe("chain tests", func() {
 			}
 		}
 	})
+
+	It("deletes chains idempotently in parallel", func() {
+		defer cleanup()
+		// number of parallel executions
+		N := 10
+		err := testChain.setup(ipt)
+		Expect(err).NotTo(HaveOccurred())
+
+		errCh := make(chan error, N)
+		for i := 0; i < N; i++ {
+			go func() {
+				errCh <- testChain.teardown(ipt)
+			}()
+		}
+		errs := 0
+		for err := range errCh {
+			if err != nil {
+				errs++
+			}
+		}
+
+		Expect(errs).Should(BeNumerically("==", 0))
+
+		chains, err := ipt.ListChains(TABLE)
+		Expect(err).NotTo(HaveOccurred())
+		for _, chain := range chains {
+			if chain == testChain.name {
+				Fail("Chain was not deleted")
+			}
+		}
+
+	})
 })