[v3.29] Avoid spurious inserted rules warning

felix/iptables/table.go

@@ -276,6 +276,7 @@ type Table struct {
 	gaugeNumChains        prometheus.Gauge
 	gaugeNumRules         prometheus.Gauge
 	countNumLinesExecuted prometheus.Counter
+	unexpectedInsertsSeen int
 
 	// Reusable buffer for writing to iptables.
 	restoreInputBuffer RestoreInputBuilder
@@ -677,20 +678,22 @@ func (t *Table) loadDataplaneState() {
 		if !t.ourChainsRegexp.MatchString(chainName) {
 			// Not one of our chains so it may be one that we're inserting rules into.
 			insertedRules := t.chainToInsertedRules[chainName]
-			if len(insertedRules) == 0 {
-				// This chain shouldn't have any inserts, make sure that's the
-				// case.  This case also covers the case where a chain was removed,
-				// making dpHashes nil.
-				dataplaneHasInserts := false
+			appendedRules := t.chainToAppendedRules[chainName]
+			if len(insertedRules) == 0 && len(appendedRules) == 0 {
+				// This chain shouldn't have any inserted/appended rules, make
+				// sure that's the case.  This case also covers the case where
+				// a chain was removed, making dpHashes nil.
+				chainHasCalicoRules := false
 				for _, hash := range dpHashes {
 					if hash != "" {
-						dataplaneHasInserts = true
+						chainHasCalicoRules = true
 						break
 					}
 				}
-				if dataplaneHasInserts {
+				if chainHasCalicoRules {
 					logCxt.WithField("actualRuleIDs", dpHashes).Warn(
 						"Chain had unexpected inserts, marking for resync")
+					t.unexpectedInsertsSeen++
 					t.dirtyInsertAppend.Add(chainName)
 				}
 				continue
@@ -1532,6 +1535,10 @@ func (t *Table) renderDeleteByValueLine(chainName string, ruleNum int) (string,
 	return strings.Replace(rule, "-A", "-D", 1), nil
 }
 
+func (t *Table) UnexpectedInsertsSeen() int {
+	return t.unexpectedInsertsSeen
+}
+
 func CalculateRuleHashes(chainName string, rules []generictables.Rule, features *environment.Features) []string {
 	chain := generictables.Chain{
 		Name:  chainName,

felix/iptables/table_test.go

@@ -1043,6 +1043,41 @@ func describeEmptyDataplaneTests(dataplaneMode string) {
 			})
 		})
 	})
+
+	Describe("only appending rules to a chain", func() {
+		BeforeEach(func() {
+			table.AppendRules("FORWARD", []generictables.Rule{
+				{Match: Match(), Action: DropAction{}, Comment: []string{"append drop rule"}},
+				{Match: Match(), Action: AcceptAction{}, Comment: []string{"append accept rule"}},
+			})
+
+			table.Apply()
+		})
+		It("should update the dataplane", func() {
+			Expect(dataplane.Chains).To(Equal(map[string][]string{
+				"FORWARD": {
+					"-m comment --comment \"cali:qNsBylRkftPwO3XF\" -m comment --comment \"append drop rule\" --jump DROP",
+					"-m comment --comment \"cali:IQ9H0Scq00rF0w4S\" -m comment --comment \"append accept rule\" --jump ACCEPT",
+				},
+				"INPUT":  {},
+				"OUTPUT": {},
+			}))
+		})
+		It("should avoid spurious insert warnings", func() {
+			table.InvalidateDataplaneCache("test")
+			table.Apply()
+			table.Apply()
+			Expect(dataplane.Chains).To(Equal(map[string][]string{
+				"FORWARD": {
+					"-m comment --comment \"cali:qNsBylRkftPwO3XF\" -m comment --comment \"append drop rule\" --jump DROP",
+					"-m comment --comment \"cali:IQ9H0Scq00rF0w4S\" -m comment --comment \"append accept rule\" --jump ACCEPT",
+				},
+				"INPUT":  {},
+				"OUTPUT": {},
+			}))
+			Expect(table.UnexpectedInsertsSeen()).To(BeZero())
+		})
+	})
 }
 
 var _ = Describe("Tests of post-update recheck behaviour with refresh timer (nft)", func() {