Fix Pushpin Process Context and make log output less noisy

Author: harmony7

pkg/commands/compute/serve.go

@@ -600,22 +600,22 @@ func (c *ServeCommand) GetPushpinProxyPort(out io.Writer) (uint16, error) {
 		}
 		pushpinProxyPort = uint16(pushpinProxyPortInt)
 		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin proxy port from --pushpin-proxy-port flag: %d\n\n", pushpinProxyPort)
+			text.Output(out, "DEBUG: Using Pushpin proxy port from --pushpin-proxy-port flag: %d", pushpinProxyPort)
 		}
 		return pushpinProxyPort, nil
 	}
 
 	pushpinProxyPort = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinProxyPort
 	if pushpinProxyPort != 0 {
 		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin proxy port via `local_server.pushpin.proxy_port` setting: %d\n\n", pushpinProxyPort)
+			text.Output(out, "DEBUG: Using Pushpin proxy port via `local_server.pushpin.proxy_port` setting: %d", pushpinProxyPort)
 		}
 		return pushpinProxyPort, nil
 	}
 
 	pushpinProxyPort = 7677
 	if c.Globals.Verbose() {
-		text.Info(out, "Using default Pushpin proxy port %d\n\n", pushpinProxyPort)
+		text.Output(out, "DEBUG: Using default Pushpin proxy port %d", pushpinProxyPort)
 	}
 	return pushpinProxyPort, nil
 }
@@ -640,22 +640,22 @@ func (c *ServeCommand) GetPushpinPublishPort(out io.Writer) (uint16, error) {
 		}
 		pushpinPublishPort = uint16(pushpinPublishPortInt)
 		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin publish handler port from --pushpin-publish-port flag: %d\n\n", pushpinPublishPort)
+			text.Output(out, "DEBUG: Using Pushpin publish handler port from --pushpin-publish-port flag: %d", pushpinPublishPort)
 		}
 		return pushpinPublishPort, nil
 	}
 
 	pushpinPublishPort = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPublishPort
 	if pushpinPublishPort != 0 {
 		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin publish handler port via `local_server.pushpin.publish_port` setting: %d\n\n", pushpinPublishPort)
+			text.Output(out, "DEBUG: Using Pushpin publish handler port via `local_server.pushpin.publish_port` setting: %d", pushpinPublishPort)
 		}
 		return pushpinPublishPort, nil
 	}
 
 	pushpinPublishPort = 5561
 	if c.Globals.Verbose() {
-		text.Info(out, "Using default Pushpin publish handler port %d\n\n", pushpinPublishPort)
+		text.Output(out, "DEBUG: Using default Pushpin publish handler port %d", pushpinPublishPort)
 	}
 	return pushpinPublishPort, nil
 }
