roachtest: extend github issue poster to include ip node mapping

Quality of life improvement, previously to find this information, you would need
to find the name of the cluster in test.log, then use that cluster name to find
it's cluster creation log in artifacts_dir/_runner-logs/cluster-create.  Now
this information will be available in the github issue.

Resolves #138356

Epic: None
Release note: None

Author: williamchoe3

pkg/cmd/bazci/githubpost/issues/condense.go

@@ -55,6 +55,13 @@ type FatalNodeRoachtest struct {
 	FatalLogs string
 }
 
+// NodeToIpMappingRoachtest contains the node to ip mapping from a roachtest
+// cluster
+type NodeToIpMappingRoachtest struct {
+	Message,
+	NodeToIpMapping string
+}
+
 // A CondensedMessage is a test log output garnished with useful helper methods
 // that extract concise information for seamless debugging.
 type CondensedMessage string
@@ -67,7 +74,16 @@ var fatalRE = regexp.MustCompile(`(?ms)(^F\d{6}.*?\n)(goroutine \d+.*?\n)\n`)
 var crasherRE = regexp.MustCompile(`(?s)( *rsg_test.go:\d{3}: Crash detected:.*?\n)(.*?;\n)`)
 var reproRE = regexp.MustCompile(`(?s)( *rsg_test.go:\d{3}: To reproduce, use schema:)`)
 
-var roachtestNodeFatalRE = regexp.MustCompile(`(?ms)\A(.*?\n)((?:^F\d{6}\b[^\n]*(?:\n|$))+)`)
+var nodeFatalRoachtestRE = regexp.MustCompile(`(?ms)\A(.*?\n)((?:^F\d{6}\b[^\n]*(?:\n|$))+)`)
+
+// nodeToIpRoachtestRE matches an entire Markdown table block
+// - including header, separator, and data rows:
+// - handles an arbitrary number of columns
+//
+// | Node | Public IP | Private IP |
+// | --- | --- | --- |
+// | node-0001 | 1.1.1.0 | 1.1.1.1 |
+var nodeToIpRoachtestRE = regexp.MustCompile(`(?m)^[[:space:]]*\|(?:[^|\n]*\|)+[[:space:]]*\n^[[:space:]]*\|(?:[[:space:]]*:?-{3,}:?[[:space:]]*\|)+[[:space:]]*\n(?:^[[:space:]]*\|(?:[^|\n]*\|)+[[:space:]]*(?:\n|$))+`)
 
 // FatalOrPanic constructs a FatalOrPanic. If no fatal or panic occurred in the
 // test, ok=false is returned.
