diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go index e5100e9c22d..d634eb88c9c 100644 --- a/felix/ipsets/ipsets.go +++ b/felix/ipsets/ipsets.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2023 Tigera, Inc. All rights reserved. +// Copyright (c) 2017-2024 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. @@ -388,19 +388,95 @@ func (s *IPSets) tryResync() (err error) { }).Debug("Finished IPSets resync") }() - // Start an 'ipset list' child process, which will emit output of the following form: + // Figure out if debug logging is enabled so we can disable some expensive-to-calculate logs + // in the tight loop below if they're not going to be emitted. This speeds up the loop + // by a factor of 3-4x! + debug := log.GetLevel() >= log.DebugLevel + + // Clear the dataplane metadata view, we'll build it back up again as we + // scan. + s.setNameToProgrammedMetadata.Dataplane().DeleteAll() + + ipSets, err := s.CalicoIPSets() + if err != nil { + s.logCxt.WithError(err).Error("Failed to get the list of ipsets") + return + } + if debug { + s.logCxt.Debugf("List of ipsets: %v", ipSets) + } + + for _, name := range ipSets { + if debug { + s.logCxt.Debugf("Parsing IP set %v.", name) + } + err = s.resyncIPSet(name) + if err != nil { + s.logCxt.WithError(err).Errorf("Failed to parse ipset %v", name) + return + } + } + + // 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 { + // In the dataplane, we should have updated its members above. + continue + } + if _, ok := s.setNameToAllMetadata[name]; !ok { + // Defensive: this IP set is not in the dataplane, and it's not + // one we are tracking, clean up its member tracker. + log.WithField("name", name).Warn( + "Cleaning up leaked(?) IP set member tracker.") + delete(s.mainSetNameToMembers, name) + continue + } + // We're tracking this IP set, but we didn't find it in the dataplane; + // reset the members set to empty. + members.Dataplane().DeleteAll() + } + + return +} + +func (s *IPSets) CalicoIPSets() ([]string, error) { + // Start an 'ipset list -name' child process, which will emit ipset's name, one at each line: // - // Name: test-100 - // Type: hash:ip - // Revision: 4 - // Header: family inet hashsize 1024 maxelem 65536 - // Size in memory: 224 - // References: 0 - // Members: - // 10.0.0.2 - // 10.0.0.1 + // test-100 + // test-1 + // ... + var ipSets []string + // Run ipset with -name to get the name of all ipsets + err := s.runIPSetList("-name", func(scanner *bufio.Scanner) error { + debug := log.GetLevel() >= log.DebugLevel + for scanner.Scan() { + name := scanner.Text() + // Look up to see if this is one of our IP sets. + if !s.IPVersionConfig.OwnsIPSet(name) { + if debug { + s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.") + } + continue + } + ipSets = append(ipSets, name) + } + return scanner.Err() + }) + if err != nil { + return nil, err + } + return ipSets, nil +} + +func (s *IPSets) resyncIPSet(ipSetName string) error { + // If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets. + // We should prevent this to not hit ipset protocol mismatch from non-calico ipsets. + if ipSetName == "" { + return fmt.Errorf("no ipset name specified") + } + // Start an 'ipset list [name]' child process, which will emit output of the following form: // - // Name: test-1 + // Name: test-1 // Type: hash:ip // Revision: 4 // Header: family inet hashsize 1024 maxelem 65536 @@ -412,219 +488,183 @@ func (s *IPSets) tryResync() (err 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. - cmd := s.newCmd("ipset", "list") - // Grab stdout as a pipe so we can stream through the (potentially very large) output. - out, err := cmd.StdoutPipe() - if err != nil { - s.logCxt.WithError(err).Error("Failed to get pipe for 'ipset list'") - return - } - // Capture error output into a buffer. - var stderr bytes.Buffer - cmd.SetStderr(&stderr) - execStartTime := time.Now() - err = cmd.Start() - if err != nil { - s.logCxt.WithError(err).Error("Failed to start 'ipset list'") - return - } - summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0) - - // Use a scanner to chunk the input into lines. - scanner := bufio.NewScanner(out) - - // Values of the last-seen header fields. - ipSetName := "" - var ipSetType IPSetType - - // Figure out if debug logging is enabled so we can disable some expensive-to-calculate logs - // in the tight loop below if they're not going to be emitted. This speeds up the loop - // by a factor of 3-4x! - debug := log.GetLevel() >= log.DebugLevel - - // Clear the dataplane metadata view, we'll build it back up again as we - // scan. - s.setNameToProgrammedMetadata.Dataplane().DeleteAll() - for scanner.Scan() { - line := scanner.Text() - if debug { - s.logCxt.Debugf("Parsing line: %q", line) - } - if strings.HasPrefix(line, "Name:") { - ipSetName = strings.Split(line, " ")[1] - if debug { - s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.") - } - } - if strings.HasPrefix(line, "Type:") { - ipSetType = IPSetType(strings.Split(line, " ")[1]) + err := s.runIPSetList(ipSetName, func(scanner *bufio.Scanner) error { + debug := log.GetLevel() >= log.DebugLevel + ipSetName := "" + var ipSetType IPSetType + for scanner.Scan() { + line := scanner.Text() if debug { - s.logCxt.WithField("type", ipSetType).Debug("Parsed type of IP set.") + s.logCxt.Debugf("Parsing line: %q", line) } - } - if strings.HasPrefix(line, "Header:") { - // When we hit the Header line we should know the name, and type of the IP set, which lets - // us update the tracker. - if !s.IPVersionConfig.OwnsIPSet(ipSetName) { - s.logCxt.WithField("name", ipSetName).Debug("Skip non-Calico/wrong version IP set.") - continue + if strings.HasPrefix(line, "Name:") { + ipSetName = strings.Split(line, " ")[1] + if debug { + s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.") + } } - parts := strings.Split(line, " ") - meta := dataplaneMetadata{ - Type: ipSetType, + if strings.HasPrefix(line, "Type:") { + ipSetType = IPSetType(strings.Split(line, " ")[1]) + if debug { + s.logCxt.WithField("type", ipSetType).Debug("Parsed type of IP set.") + } } - for idx, p := range parts { - if p == "maxelem" { - if idx+1 >= len(parts) { - log.WithField("line", line).Error( - "Failed to parse ipset list Header line, nothing after 'maxelem'.") - break - } - maxElem, err := strconv.Atoi(parts[idx+1]) - if err != nil { - log.WithError(err).WithField("line", line).Error( - "Failed to parse ipset list Header line.") - break - } - meta.MaxSize = maxElem - break + if strings.HasPrefix(line, "Header:") { + // 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, } - if p == "range" { - if idx+1 >= len(parts) { - log.WithField("line", line).Error( - "Failed to parse ipset list Header line, nothing after 'range'.") + for idx, p := range parts { + if p == "maxelem" { + if idx+1 >= len(parts) { + return fmt.Errorf( + "failed to parse ipset list Header line, nothing after 'maxelem'. line: '%v'", line) + } + maxElem, err := strconv.Atoi(parts[idx+1]) + if err != nil { + return fmt.Errorf( + "Failed to parse ipset list Header line. line: '%v', err: %w", line, err) + } + meta.MaxSize = maxElem break } - // For bitmaps, we see "range 123-456" - rMin, rMAx, err := ParseRange(parts[idx+1]) - if err != nil { - log.WithError(err).WithField("line", line).Error( - "Failed to parse ipset list Header line.") + if p == "range" { + if idx+1 >= len(parts) { + return fmt.Errorf( + "Failed to parse ipset list Header line, nothing after 'range'. line: '%v'", line) + } + // For bitmaps, we see "range 123-456" + rMin, rMAx, err := ParseRange(parts[idx+1]) + if err != nil { + return fmt.Errorf( + "Failed to parse ipset list Header line. line: '%v', err: %w", line, err) + } + meta.RangeMin = rMin + meta.RangeMax = rMAx break } - meta.RangeMin = rMin - meta.RangeMax = rMAx - break } + s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta) } - s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta) - } - if strings.HasPrefix(line, "Members:") { - // Start of a Members entry, following this, there'll be one member per - // line then EOF or a blank line. + if strings.HasPrefix(line, "Members:") { + // Start of a Members entry, following this, there'll be one member per + // line then EOF or a blank line. - // Look up to see if this is one of our IP sets. - if !s.IPVersionConfig.OwnsIPSet(ipSetName) || s.IPVersionConfig.IsTempIPSetName(ipSetName) { - if debug { - s.logCxt.WithField("name", ipSetName).Debug("Skip parsing members of IP set.") - } - for scanner.Scan() { - line := scanner.Bytes() - if len(line) == 0 { - // End of members - break + // Look up to see if this is one of our IP sets. + if !s.IPVersionConfig.OwnsIPSet(ipSetName) || s.IPVersionConfig.IsTempIPSetName(ipSetName) { + if debug { + s.logCxt.WithField("name", ipSetName).Debug("Skip parsing members of IP set.") } + return nil } - ipSetName = "" - ipSetType = "" - continue - } - if !ipSetType.IsValid() { - s.logCxt.WithFields(log.Fields{ - "setName": ipSetName, - "type": string(ipSetType), - }).Warning("Dataplane IP set has unknown type.") - } + if !ipSetType.IsValid() { + s.logCxt.WithFields(log.Fields{ + "setName": ipSetName, + "type": string(ipSetType), + }).Warning("Dataplane IP set has unknown type.") + } - // One of our IP sets; we need to parse its members. - logCxt := s.logCxt.WithField("setName", ipSetName) - memberTracker := s.getOrCreateMemberTracker(ipSetName) - numExtrasExpected := memberTracker.PendingDeletions().Len() - err = memberTracker.Dataplane().ReplaceFromIter(func(f func(k IPSetMember)) error { - for scanner.Scan() { - line := scanner.Text() - if line == "" { - // End of members - break - } - var canonMember IPSetMember - if ipSetType.IsValid() { - canonMember = ipSetType.CanonicaliseMember(line) - } else { - // Unknown type found in dataplane, record it as - // a raw string. Then we'll clean up the IP set - // when we go to sync. - canonMember = rawIPSetMember(line) - } - if debug { - logCxt.WithFields(log.Fields{ - "member": line, - "canon": canonMember, - }).Debug("Found member in dataplane") + // One of our IP sets; we need to parse its members. + logCxt := s.logCxt.WithField("setName", ipSetName) + memberTracker := s.getOrCreateMemberTracker(ipSetName) + numExtrasExpected := memberTracker.PendingDeletions().Len() + err := memberTracker.Dataplane().ReplaceFromIter(func(f func(k IPSetMember)) error { + for scanner.Scan() { + line := scanner.Text() + if line == "" { + // End of members + break + } + var canonMember IPSetMember + if ipSetType.IsValid() { + canonMember = ipSetType.CanonicaliseMember(line) + } else { + // Unknown type found in dataplane, record it as + // a raw string. Then we'll clean up the IP set + // when we go to sync. + canonMember = rawIPSetMember(line) + } + if debug { + logCxt.WithFields(log.Fields{ + "member": line, + "canon": canonMember, + }).Debug("Found member in dataplane") + } + f(canonMember) } - f(canonMember) + return scanner.Err() + }) + if err != nil { + return fmt.Errorf("Failed to read members from 'ipset list'. err: %w", err) } - return scanner.Err() - }) - if err != nil { - logCxt.WithError(err).Error("Failed to read members from 'ipset list'.") - break - } - if numMissing := memberTracker.PendingUpdates().Len(); numMissing > 0 { - logCxt.WithField("numMissing", numMissing).Info( - "Resync found members missing from dataplane.") - } - if numExtras := memberTracker.PendingDeletions().Len() - numExtrasExpected; numExtras > 0 { - logCxt.WithField("numExtras", numExtras).Info( - "Resync found extra members in dataplane.") - } - - s.updateDirtiness(ipSetName) + if numMissing := memberTracker.PendingUpdates().Len(); numMissing > 0 { + logCxt.WithField("numMissing", numMissing).Info( + "Resync found members missing from dataplane.") + } + if numExtras := memberTracker.PendingDeletions().Len() - numExtrasExpected; numExtras > 0 { + logCxt.WithField("numExtras", numExtras).Info( + "Resync found extra members in dataplane.") + } - ipSetName = "" - ipSetType = "" + s.updateDirtiness(ipSetName) + } } + return scanner.Err() + }) + if err != nil { + return err + } + return nil +} + +func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) error) error { + cmd := s.newCmd("ipset", "list", arg) + cmdStr := fmt.Sprintf("ipset list %v", arg) + // Grab stdout as a pipe so we can stream through the (potentially very large) output. + out, err := cmd.StdoutPipe() + if err != nil { + s.logCxt.WithError(err).Errorf("Failed to get pipe for '%v'.", cmdStr) + return err + } + // Capture error output into a buffer. + var stderr bytes.Buffer + cmd.SetStderr(&stderr) + execStartTime := time.Now() + err = cmd.Start() + if err != nil { + s.logCxt.WithError(err).Errorf("Failed to start '%v'.", cmdStr) + return err } + summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0) + + // Use a scanner to chunk the input into lines. + scanner := bufio.NewScanner(out) + parsingErr := parsingFunc(scanner) closeErr := out.Close() err = cmd.Wait() logCxt := s.logCxt.WithField("stderr", stderr.String()) if scanner.Err() != nil { - logCxt.WithError(scanner.Err()).Error("Failed to read 'ipset list' output.") err = scanner.Err() - return + logCxt.WithError(err).Errorf("Failed to read '%v' output.", cmdStr) + return err } if err != nil { - logCxt.WithError(err).Error("Bad return code from 'ipset list'.") - return + logCxt.WithError(err).Errorf("Bad return code from '%v'.", cmdStr) + return err } if closeErr != nil { - err = closeErr - logCxt.WithError(err).Error("Failed to close stdout from 'ipset list'.") - return + logCxt.WithError(closeErr).Errorf("Failed to close stdout from '%v'.", cmdStr) + return closeErr } - - // 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 { - // In the dataplane, we should have updated its members above. - continue - } - if _, ok := s.setNameToAllMetadata[name]; !ok { - // Defensive: this IP set is not in the dataplane, and it's not - // one we are tracking, clean up its member tracker. - log.WithField("name", name).Warn( - "Cleaning up leaked(?) IP set member tracker.") - delete(s.mainSetNameToMembers, name) - continue - } - // We're tracking this IP set, but we didn't find it in the dataplane; - // reset the members set to empty. - members.Dataplane().DeleteAll() + if parsingErr != nil { + logCxt.WithError(parsingErr).Errorf("Failed to process '%v' output.", cmdStr) + return parsingErr } - - return + return nil } func ParseRange(s string) (min int, max int, err error) { @@ -966,13 +1006,38 @@ func (s *IPSets) deleteIPSet(setName string) error { } func (s *IPSets) dumpIPSetsToLog() { - cmd := s.newCmd("ipset", "list") - output, err := cmd.Output() + ipSets, err := s.CalicoIPSets() if err != nil { - s.logCxt.WithError(err).Error("Failed to read IP sets") + s.logCxt.WithError(err).Error("Failed to get the list of IP sets.") return } - s.logCxt.WithField("output", string(output)).Info("Current state of IP sets") + s.logCxt.Infof("Current state of IP sets: %v", strings.Join(ipSets, " ")) + + for _, name := range ipSets { + s.logCxt.Infof("Dumping IP set %v.", name) + + // Start an 'ipset list [name]' child process, which will emit output of the following form: + // + // Name: test-100 + // Type: hash:ip + // Revision: 4 + // Header: family inet hashsize 1024 maxelem 65536 + // Size in memory: 224 + // References: 0 + // Members: + // 10.0.0.2 + // 10.0.0.1 + err := s.runIPSetList(name, func(scanner *bufio.Scanner) error { + for scanner.Scan() { + s.logCxt.Infof("%v", scanner.Text()) + } + return scanner.Err() + }) + if err != nil { + s.logCxt.WithError(err).Errorf("Failed to read ipset %v", name) + continue + } + } } func firstNonNilErr(errs ...error) error { diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go index 10360b296bc..bda7be4aef5 100644 --- a/felix/ipsets/ipsets_test.go +++ b/felix/ipsets/ipsets_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2023 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2024 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. @@ -952,7 +952,7 @@ var _ = Describe("IP sets dataplane", func() { It("shouldn't do any work on resync", func() { dataplane.CmdNames = nil resyncAndApply() - Expect(dataplane.CmdNames).To(ConsistOf("list")) + Expect(dataplane.CmdNames).To(ConsistOf("list", "list")) }) }) @@ -1018,6 +1018,16 @@ var _ = Describe("IP sets dataplane", func() { resyncAndApply() dataplane.ExpectMembers(map[string][]string{"noncali": v4Members1And2}) }) + It("CalicoIPSets() should ignore non-calico IP sets", func() { + dataplane.IPSetMembers["noncali"] = + set.From("10.0.0.1", "10.0.0.2") + dataplane.IPSetMembers[v4MainIPSetName] = + set.From("10.0.0.1", "10.0.0.3", "10.0.0.4") + + ipsets, err := ipsets.CalicoIPSets() + Expect(err).NotTo(HaveOccurred()) + Expect(ipsets).Should(Equal([]string{v4MainIPSetName})) + }) }) var _ = Describe("Standard IPv4 IPVersionConfig", func() { diff --git a/felix/ipsets/utils_for_test.go b/felix/ipsets/utils_for_test.go index 6f54229bdd5..f7babe06af5 100644 --- a/felix/ipsets/utils_for_test.go +++ b/felix/ipsets/utils_for_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2018 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2024 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. @@ -108,11 +108,16 @@ func (d *mockDataplane) newCmd(name string, arg ...string) CmdIface { SetName: name, } case "list": - Expect(len(arg)).To(Equal(1)) - cmd = &listCmd{ + Expect(len(arg)).To(Equal(2)) + command := &listCmd{ Dataplane: d, resultC: make(chan error), + allIpSets: arg[1] == "-name", + } + if !command.allIpSets { + command.SetName = arg[1] } + cmd = command default: Fail(fmt.Sprintf("Unexpected command %v", arg)) } @@ -521,6 +526,7 @@ func (d *destroyCmd) CombinedOutput() ([]byte, error) { type listCmd struct { Dataplane *mockDataplane SetName string + allIpSets bool Stdout *io.PipeWriter resultC chan error } @@ -697,36 +703,41 @@ func (c *listCmd) main() { return } - first := true - for setName, members := range c.Dataplane.IPSetMembers { - if !first { - fmt.Fprint(c.Stdout, "\n") + if c.allIpSets { + for setName := range c.Dataplane.IPSetMembers { + fmt.Fprintf(c.Stdout, "%s\n", setName) } - fmt.Fprintf(c.Stdout, "Name: %s\n", setName) - meta, ok := c.Dataplane.IPSetMetadata[setName] - if !ok { - // Default metadata for IP sets created by tests. - meta = setMetadata{ - Name: v4MainIPSetName, - Family: IPFamilyV4, - Type: IPSetTypeHashIP, - MaxSize: 1234, - } - } - fmt.Fprintf(c.Stdout, "Type: %s\n", meta.Type) - if meta.Type == IPSetTypeBitmapPort { - fmt.Fprintf(c.Stdout, "Header: family %s range %d-%d\n", meta.Family, meta.RangeMin, meta.RangeMax) - } else if meta.Type == "unknown:type" { - fmt.Fprintf(c.Stdout, "Header: floop\n") - } else { - fmt.Fprintf(c.Stdout, "Header: family %s hashsize 1024 maxelem %d\n", meta.Family, meta.MaxSize) + return + } + + members, exists := c.Dataplane.IPSetMembers[c.SetName] + if !exists { + result = fmt.Errorf("ipset %v does not exists", c.SetName) + return + } + fmt.Fprintf(c.Stdout, "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, } - fmt.Fprint(c.Stdout, "Field: foobar\n") // Dummy field, should get ignored. - fmt.Fprint(c.Stdout, "Members:\n") - members.Iter(func(member string) error { - fmt.Fprintf(c.Stdout, "%s\n", member) - return nil - }) - first = false } + fmt.Fprintf(c.Stdout, "Type: %s\n", meta.Type) + if meta.Type == IPSetTypeBitmapPort { + fmt.Fprintf(c.Stdout, "Header: family %s range %d-%d\n", meta.Family, meta.RangeMin, meta.RangeMax) + } else if meta.Type == "unknown:type" { + fmt.Fprintf(c.Stdout, "Header: floop\n") + } else { + fmt.Fprintf(c.Stdout, "Header: family %s hashsize 1024 maxelem %d\n", meta.Family, meta.MaxSize) + } + fmt.Fprint(c.Stdout, "Field: foobar\n") // Dummy field, should get ignored. + fmt.Fprint(c.Stdout, "Members:\n") + members.Iter(func(member string) error { + fmt.Fprintf(c.Stdout, "%s\n", member) + return nil + }) }