From 9ca28436420faa0d9ab2e78148ed0d4b71632662 Mon Sep 17 00:00:00 2001 From: Mazdak Nasab Date: Wed, 10 Jan 2024 15:27:26 -0800 Subject: [PATCH] Skip non-calico ipsets --- felix/ipsets/ipsets.go | 471 ++++++++++++++++++++------------- felix/ipsets/ipsets_test.go | 4 +- felix/ipsets/utils_for_test.go | 70 ++--- 3 files changed, 322 insertions(+), 223 deletions(-) diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go index e5100e9c22d..e24be4223f3 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,7 +388,91 @@ 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.list(debug) + if err != nil { + s.logCxt.WithError(err).Error("Failed to get the list of ipsets") + return + } + + for _, name := range ipsets { + // 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 + } + err = s.parse(name, debug) + 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) list(debug bool) ([]string, error) { + // Start an 'ipset list -name' child process, which will emit ipset's name, one at each line: + // + // test-100 + // test-1 + // ... + getNames := func(scanner *bufio.Scanner) ([]string, error) { + var ipSets []string + for scanner.Scan() { + ipSets = append(ipSets, scanner.Text()) + } + return ipSets, nil + } + + // Run ipset with -name to get the name of all ipsets + ipSets, err := s.runIpSetList("-name", debug, getNames) + if err != nil { + return nil, err + } + s.logCxt.Debugf("List of ipsets: %v", ipSets) + return ipSets, nil +} + +func (s *IPSets) get(ipSetName string, debug bool) ([]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 nil, fmt.Errorf("no ipset name specified") + } + if debug { + s.logCxt.WithField("setName", ipSetName).Debug("Getting IP set.") + } + // Start an 'ipset list [name]' child process, which will emit output of the following form: // // Name: test-100 // Type: hash:ip @@ -399,232 +483,219 @@ func (s *IPSets) tryResync() (err error) { // Members: // 10.0.0.2 // 10.0.0.1 + getIpSet := func(scanner *bufio.Scanner) ([]string, error) { + var lines []string + for scanner.Scan() { + lines = append(lines, scanner.Text()) + } + return lines, nil + } + + lines, err := s.runIpSetList(ipSetName, debug, getIpSet) + if err != nil { + return nil, err + } + return lines, nil +} + +func (s *IPSets) parse(ipSetName string, debug bool) 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") + } + if debug { + s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.") + } + // Start an 'ipset list [name]' child process, which will emit output of the following form: // - // Name: test-1 + // 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.1 // 10.0.0.2 - // - // 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]) + // 10.0.0.1 + parseIpSet := func(scanner *bufio.Scanner) ([]string, error) { + 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 nil, 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 nil, 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 nil, 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 nil, 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, 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 nil, 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.") + 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) } + } + return nil, nil + } - s.updateDirtiness(ipSetName) + _, err := s.runIpSetList(ipSetName, debug, parseIpSet) + if err != nil { + return err + } + return nil +} - ipSetName = "" - ipSetType = "" - } +func (s *IPSets) runIpSetList(arg string, debug bool, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, 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 nil, 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 nil, err + } + summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0) + + // Use a scanner to chunk the input into lines. + scanner := bufio.NewScanner(out) + res, 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 nil, 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 nil, 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 nil, 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 nil, parsingErr } - - return + return res, nil } func ParseRange(s string) (min int, max int, err error) { @@ -966,13 +1037,33 @@ func (s *IPSets) deleteIPSet(setName string) error { } func (s *IPSets) dumpIPSetsToLog() { - cmd := s.newCmd("ipset", "list") - output, err := cmd.Output() + debug := log.GetLevel() >= log.DebugLevel + ipSets, err := s.list(debug) 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") + output := []string{ + fmt.Sprintf("Lists of IP sets: %v", strings.Join(ipSets, " ")), + "Dump of Calico IP sets:", + } + + for _, name := range ipSets { + // 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 + } + lines, err := s.get(name, debug) + if err != nil { + s.logCxt.WithError(err).Errorf("Failed to read ipset %v", name) + return + } + output = append(output, lines...) + } + s.logCxt.Infof("Current state of IP sets:\n%v", strings.Join(output, "\n")) } func firstNonNilErr(errs ...error) error { diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go index 10360b296bc..ecb59e2a89a 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")) }) }) diff --git a/felix/ipsets/utils_for_test.go b/felix/ipsets/utils_for_test.go index 6f54229bdd5..e81a084ac09 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,10 +108,12 @@ func (d *mockDataplane) newCmd(name string, arg ...string) CmdIface { SetName: name, } case "list": - Expect(len(arg)).To(Equal(1)) + Expect(len(arg)).To(Equal(2)) cmd = &listCmd{ Dataplane: d, resultC: make(chan error), + allIpSets: arg[1] == "-name", + SetName: arg[1], // either ipset name or '-name' to return the list of all ipsets } default: Fail(fmt.Sprintf("Unexpected command %v", arg)) @@ -521,6 +523,7 @@ func (d *destroyCmd) CombinedOutput() ([]byte, error) { type listCmd struct { Dataplane *mockDataplane SetName string + allIpSets bool Stdout *io.PipeWriter resultC chan error } @@ -697,36 +700,41 @@ func (c *listCmd) main() { return } - first := true - for setName, members := range c.Dataplane.IPSetMembers { - if !first { - fmt.Fprint(c.Stdout, "\n") - } - 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, - } + if c.allIpSets { + for setName := range c.Dataplane.IPSetMembers { + fmt.Fprintf(c.Stdout, "%s\n", setName) } - 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 + }) }