Skip to content

Commit

Permalink
Merge #141430
Browse files Browse the repository at this point in the history
141430: roachtest: run post test health checks on system r=srosenberg a=DarrylWong

This changes the post test assertions to always run the health checks on the system tenant (KV pod). Previously it would run these health checks on the default virtual cluster, which may be a SQL pod. We run consistency checks on the system tenant so we care about the liveness of the system tenant here.

Fixes: #140774


-----

After this change, running `decomission/mixed-versions`:

```
test-post-assertions: 2025/02/13 16:23:02 cluster.go:2577: running cmd `./cockroach auth-session lo...` on nodes [:1]; details in run_162302.356113000_n1_cockroach-authsessio.log
test-post-assertions: 2025/02/13 16:23:17 test_runner.go:1502: n1: https://127.0.0.1:29001/health?ready=1 status=503 body={
  "error": "node is shutting down",
  "code": 14,
  "message": "node is shutting down",
  "details": [
  ]
}
test-post-assertions: 2025/02/13 16:23:17 test_runner.go:1509: n2: https://127.0.0.1:29005/health?ready=1 status=200 ok
test-post-assertions: 2025/02/13 16:23:17 test_runner.go:1509: n3: https://127.0.0.1:29003/health?ready=1 status=200 ok
test-post-assertions: 2025/02/13 16:23:17 test_runner.go:1509: n4: https://127.0.0.1:29007/health?ready=1 status=200 ok
test-post-assertions: 2025/02/13 16:23:17 test_runner.go:1525: running validation checks on node 2 (<10m)
```

We can see it correctly identifies node 1 as shutting down and runs the validation checks on node 2. It also only attempts one `auth-session login`.

Co-authored-by: DarrylWong <[email protected]>
  • Loading branch information
craig[bot] and DarrylWong committed Feb 14, 2025
2 parents 9f7bab1 + f33b7c5 commit 69f935d
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
3 changes: 2 additions & 1 deletion pkg/cmd/roachtest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,8 @@ func (c *clusterImpl) HealthStatus(
return nil, nil // unit tests
}

adminAddrs, err := c.ExternalAdminUIAddr(ctx, l, nodes)
// Make sure we run the health checks on the KV pod.
adminAddrs, err := c.ExternalAdminUIAddr(ctx, l, nodes, option.VirtualClusterName(install.SystemInterfaceName))
if err != nil {
return nil, errors.WithDetail(err, "Unable to get admin UI address(es)")
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/cmd/roachtest/roachtestutil/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ func (r *RoachtestHTTPClient) addCookies(ctx context.Context, cookieUrl string)
if !r.cluster.IsSecure() {
return nil
}

r.mu.Lock()
defer r.mu.Unlock()
// If we haven't extracted the sessionID yet, do so.
if r.sessionID == "" {
id, err := getSessionID(ctx, r.cluster, r.l, r.cluster.All(), r.virtualClusterName)
Expand Down Expand Up @@ -169,8 +172,6 @@ func (r *RoachtestHTTPClient) addCookies(ctx context.Context, cookieUrl string)
// SetCookies is a helper that checks if a client.CookieJar exists and creates
// one if it doesn't. It then sets the provided cookies through CookieJar.SetCookies.
func (r *RoachtestHTTPClient) SetCookies(u *url.URL, cookies []*http.Cookie) error {
r.mu.Lock()
defer r.mu.Unlock()
if r.client.Jar == nil {
jar, err := cookiejar.New(nil)
if err != nil {
Expand Down

0 comments on commit 69f935d

Please sign in to comment.