Merge #124721

124721: roachprod: fix check for errors in `find-ports` r=srosenberg,herkolategan a=renatolabs

When finding ports, we only check for the `err` return value from `runCmdOnSingleNode`. However, that function only returns an error if there is a string expansion error. For command errors, the caller needs to check `res.Err`, which was not happening before.

Informs: #124715

Co-authored-by: Renato Costa <[email protected]>

pkg/roachprod/install/cluster_synced.go

@@ -1146,14 +1146,20 @@ func defaultCmdOpts(debugName string) RunCmdOptions {
 // - specifying the stdin, stdout, and stderr streams
 // - specifying the remote session options
 // - whether the command should be run with the ROACHPROD env variable (true for all user commands)
+//
+// NOTE: do *not* return a `nil` `*RunResultDetails` in this function:
+// we want to support callers being able to use
+// `errors.CombineErrors(err, res.Err)` when they don't care about the
+// origin of the error.
 func (c *SyncedCluster) runCmdOnSingleNode(
 	ctx context.Context, l *logger.Logger, node Node, cmd string, opts RunCmdOptions,
 ) (*RunResultDetails, error) {
+	var noResult RunResultDetails
 	// Argument template expansion is node specific (e.g. for {store-dir}).
 	e := expander{node: node}
 	expandedCmd, err := e.expand(ctx, l, c, cmd)
 	if err != nil {
-		return nil, errors.WithDetailf(err, "error expanding command: %s", cmd)
+		return &noResult, errors.WithDetailf(err, "error expanding command: %s", cmd)
 	}
 
 	nodeCmd := expandedCmd

pkg/roachprod/install/services.go

@@ -425,8 +425,8 @@ func (c *SyncedCluster) FindOpenPorts(
 	}
 
 	res, err := c.runCmdOnSingleNode(ctx, l, node, buf.String(), defaultCmdOpts("find-ports"))
-	if err != nil {
-		return nil, transientFailure(errors.Wrapf(err, "output:\n%s", res.CombinedOut))
+	if findPortsErr := errors.CombineErrors(err, res.Err); findPortsErr != nil {
+		return nil, transientFailure(errors.Wrapf(findPortsErr, "output:\n%s", res.CombinedOut))
 	}
 	ports, err = stringToIntegers(strings.TrimSpace(res.CombinedOut))
 	if err != nil {