Merge #147958 #147997

147958: roachtest: fix close body r=golgeek,DarrylWong a=herkolategan

Previously, we were attempting to close the body on a nil response. This caused a panic. On an error there is no response, hence no body to close.

Epic: None
Release note: None

147997: rpc,upgrade: add `NodeDialerNoBreaker` interface r=cthumuluru-crdb a=cthumuluru-crdb

Currently, `NodeDialer` interface only has `Dial()` method which most code paths use. But there are also code paths that use `DialNoBreaker()` method. To support those cases, this commit adds a new interface `NodeDialerNoBreaker`.

Epic: CRDB-48923
Informs: #147757
Release note: none

Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Chandra Thumuluru <[email protected]>

Author: 3 people

pkg/cmd/roachtest/roachtestutil/profile.go

@@ -340,7 +340,6 @@ func getProfileWithTimeout(
 		resp, err := client.Get(ctx, url)
 		if err != nil {
 			latestError = err
-			resp.Body.Close()
 			continue
 		}
 

pkg/rpc/rpcbase/nodedialer.go

@@ -21,3 +21,10 @@ const TODODRPC = false
 type NodeDialer interface {
 	Dial(context.Context, roachpb.NodeID, ConnectionClass) (_ *grpc.ClientConn, err error)
 }
+
+// NodeDialerNoBreaker interface defines methods for dialing peer nodes using their
+// node IDs. This interface is similar to NodeDialer but does not check the
+// breaker before dialing.
+type NodeDialerNoBreaker interface {
+	DialNoBreaker(context.Context, roachpb.NodeID, ConnectionClass) (_ *grpc.ClientConn, err error)
+}

pkg/upgrade/upgradecluster/BUILD.bazel

@@ -27,7 +27,6 @@ go_library(
         "//pkg/util/retry",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_redact//:redact",
-        "@org_golang_google_grpc//:grpc",
     ],
 )
 

pkg/upgrade/upgradecluster/cluster.go

@@ -22,7 +22,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/redact"
-	"google.golang.org/grpc"
 )
 
 // Cluster mediates interacting with a cockroach cluster.
@@ -37,7 +36,7 @@ type ClusterConfig struct {
 	NodeLiveness livenesspb.NodeVitalityInterface
 
 	// Dialer constructs connections to other nodes.
-	Dialer NodeDialer
+	Dialer rpcbase.NodeDialer
 
 	// RangeDescScanner paginates through all range descriptors.
 	RangeDescScanner rangedesc.Scanner
@@ -50,12 +49,6 @@ type ClusterConfig struct {
 	DB *kv.DB
 }
 
-// NodeDialer abstracts connecting to other nodes in the cluster.
-type NodeDialer interface {
-	// Dial returns a grpc connection to the given node.
-	Dial(context.Context, roachpb.NodeID, rpcbase.ConnectionClass) (*grpc.ClientConn, error)
-}
-
 // New constructs a new Cluster with the provided dependencies.
 func New(cfg ClusterConfig) *Cluster {
 	return &Cluster{c: cfg}

pkg/upgrade/upgradecluster/helper_test.go

@@ -29,7 +29,7 @@ func (n NoopDialer) Dial(
 	return nil, nil
 }
 
-var _ NodeDialer = NoopDialer{}
+var _ rpcbase.NodeDialer = NoopDialer{}
 
 func TestHelperEveryNode(t *testing.T) {
 	defer leaktest.AfterTest(t)()

pkg/upgrade/upgradecluster/tenant_cluster.go

@@ -129,7 +129,7 @@ import (
 //     again in the next release.
 type TenantCluster struct {
 	// Dialer allows for the construction of connections to other SQL pods.
-	Dialer          NodeDialer
+	Dialer          rpcbase.NodeDialer
 	InstanceReader  *instancestorage.Reader
 	instancesAtBump []sqlinstance.InstanceInfo
 	DB              *kv.DB
@@ -138,7 +138,7 @@ type TenantCluster struct {
 // TenantClusterConfig configures a TenantCluster.
 type TenantClusterConfig struct {
 	// Dialer allows for the construction of connections to other SQL pods.
-	Dialer NodeDialer
+	Dialer rpcbase.NodeDialer
 
 	// InstanceReader is used to retrieve all SQL pods for a given tenant.
 	InstanceReader *instancestorage.Reader