Merge pull request #116 from scrapli/limit-channel-re-search-depth

refactor: limit channel search depth to not be very slow and dumb

.clab/topo-ci.yaml

@@ -9,4 +9,5 @@ topology:
     srl:
       kind: srl
       mgmt_ipv4: 172.20.20.16
-      mgmt_ipv6: 2001:172:20:20::16
+      mgmt_ipv6: 2001:172:20:20::16
+      startup-config: configs/srl-startup.json

.clab/topo-full.yaml

@@ -45,6 +45,7 @@ topology:
       kind: srl
       mgmt_ipv4: 172.20.20.16
       mgmt_ipv6: 2001:172:20:20::16
+      startup-config: configs/srl-startup.json
   links:
     # forces "front panel port" for ceos, without this there is no ip routing
     - endpoints: ["ceos:eth1", "ceos:eth2"]

channel/channel.go

@@ -17,12 +17,15 @@ const (
 	// DefaultTimeoutOpsSeconds is the default time value for operations -- 60 seconds.
 	DefaultTimeoutOpsSeconds = 60
 	// DefaultReadDelayMicroSeconds is the default value for the delay between reads of the
-	// transport -- 5 microseconds.
-	DefaultReadDelayMicroSeconds = 5
+	// transport -- 100 microseconds. Going very low is likely to lead to very high cpu and not
+	// yield any recognizable gains, so be careful changing this!
+	DefaultReadDelayMicroSeconds = 250
 	// DefaultReturnChar is the character used to send an "enter" key to the device, "\n".
 	DefaultReturnChar = "\n"
-
-	redacted = "redacted"
+	// DefaultPromptSearchDepth -- is the default depth to search for the prompt in the received
+	// bytes.
+	DefaultPromptSearchDepth = 1_000
+	redacted                 = "redacted"
 )
 
 var (
@@ -57,8 +60,9 @@ func NewChannel(
 		PasswordPattern:   patterns.password,
 		PassphrasePattern: patterns.passphrase,
 
-		PromptPattern: getPromptPattern(),
-		ReturnChar:    []byte(DefaultReturnChar),
+		PromptSearchDepth: DefaultPromptSearchDepth,
+		PromptPattern:     getPromptPattern(),
+		ReturnChar:        []byte(DefaultReturnChar),
 
 		done: make(chan bool),
 
@@ -96,8 +100,9 @@ type Channel struct {
 	PasswordPattern   *regexp.Regexp
 	PassphrasePattern *regexp.Regexp
 
-	PromptPattern *regexp.Regexp
-	ReturnChar    []byte
+	PromptSearchDepth int
+	PromptPattern     *regexp.Regexp
+	ReturnChar        []byte
 
 	done chan bool
 

channel/read.go

@@ -94,6 +94,22 @@ func (c *Channel) ReadUntilInput(b []byte) ([]byte, error) {
 	return c.ReadUntilExplicit(b)
 }
 
+func (c *Channel) processReadBuf(rb []byte) []byte {
+	if len(rb) <= c.PromptSearchDepth {
+		return rb
+	}
+
+	prb := rb[len(rb)-c.PromptSearchDepth:]
+
+	partitionIdx := bytes.Index(prb, []byte("\n"))
+
+	if partitionIdx > 0 {
+		prb = prb[partitionIdx:]
+	}
+
+	return prb
+}
+
 // ReadUntilPrompt reads bytes out of the channel Q object until the channel PromptPattern regex
 // pattern is seen in the output. Once that pattern is seen, all read bytes are returned.
 func (c *Channel) ReadUntilPrompt() ([]byte, error) {
@@ -104,6 +120,7 @@ func (c *Channel) ReadUntilPrompt() ([]byte, error) {
 		case err := <-c.Errs:
 			return nil, err
 		default:
+			time.Sleep(c.ReadDelay)
 		}
 
 		nb := c.Q.Dequeue()
@@ -114,7 +131,7 @@ func (c *Channel) ReadUntilPrompt() ([]byte, error) {
 
 		rb = append(rb, nb...)
 
-		if c.PromptPattern.Match(rb) {
+		if c.PromptPattern.Match(c.processReadBuf(rb)) {
 			c.l.Debugf("channel read %#v", string(rb))
 
 			return rb, nil
@@ -132,6 +149,7 @@ func (c *Channel) ReadUntilAnyPrompt(prompts []*regexp.Regexp) ([]byte, error) {
 		case err := <-c.Errs:
 			return nil, err
 		default:
+			time.Sleep(c.ReadDelay)
 		}
 
 		nb := c.Q.Dequeue()
@@ -162,6 +180,7 @@ func (c *Channel) ReadUntilExplicit(b []byte) ([]byte, error) {
 		case err := <-c.Errs:
 			return nil, err
 		default:
+			time.Sleep(c.ReadDelay)
 		}
 
 		nb := c.Q.Dequeue()

driver/network/sendcommand_test.go

@@ -226,3 +226,75 @@ func TestSendCommandFunctional(t *testing.T) {
 		}
 	}
 }
+
+func TestSendCommandFunctionalGiant(t *testing.T) {
+	testName := "send-command-giant"
+	platformName := platform.NokiaSrl
+
+	if !*functional {
+		t.Skip("skip: functional tests skipped without the '-functional' flag being passed")
+	}
+
+	t.Logf("%s: starting", testName)
+
+	for _, transportName := range []string{transport.SystemTransport, transport.StandardTransport} {
+		d := prepareFunctionalDriver(t, testName, platformName, transportName)
+
+		r, err := d.SendCommand("info from running")
+		if err != nil {
+			t.Errorf(
+				"%s: encountered error running network Driver SendCommand (giant), error: %s",
+				testName,
+				err,
+			)
+		}
+
+		if r.Failed != nil {
+			t.Fatalf("%s: response object indicates failure",
+				testName)
+		}
+
+		err = d.Close()
+		if err != nil {
+			t.Fatalf("%s: failed closing connection",
+				testName)
+		}
+
+		actualOut := r.Result
+
+		if *update {
+			writeGoldenFunctional(
+				t,
+				fmt.Sprintf("%s-%s-%s", testName, platformName, transportName),
+				actualOut,
+			)
+		}
+
+		cleanF := util.GetCleanFunc(platformName)
+
+		expectedOut := readFile(
+			t,
+			fmt.Sprintf("golden/%s-%s-%s-out.txt", testName, platformName, transportName),
+		)
+
+		if !cmp.Equal(
+			cleanF(actualOut),
+			cleanF(string(expectedOut)),
+		) {
+			t.Fatalf(
+				"%s: actual and expected outputs do not match\nactual: %s\nexpected:%s",
+				testName,
+				cleanF(actualOut),
+				cleanF(string(expectedOut)),
+			)
+		}
+
+		// in seconds; realistically this looks like it finishes in ~10-12s pretty consistently in
+		// local tests, but we need to make sure it doesn't go kaboom in ci as well. also note that
+		// this is usually ran w/ race flag so that slows things down even more.
+		if r.ElapsedTime > 30 {
+			t.Fatalf("%s: test completed but was greater than maximum expected duration, took %fs",
+				testName, r.ElapsedTime)
+		}
+	}
+}