[Release v3.30] IPSets: re-create ipsets failed during re-sync

felix/ipsets/ipsets.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2017-2024 Tigera, Inc. All rights reserved.
+// Copyright (c) 2017-2025 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@ import (
 
 const (
 	MaxIPSetDeletionsPerIteration = 1
+	MaxRetryAttempt               = 10
 )
 
 type dataplaneMetadata struct {
@@ -44,6 +45,7 @@ type dataplaneMetadata struct {
 	RangeMin     int
 	RangeMax     int
 	DeleteFailed bool
+	ListError    error
 }
 
 // IPSets manages a whole "plane" of IP sets, i.e. all the IPv4 sets, or all the IPv6 IP sets.
@@ -339,10 +341,11 @@ func (s *IPSets) ApplyUpdates() {
 		retryDelay *= 2
 	}
 
-	for attempt := 0; attempt < 10; attempt++ {
+	for attempt := 0; attempt < MaxRetryAttempt; attempt++ {
 		if attempt > 0 {
 			s.logCxt.Info("Retrying after an ipsets update failure...")
 		}
+		failureMaybeTransient := attempt < MaxRetryAttempt/2
 		if s.resyncRequired {
 			// Compare our in-memory state against the dataplane and queue up
 			// modifications to fix any inconsistencies.
@@ -351,8 +354,16 @@ func (s *IPSets) ApplyUpdates() {
 
 			if err := s.tryResync(); err != nil {
 				s.logCxt.WithError(err).Warning("Failed to resync with dataplane")
-				backOff()
-				continue
+
+				// After a few attempts, most likely, we are dealing with a persistent failure.
+				// This could be due to different failures, like userspace and kernel incompatibility.
+				// The incompatibility failure can be fixed by swapping the one Felix understand (created from
+				// desired state) and the one (with higher revision) in dataplane. As such, we should stop re-trying
+				// to re-sync, and instead continue with the next steps.
+				if failureMaybeTransient {
+					backOff()
+					continue
+				}
 			}
 			s.resyncRequired = false
 		}
@@ -413,17 +424,21 @@ func (s *IPSets) tryResync() (err error) {
 		s.logCxt.Debugf("List of ipsets: %v", ipSets)
 	}
 
+	var failedIPSets []string
 	for _, name := range ipSets {
 		if debug {
 			s.logCxt.Debugf("Parsing IP set %v.", name)
 		}
-		err = s.resyncIPSet(name)
-		if err != nil {
+		if err = s.resyncIPSet(name); err != nil {
 			s.logCxt.WithError(err).Errorf("Failed to parse ipset %v", name)
-			return
+			failedIPSets = append(failedIPSets, name)
 		}
 	}
 
+	if len(failedIPSets) > 0 {
+		return fmt.Errorf("failed to parse IPSets %v", strings.Join(failedIPSets, ","))
+	}
+
 	// Mark any IP sets that we didn't see as empty.
 	for name, members := range s.mainSetNameToMembers {
 		if _, ok := s.setNameToProgrammedMetadata.Dataplane().Get(name); ok {
@@ -495,8 +510,9 @@ func (s *IPSets) resyncIPSet(ipSetName string) error {
 	//
 	// As we stream through the data, we extract the name of the IP set and its members. We
 	// use the IP set's metadata to convert each member to its canonical form for comparison.
+	meta := dataplaneMetadata{}
+	debug := log.GetLevel() >= log.DebugLevel
 	err := s.runIPSetList(ipSetName, func(scanner *bufio.Scanner) error {
-		debug := log.GetLevel() >= log.DebugLevel
 		ipSetName := ""
 		var ipSetType IPSetType
 		for scanner.Scan() {
@@ -520,9 +536,7 @@ func (s *IPSets) resyncIPSet(ipSetName string) error {
 				// When we hit the Header line we should know the name, and type of the IP set, which lets
 				// us update the tracker.
 				parts := strings.Split(line, " ")
-				meta := dataplaneMetadata{
-					Type: ipSetType,
-				}
+				meta.Type = ipSetType
 				for idx, p := range parts {
 					if p == "maxelem" {
 						if idx+1 >= len(parts) {
@@ -553,7 +567,6 @@ func (s *IPSets) resyncIPSet(ipSetName string) error {
 						break
 					}
 				}
-				s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta)
 			}
 			if strings.HasPrefix(line, "Members:") {
 				// Start of a Members entry, following this, there'll be one member per
@@ -624,10 +637,12 @@ func (s *IPSets) resyncIPSet(ipSetName string) error {
 		}
 		return scanner.Err()
 	})
-	if err != nil {
-		return err
+	meta.ListError = err
+	if debug {
+		s.logCxt.WithField("setName", ipSetName).Debugf("Parsed metadata from dataplane %+v", meta)
 	}
-	return nil
+	s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta)
+	return err
 }
 
 func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) error) error {

felix/ipsets/ipsets_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024 Tigera, Inc. All rights reserved.
+// Copyright (c) 2016-2025 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -457,6 +457,55 @@ var _ = Describe("IP sets dataplane", func() {
 		})
 	})
 
+	It("Calico IP sets with unsupported revision and in desired sets should be re-created", func() {
+		dataplane.IPSetMetadata = map[string]setMetadata{
+			v4MainIPSetName: {
+				Name:     v4MainIPSetName,
+				Family:   "inet",
+				Type:     IPSetTypeHashIP,
+				MaxSize:  1234,
+				Revision: supportedMockRevision + 1,
+			},
+			v4MainIPSetName2: {
+				Name:     v4MainIPSetName2,
+				Family:   "inet",
+				Type:     IPSetTypeHashIP,
+				MaxSize:  1234,
+				Revision: supportedMockRevision + 1,
+			},
+			v4MainIPSetName3: {
+				Name:     v4MainIPSetName3,
+				Family:   "inet",
+				Type:     IPSetTypeHashIP,
+				MaxSize:  1234,
+				Revision: supportedMockRevision,
+			},
+		}
+		dataplane.IPSetMembers[v4MainIPSetName] = set.From("10.0.0.1", "10.0.0.3")
+		dataplane.IPSetMembers[v4MainIPSetName2] = set.From("10.0.0.1", "10.0.0.4")
+		dataplane.IPSetMembers[v4MainIPSetName3] = set.From("10.0.0.5", "10.0.0.6")
+
+		ipsets.AddOrReplaceIPSet(meta, []string{"10.0.0.1", "10.0.0.2"})
+		ipsets.AddOrReplaceIPSet(meta3, []string{"10.0.0.5", "10.0.0.6"})
+		apply()
+
+		dataplane.ExpectMembers(map[string][]string{
+			v4MainIPSetName:  []string{"10.0.0.1", "10.0.0.2"}, // New IPSet from the desired state.
+			v4TempIPSetName0: []string{"10.0.0.1", "10.0.0.3"}, // Temp IPSet from dataplane state.
+
+			// v4MainIPSetName2 should be destroyed since it's not in the desired state.
+
+			v4MainIPSetName3: []string{"10.0.0.5", "10.0.0.6"}, // This IPSet should not be touched.
+		})
+
+		apply()
+		dataplane.ExpectMembers(map[string][]string{
+			v4MainIPSetName: []string{"10.0.0.1", "10.0.0.2"},
+			// Temp IPSet should be destroyed.
+			v4MainIPSetName3: []string{"10.0.0.5", "10.0.0.6"}, // This IPSet should not be touched.
+		})
+	})
+
 	Describe("with many left-over IP sets in place", func() {
 		BeforeEach(func() {
 			for i := 0; i < MaxIPSetDeletionsPerIteration*3; i++ {

felix/ipsets/utils_for_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2024 Tigera, Inc. All rights reserved.
+// Copyright (c) 2016-2025 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -36,6 +36,10 @@ import (
 
 // This file contains shared test infrastructure for testing the ipsets package.
 
+const (
+	supportedMockRevision = 5
+)
+
 var (
 	transientFailure = errors.New("Simulated transient failure")
 	permanentFailure = errors.New("Simulated permanent failure")
@@ -454,6 +458,7 @@ type setMetadata struct {
 	Name     string
 	Family   IPFamily
 	Type     IPSetType
+	Revision int
 	MaxSize  int
 	RangeMin int
 	RangeMax int
@@ -726,18 +731,26 @@ func (c *listCmd) main() {
 		result = fmt.Errorf("ipset %v does not exists", c.SetName)
 		return
 	}
-	writef("Name: %s\n", c.SetName)
 	meta, ok := c.Dataplane.IPSetMetadata[c.SetName]
 	if !ok {
 		// Default metadata for IP sets created by tests.
 		meta = setMetadata{
-			Name:    v4MainIPSetName,
-			Family:  IPFamilyV4,
-			Type:    IPSetTypeHashIP,
-			MaxSize: 1234,
+			Name:     v4MainIPSetName,
+			Family:   IPFamilyV4,
+			Type:     IPSetTypeHashIP,
+			Revision: supportedMockRevision,
+			MaxSize:  1234,
 		}
 	}
+
+	if meta.Revision > supportedMockRevision {
+		result = fmt.Errorf("revision %v not supported", meta.Revision)
+		return
+	}
+
+	writef("Name: %s\n", c.SetName)
 	writef("Type: %s\n", meta.Type)
+	writef("Revision: %d\n", meta.Revision)
 	if meta.Type == IPSetTypeBitmapPort {
 		writef("Header: family %s range %d-%d\n", meta.Family, meta.RangeMin, meta.RangeMax)
 	} else if meta.Type == "unknown:type" {