@@ -112,17 +128,35 @@ func (s CondensedMessage) RSGCrash(lineLimit int) (c RSGCrash, ok bool) {
 // ok=false is returned
 func (s CondensedMessage) FatalNodeRoachtest() (fnr FatalNodeRoachtest, ok bool) {
 	ss := string(s)
-	if matches := roachtestNodeFatalRE.FindStringSubmatchIndex(ss); matches != nil {
+	if matches := nodeFatalRoachtestRE.FindStringSubmatchIndex(ss); matches != nil {
 		if len(matches) != 6 {
 			return FatalNodeRoachtest{}, false
 		}
-		fnr.Message = ss[matches[2] : matches[3]-1]
+		msg := ss[matches[2]:matches[3]]
+		fnr.Message = strings.TrimRight(msg, "\n")
 		fnr.FatalLogs = ss[matches[4]:matches[5]]
 		return fnr, true
 	}
 	return FatalNodeRoachtest{}, false
 }
 
+// NodeToIpMappingRoachtest constructs a NodeToIpMappingRoachtest which is
+// used to construct an issue that contains cluster information
+func (s CondensedMessage) NodeToIpMappingRoachtest() (nodeIpMap NodeToIpMappingRoachtest, ok bool) {
+	ss := string(s)
+	if matches := nodeToIpRoachtestRE.FindStringSubmatchIndex(ss); matches != nil {
+		if len(matches) != 2 {
+			// only expecting a single match
+			return NodeToIpMappingRoachtest{}, false
+		}
+		msg := ss[0:matches[0]]
+		nodeIpMap.Message = strings.TrimRight(msg, "\n")
+		nodeIpMap.NodeToIpMapping = ss[matches[0]:matches[1]]
+		return nodeIpMap, true
+	}
+	return NodeToIpMappingRoachtest{}, false
+}
+
 // String calls .Digest(30).
 func (s CondensedMessage) String() string {
 	return s.Digest(30)

pkg/cmd/bazci/githubpost/issues/formatter_unit.go

@@ -74,6 +74,17 @@ func (unitTestFormatterTyp) Body(r *Renderer, data TemplateData) error {
 		r.CodeBlock("", fnr.Message)
 		r.Escaped("Fatal entries found in Cockroach logs:")
 		r.CodeBlock("", fnr.FatalLogs)
+		if nodeIpMap, ok := data.CondensedMessage.NodeToIpMappingRoachtest(); ok {
+			r.Escaped("Cluster Node to Ip Mapping:")
+			r.nl()
+			r.Escaped(nodeIpMap.NodeToIpMapping)
+		}
+	} else if nodeIpMap, ok := data.CondensedMessage.NodeToIpMappingRoachtest(); ok {
+		r.Escaped("Failed with:")
+		r.CodeBlock("", nodeIpMap.Message)
+		r.Escaped("Cluster Node to Ip Mapping:")
+		r.nl()
+		r.Escaped(nodeIpMap.NodeToIpMapping)
 	} else {
 		r.CodeBlock("", data.CondensedMessage.Digest(50))
 	}

pkg/cmd/roachtest/cluster.go

@@ -797,7 +797,7 @@ type clusterFactory struct {
 	// counter is incremented with every new cluster. It's used as part of the cluster's name.
 	// Accessed atomically.
 	counter atomic.Uint64
-	// The registry with whom all clustered will be registered.
+	// The registry with whom all clusters will be registered.
 	r *clusterRegistry
 	// artifactsDir is the directory in which the cluster creation log file will be placed.
 	artifactsDir string
@@ -977,7 +977,7 @@ func (f *clusterFactory) newCluster(
 		if i > 1 {
 			retryStr = "-retry" + strconv.Itoa(i-1)
 		}
-		logPath := filepath.Join(f.artifactsDir, runnerLogsDir, "cluster-create", genName+retryStr+".log")
+		logPath := filepath.Join(f.artifactsDir, runnerLogsDir, clusterCreateDir, genName+retryStr+".log")
 		l, err := logger.RootLogger(logPath, teeOpt)
 		if err != nil {
 			log.Fatalf("%v", err)

pkg/cmd/roachtest/github_test.go

@@ -121,6 +121,7 @@ func TestCreatePostRequest(t *testing.T) {
 	type githubIssueOpts struct {
 		failures        []failure
 		loadTeamsFailed bool
+		message         string
 	}
 
 	datadriven.Walk(t, datapathutils.TestDataPath(t, "github"), func(t *testing.T, path string) {
@@ -168,7 +169,7 @@ func TestCreatePostRequest(t *testing.T) {
 					// on where this test is run and prone to flaking.
 					fmt.Fprintf(&b, "%v", f.squashedErr)
 				}
-				message := b.String()
+				message := b.String() + testCase.message
 
 				params := getTestParameters(ti, issueInfo.cluster, issueInfo.vmCreateOpts)
 				req, err := github.createPostRequest(
@@ -225,10 +226,7 @@ func TestCreatePostRequest(t *testing.T) {
 							// Lose the error object which should make our flake detection fail.
 							refError = errors.Newf("%s", redact.SafeString(refError.Error()))
 						case "node-fatal":
-							refError = errors.Newf(`(monitor.go:267).Wait: monitor failure: dial tcp 127.0.0.1:29000: connect: connection refused
-test artifacts and logs in: artifacts/roachtest/manual/monitor/test-failure/node-fatal-explicit-monitor/cpu_arch=arm64/run_1
-F250826 19:49:07.194443 3106 sql/sem/builtins/builtins.go:6063 ⋮ [T1,Vsystem,n1,client=127.0.0.1:54552,hostssl,user=‹roachprod›] 250  force_log_fatal(): ‹oops›
-`)
+							refError = errors.Newf(`(monitor.go:267).Wait: monitor failure: dial tcp 127.0.0.1:29000: connect: connection refused`)
 						}
 					}
 				}
@@ -251,6 +249,18 @@ F250826 19:49:07.194443 3106 sql/sem/builtins/builtins.go:6063 ⋮ [T1,Vsystem,n
 				ti.spec.CockroachBinary = registry.RuntimeAssertionsCockroach
 			case "set-coverage-enabled-build":
 				ti.goCoverEnabled = true
+			case "add-additional-info":
+				msg_type := d.CmdArgs[0].Vals[0]
+				switch msg_type {
+				case "ip-node-info":
+					testCase.message = fmt.Sprintf("%s\n%s", testCase.message, `| Node | Public IP | Private IP |
+| --- | --- | --- |
+| teamcity-1758834520-01-n1cpu4-0001 | 34.139.44.53 | 10.142.0.2 |`)
+				case "fatal-logs":
+					testCase.message = fmt.Sprintf("%s\n%s", testCase.message, `F250826 19:49:07.194443 3106 sql/sem/builtins/builtins.go:6063 ⋮ [T1,Vsystem,n1,client=127.0.0.1:54552,hostssl,user=‹roachprod›] 250  force_log_fatal(): ‹oops›`)
+				default:
+					return fmt.Sprintf("unknown additional info argument: %s", msg_type)
+				}
 			}
 
 			return "ok"

pkg/cmd/roachtest/main.go

@@ -56,6 +56,10 @@ const (
 	// runnerLogsDir is the dir under the artifacts root where the test runner log
 	// and other runner-related logs (i.e. cluster creation logs) will be written.
 	runnerLogsDir = "_runner-logs"
+
+	// clusterCreateDir is the dir under runnerLogsDir where cluster creation
+	// related logs will be written
+	clusterCreateDir = "cluster-create"
 )
 
 func main() {

pkg/cmd/roachtest/roachtestutil/utils.go

@@ -395,3 +395,42 @@ var ClusterSettingRetryOpts = retry.Options{
 	MaxBackoff:     5 * time.Second,
 	MaxRetries:     5,
 }
+
+var (
+	errEmptyTableData = errors.New("empty table data")
+	errMismatchedCols = errors.New("row has mismatched number of columns")
+)
+
+// ToMarkdownTable renders a 2D list of strings as a Markdown table. Assumes
+// header is the first entry.
+//
+// Example:
+//
+//	| Name    | Age | City          |
+//	| ---     | --- | ---           |
+//	| Alice   | 30  | New York      |
+//	| Bob     | 25  | San Francisco |
+//	| Charlie | 35  | Chicago       |
+func ToMarkdownTable(data [][]string) (string, error) {
+	if len(data) == 0 || len(data[0]) == 0 {
+		return "", errEmptyTableData
+	}
+	var sb strings.Builder
+	// Write header row
+	sb.WriteString("| " + strings.Join(data[0], " | ") + " |\n")
+	// Write separator row (--- under each header)
+	separators := make([]string, len(data[0]))
+	for i := range separators {
+		separators[i] = "---"
+	}
+	sb.WriteString("| " + strings.Join(separators, " | ") + " |\n")
+	// Write remaining rows
+	for i, row := range data[1:] {
+		if len(data[0]) != len(row) {
+			return "", fmt.Errorf("%w: row %d has %d columns, expected %d",
+				errMismatchedCols, i, len(row), len(data[0]))
+		}
+		sb.WriteString("| " + strings.Join(row, " | ") + " |\n")
+	}
+	return sb.String(), nil
+}

pkg/cmd/roachtest/roachtestutil/utils_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCmdLogFileName(t *testing.T) {
@@ -27,3 +28,54 @@ func TestCmdLogFileName(t *testing.T) {
 		cmdLogFileName(ts, nodes, "./cockroach bla --foo bar"),
 	)
 }
+
+func TestToMarkdownTable(t *testing.T) {
+	tests := []struct {
+		name        string
+		input       [][]string
+		expectedOut string
+		expectedErr error
+	}{
+		{
+			name: "valid table",
+			input: [][]string{
+				{"Name", "Age", "City"},
+				{"Alice", "30", "New York"},
+				{"Bob", "25", "San Francisco"},
+			},
+			expectedOut: `| Name | Age | City |
+| --- | --- | --- |
+| Alice | 30 | New York |
+| Bob | 25 | San Francisco |
+`,
+			expectedErr: nil,
+		},
+		{
+			name:        "empty data",
+			input:       [][]string{},
+			expectedOut: "",
+			expectedErr: errEmptyTableData,
+		},
+		{
+			name: "row with wrong number of columns",
+			input: [][]string{
+				{"Name", "Age", "City"},
+				{"Alice", "30"},
+			},
+			expectedOut: "",
+			expectedErr: errMismatchedCols,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out, err := ToMarkdownTable(tt.input)
+			if tt.expectedErr != nil {
+				require.ErrorIs(t, err, tt.expectedErr, "expected %v, got %v", tt.expectedErr, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.expectedOut, out)
+			}
+		})
+	}
+}

pkg/cmd/roachtest/test_impl.go

@@ -141,9 +141,13 @@ type testImpl struct {
 		// in case github issue posting is disabled.
 		extraParams map[string]string
 
-		// githubMessage contains additional message information that will be
-		// passed to github.MaybePost
-		githubMessage string
+		// githubFatalLogs contains node fatal logs that will be passed to
+		// github.MaybePost
+		githubFatalLogs string
+
+		// githubIpToNodeMapping contains the ip to node map that will be passed to
+		// github.MaybePost
+		githubIpToNodeMapping string
 	}
 	// Map from version to path to the cockroach binary to be used when
 	// mixed-version test wants a binary for that binary. If a particular version
@@ -562,16 +566,45 @@ func (t *testImpl) failureMsg() string {
 	return b.String()
 }
 
-func (t *testImpl) getGithubMessage() string {
+func (t *testImpl) getGithubFatalLogs() string {
 	t.mu.RLock()
 	defer t.mu.RUnlock()
-	return t.mu.githubMessage
+	return t.mu.githubFatalLogs
 }
 
-func (t *testImpl) appendGithubMessage(msg string) {
+func (t *testImpl) appendGithubFatalLogs(msg string) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
-	t.mu.githubMessage += msg
+	t.mu.githubFatalLogs += msg
+}
+
+func (t *testImpl) getGithubIpToNodeMapping() string {
+	t.mu.RLock()
+	defer t.mu.RUnlock()
+	return t.mu.githubIpToNodeMapping
+}
+
+func (t *testImpl) appendGithubIpToNodeMapping(msg string) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	t.mu.githubIpToNodeMapping += msg
+}
+
+// getGithubMessage abstracts details on how the Github message is constructed
+// TODO(wchoe): Currently, all information we want to include in the Github
+// message must be passed in as a single string. This is not ideal, especially
+// as we add more information or want to format based on the failure or what
+// information is available. #157021 aims to address this.
+func (t *testImpl) getGithubMessage(failureMsg string) string {
+	githubMsg := failureMsg
+	if githubFatalLogs := t.getGithubFatalLogs(); githubFatalLogs != "" {
+		githubMsg = fmt.Sprintf("%s\n%s", githubMsg, githubFatalLogs)
+	}
+	if githubIpToNodeMapping := t.getGithubIpToNodeMapping(); githubIpToNodeMapping != "" {
+		githubMsg = fmt.Sprintf("%s\n%s", githubMsg, githubIpToNodeMapping)
+	}
+	t.L().Printf("github message: %s", githubMsg)
+	return githubMsg
 }
 
 // failuresMatchingError checks whether the first error in trees of