Improve nftables connmark error handling and fix test typo

Author: yash97

.github/workflows/pr-automated-tests.yaml

@@ -41,7 +41,7 @@ jobs:
         uses: codecov/codecov-action@79066c46f8dcdf8d7355f820dbac958c5b4cb9d3 # refs/tags/v4.5.0
         with:
             token: ${{ secrets.CODECOV_TOKEN }}
-  docker-build:
+  docker-build-arch:
     name: Build Docker images (${{ matrix.arch }})
     strategy:
       matrix:
@@ -58,7 +58,13 @@ jobs:
         uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # refs/tags/v3.11.1
       - name: Build CNI image
         run: make docker
-  docker-build-init:
+  docker-build:
+    name: Build Docker images
+    needs: docker-build-arch
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "All docker builds completed successfully"
+  docker-build-init-arch:
     name: Build Docker init images (${{ matrix.arch }})
     strategy:
       matrix:
@@ -75,3 +81,9 @@ jobs:
         uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # refs/tags/v3.11.1
       - name: Build CNI Init image
         run: make docker-init
+  docker-build-init:
+    name: Build Docker init images
+    needs: docker-build-init-arch
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "All docker init builds completed successfully"

pkg/networkutils/iptables_connmark.go

@@ -59,7 +59,6 @@ func (c *iptablesConnmark) Cleanup() error {
 
 	// Clear and delete chain
 	_ = ipt.ClearChain("nat", "AWS-CONNMARK-CHAIN-0")
-	//_ = ipt.DeleteChain("nat", "AWS-CONNMARK-CHAIN-0")
 
 	return nil
 }

pkg/networkutils/mocks/network_mocks.go

@@ -848,7 +848,7 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) {
 		}, mockIptables.(*mock_iptables.MockIptables).DataplaneState)
 }
 
-func TestUUpdateHostSNATRules(t *testing.T) {
+func TestUpdateHostSNATRules(t *testing.T) {
 	ctrl, mockNetLink, _, mockNS, mockIptables := setup(t)
 	defer ctrl.Finish()
 

pkg/networkutils/network_test.go

@@ -116,12 +116,14 @@ func (c *nftConnmark) ensureBaseChain(table *nftables.Table) *nftables.Chain {
 	}
 	if existing == nil {
 		priority := c.getDesiredPriority()
+		policy := nftables.ChainPolicyAccept
 		chain := c.nft.AddChain(&nftables.Chain{
 			Name:     nftBaseChainName,
 			Table:    table,
 			Type:     nftables.ChainTypeNAT,
 			Hooknum:  nftables.ChainHookPrerouting,
 			Priority: &priority,
+			Policy:   &policy,
 		})
 		return chain
 	}
@@ -276,7 +278,9 @@ func (c *nftConnmark) ensureConnmarkChainRules(table *nftables.Table, chain *nft
 
 	// Delete unknown rules
 	for _, r := range unknownRules {
-		c.nft.DelRule(r)
+		if err := c.nft.DelRule(r); err != nil {
+			log.Errorf("failed to delete unknown rule: %s, id: %d", err, r.Handle)
+		}
 	}
 
 	desiredCIDRs := make(map[string]bool)
@@ -287,13 +291,20 @@ func (c *nftConnmark) ensureConnmarkChainRules(table *nftables.Table, chain *nft
 	// Delete stale CIDRs
 	for cidr, rule := range currentCIDRs {
 		if !desiredCIDRs[cidr] {
-			c.nft.DelRule(rule)
+			if err := c.nft.DelRule(rule); err != nil {
+				log.Errorf("failed to delete stale cidr rule %s in nf table: %s, id: %d", cidr, err, rule.Handle)
+			}
 		}
 	}
 
 	// Insert missing CIDRs (prepends - order doesn't matter for CIDR rules)
-	for cidr := range desiredCIDRs {
-		if _, exists := currentCIDRs[cidr]; !exists {
+	for cidrStr := range desiredCIDRs {
+		if _, exists := currentCIDRs[cidrStr]; !exists {
+			_, cidr, err := net.ParseCIDR(cidrStr)
+			if err != nil {
+				log.Errorf("failed to insert cidr %s in nf table: %s", cidrStr, err)
+				continue
+			}
 			c.insertCIDRReturnRule(table, chain, cidr) // InsertRule
 		}
 	}
@@ -489,11 +500,7 @@ func (c *nftConnmark) addSetMarkRule(table *nftables.Table, chain *nftables.Chai
 	})
 }
 
-func (c *nftConnmark) insertCIDRReturnRule(table *nftables.Table, chain *nftables.Chain, cidrStr string) {
-	_, cidr, err := net.ParseCIDR(cidrStr)
-	if err != nil {
-		return
-	}
+func (c *nftConnmark) insertCIDRReturnRule(table *nftables.Table, chain *nftables.Chain, cidr *net.IPNet) {
 	c.nft.InsertRule(&nftables.Rule{
 		Table: table,
 		Chain: chain,

pkg/networkutils/nftables_connmark.go

@@ -110,6 +110,7 @@ var _ = Describe("test SNAT with kube-proxy modes", func() {
 			By("detecting iptables backend on node")
 			backend := detectIptablesBackend(primaryNode.Name)
 			fmt.Fprintf(GinkgoWriter, "detected iptables backend: %s\n", backend)
+			Expect(backend).To(Or(Equal("legacy"), Equal("nftables")))
 
 			By("verifying CNI connmark rules exist")
 			verifyConnmarkRules(primaryNode.Name, backend)