@@ -669,21 +669,21 @@ func (c *ServeCommand) GetPushpinRunner(out io.Writer) (bin string, err error) {
 	pushpinRunnerBinPath := c.pushpinRunnerBinPath
 	if pushpinRunnerBinPath != "" {
 		if c.Globals.Verbose() {
-			text.Info(out, "Using user provided install of Pushpin runner via --pushpin-path flag: %s\n\n", pushpinRunnerBinPath)
+			text.Output(out, "DEBUG: Using user provided install of Pushpin runner via --pushpin-path flag: %s", pushpinRunnerBinPath)
 		}
 		return filepath.Abs(pushpinRunnerBinPath)
 	}
 
 	pushpinRunnerBinPath = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPath
 	if pushpinRunnerBinPath != "" {
 		if c.Globals.Verbose() {
-			text.Info(out, "Using user provided install of Pushpin runner via `local_server.pushpin.pushpin_path` setting: %s\n\n", pushpinRunnerBinPath)
+			text.Output(out, "DEBUG: Using user provided install of Pushpin runner via `local_server.pushpin.pushpin_path` setting: %s", pushpinRunnerBinPath)
 		}
 		return filepath.Abs(pushpinRunnerBinPath)
 	}
 
 	if c.Globals.Verbose() {
-		text.Info(out, "No --pushpin-path provided, attempting to find 'pushpin' in your PATH...")
+		text.Output(out, "DEBUG: No --pushpin-path provided, attempting to find 'pushpin' in your PATH...")
 	}
 	pushpinRunnerBinPath, err = exec.LookPath("pushpin")
 	if err != nil {
@@ -694,7 +694,7 @@ func (c *ServeCommand) GetPushpinRunner(out io.Writer) (bin string, err error) {
 	}
 
 	if c.Globals.Verbose() {
-		text.Info(out, "Found Pushpin runner via $PATH lookup: %s\n\n", pushpinRunnerBinPath)
+		text.Output(out, "DEBUG: Found Pushpin runner via $PATH lookup: %s", pushpinRunnerBinPath)
 	}
 	return filepath.Abs(pushpinRunnerBinPath)
 }
@@ -767,6 +767,8 @@ func formatPushpinLog(line string) (string, string) {
 // pushpinContext contains information about the instance of Pushpin that is
 // executed when enabled.
 type pushpinContext struct {
+	instanceID       uint32
+	confFilePath     string
 	pushpinRunnerBin string
 	pushpinRunDir    string
 	pushpinLogDir    string
@@ -811,45 +813,77 @@ func (c *pushpinContext) buildPushpinConf() string {
 // needs to eventually be called by the caller to shut down Pushpin.
 func (c *ServeCommand) startPushpin(spinner text.Spinner, out io.Writer) (pushpinContext, error) {
 	text.Output(out, "%s: %s", text.BoldYellow("EXPERIMENTAL"), "Enabling Pushpin support for local testing of Fanout.")
-	text.Break(out)
 
 	pushpinCtx := pushpinContext{}
 
 	var cleanup func()
 
-	err := spinner.Start()
+	// Generate a non-zero instance ID to represent this Pushpin instance and build temporary
+	// files
+	for {
+		p := make([]byte, 4)
+		_, _ = rand.Read(p)
+		pushpinCtx.instanceID = binary.BigEndian.Uint32(p)
+		if pushpinCtx.instanceID != 0 {
+			break
+		}
+	}
+
+	var err error
+	pushpinCtx.proxyPort, err = c.GetPushpinProxyPort(out)
 	if err != nil {
 		return pushpinCtx, err
 	}
-	msg := "Running local Pushpin"
-	spinner.Message(msg + "...")
-
-	spinner.StopMessage(msg)
-	err = spinner.Stop()
+	pushpinCtx.publishPort, err = c.GetPushpinPublishPort(out)
+	if err != nil {
+		return pushpinCtx, err
+	}
+	pushpinCtx.pushpinRunnerBin, err = c.GetPushpinRunner(out)
 	if err != nil {
 		return pushpinCtx, err
 	}
 
 	pwd, _ := os.Getwd()
 	pushpinCtx.pushpinLogDir = filepath.Join(pwd, "pushpin-logs")
 
-	pushpinCtx.proxyPort, err = c.GetPushpinProxyPort(out)
+	pushpinCtx.pushpinRunDir = filepath.Join(
+		os.TempDir(),
+		fmt.Sprintf("pushpin-%08x", pushpinCtx.instanceID),
+	)
+	pushpinCtx.confFilePath = filepath.Join(
+		os.TempDir(),
+		fmt.Sprintf("pushpin-%08x.conf", pushpinCtx.instanceID),
+	)
+	pushpinCtx.routesFilePath = filepath.Join(
+		os.TempDir(),
+		fmt.Sprintf("pushpin-routes-%08x", pushpinCtx.instanceID),
+	)
+
+	text.Break(out)
+
+	err = spinner.Start()
 	if err != nil {
 		return pushpinCtx, err
 	}
-	pushpinCtx.publishPort, err = c.GetPushpinPublishPort(out)
+	msg := "Running local Pushpin"
+	spinner.Message(msg + "...")
+
+	spinner.StopMessage(msg)
+	err = spinner.Stop()
 	if err != nil {
 		return pushpinCtx, err
 	}
-	pushpinCtx.pushpinRunnerBin, err = c.GetPushpinRunner(out)
+
+	pushpinConfContents := pushpinCtx.buildPushpinConf()
+	err = os.WriteFile(pushpinCtx.confFilePath, []byte(pushpinConfContents), 0o600)
 	if err != nil {
-		return pushpinCtx, err
+		return pushpinCtx, fmt.Errorf("error writing config file %s: %w", pushpinCtx.confFilePath, err)
 	}
-	if c.Globals.Verbose() {
-		text.Info(out, "Local pushpin proxy port: %d", pushpinCtx.proxyPort)
-		text.Info(out, "Local pushpin publisher port: %d", pushpinCtx.publishPort)
-		text.Info(out, "Local pushpin other reserved ports: %d - %d", pushpinCtx.publishPort+1, pushpinCtx.publishPort+3)
-		text.Info(out, "Local pushpin runner: %s", pushpinCtx.pushpinRunnerBin)
+
+	pushpinRoutesContents := strings.Join(c.BuildPushpinRoutes(), "\n") + "\n"
+	err = os.WriteFile(pushpinCtx.routesFilePath, []byte(pushpinRoutesContents), 0o600)
+	if err != nil {
+		return pushpinCtx, fmt.Errorf("error writing routes file %s: %w", pushpinCtx.routesFilePath, err)
 	}
 
 	// Pushpin is configured with the following.
@@ -866,63 +900,11 @@ func (c *ServeCommand) startPushpin(spinner text.Spinner, out io.Writer) (pushpi
 	//   - if the backend enables HTTPS, then we enable that
 	//   - if the backend has a path prefix, then we set that up
 	//   - enables WebSocket-over-HTTP
-
 	// The runtime temporary directory, as well as the conf file and routes file
 	// are set up and torn down along with fastly compute serve.
-	var pushpinInstanceID uint32
-	for {
-		p := make([]byte, 4)
-		_, _ = rand.Read(p)
-		pushpinInstanceID = binary.BigEndian.Uint32(p)
-		if pushpinInstanceID != 0 {
-			break
-		}
-	}
-
-	pushpinRunDir := filepath.Join(
-		os.TempDir(),
-		fmt.Sprintf("pushpin-%08x", pushpinInstanceID),
-	)
-	if c.Globals.Verbose() {
-		text.Break(out)
-		text.Info(out, "Pushpin temporary runtime directory is %s", pushpinRunDir)
-	}
-
-	pushpinConfFilePath := filepath.Join(
-		os.TempDir(),
-		fmt.Sprintf("pushpin-%08x.conf", pushpinInstanceID),
-	)
-	pushpinRoutesFilePath := filepath.Join(
-		os.TempDir(),
-		fmt.Sprintf("pushpin-routes-%08x", pushpinInstanceID),
-	)
-
-	if c.Globals.Verbose() {
-		text.Info(out, "Writing config file to %s...", pushpinConfFilePath)
-	}
-
-	pushpinConfContents := pushpinCtx.buildPushpinConf()
-	err = os.WriteFile(pushpinConfFilePath, []byte(pushpinConfContents), 0o600)
-	if err != nil {
-		return pushpinCtx, fmt.Errorf("error writing config file %s: %w", pushpinConfFilePath, err)
-	}
-
-	if c.Globals.Verbose() {
-		text.Info(out, "Writing routes file to %s...", pushpinRoutesFilePath)
-	}
-	pushpinRoutesContents := strings.Join(c.BuildPushpinRoutes(), "\n") + "\n"
-	err = os.WriteFile(pushpinRoutesFilePath, []byte(pushpinRoutesContents), 0o600)
-	if err != nil {
-		return pushpinCtx, fmt.Errorf("error writing routes file %s: %w", pushpinRoutesFilePath, err)
-	}
-
-	if c.Globals.Verbose() {
-		text.Info(out, "Starting local Pushpin...")
-		text.Break(out)
-	}
 
 	args := []string{
-		fmt.Sprintf("--config=%s", pushpinConfFilePath),
+		fmt.Sprintf("--config=%s", pushpinCtx.confFilePath),
 		"--verbose",
 	}
 
@@ -940,17 +922,17 @@ func (c *ServeCommand) startPushpin(spinner text.Spinner, out io.Writer) (pushpi
 				killProcessTree(pushpinCmd.Process.Pid)
 			}
 			if c.Globals.Verbose() {
-				text.Output(out, "removing %s", pushpinRunDir)
+				text.Output(out, "removing %s", pushpinCtx.pushpinRunDir)
 			}
-			_ = os.RemoveAll(pushpinRunDir)
+			_ = os.RemoveAll(pushpinCtx.pushpinRunDir)
 			if c.Globals.Verbose() {
-				text.Output(out, "deleting %s", pushpinConfFilePath)
+				text.Output(out, "deleting %s", pushpinCtx.confFilePath)
 			}
-			_ = os.Remove(pushpinConfFilePath)
+			_ = os.Remove(pushpinCtx.confFilePath)
 			if c.Globals.Verbose() {
-				text.Output(out, "deleting %s", pushpinRoutesFilePath)
+				text.Output(out, "deleting %s", pushpinCtx.routesFilePath)
 			}
-			_ = os.Remove(pushpinRoutesFilePath)
+			_ = os.Remove(pushpinCtx.routesFilePath)
 			cancel()
 		})
 	}
@@ -977,8 +959,13 @@ func (c *ServeCommand) startPushpin(spinner text.Spinner, out io.Writer) (pushpi
 
 	// Start Pushpin
 	if c.Globals.Verbose() {
-		text.Info(out, "Starting Pushpin runner in the background...")
 		text.Output(out, "%s: %s", text.BoldYellow("Pushpin command"), strings.Join(pushpinCmd.Args, " "))
+		text.Output(out, "%s: %d", text.BoldYellow("Pushpin proxy port"), pushpinCtx.proxyPort)
+		text.Output(out, "%s: %d", text.BoldYellow("Pushpin publisher port"), pushpinCtx.publishPort)
+		text.Output(out, "%s: %d - %d", text.BoldYellow("Pushpin other reserved ports"), pushpinCtx.publishPort+1, pushpinCtx.publishPort+3)
+		text.Output(out, "%s: %s", text.BoldYellow("Pushpin temporary runtime directory"), pushpinCtx.pushpinRunDir)
+		text.Output(out, "%s: %s", text.BoldYellow("Pushpin conf file"), pushpinCtx.confFilePath)
+		text.Output(out, "%s: %s", text.BoldYellow("Pushpin routes file"), pushpinCtx.routesFilePath)
 	}
 	if err := pushpinCmd.Start(); err != nil {
 		return pushpinCtx, fmt.Errorf("failed to start Pushpin runner: %w", err)
@@ -1018,14 +1005,12 @@ func (c *ServeCommand) startPushpin(spinner text.Spinner, out io.Writer) (pushpi
 	if err != nil {
 		return pushpinCtx, fsterr.RemediationError{
 			Inner:       err,
-			Remediation: fmt.Sprintf("A process may already be running on port %d.", pushpinCtx.proxyPort),
+			Remediation: fmt.Sprintf("Check that your disk isn't full and that a process isn't already running on ports %d or %d - %d.", pushpinCtx.proxyPort, pushpinCtx.publishPort, pushpinCtx.publishPort+3),
 		}
 	}
 
-	if c.Globals.Verbose() {
-		text.Info(out, "Local Pushpin started on port %d.", pushpinCtx.proxyPort)
-		text.Break(out)
-	}
+	text.Success(out, "Local Pushpin started.")
+	text.Break(out)
 
 	return pushpinCtx, nil
 }