Merge #72098 #72237 #72246

72098: roachtest: fix up `mockgen` usage w/ Bazel r=irfansharif a=rickystewart

We were pointing to the checked-in generated files in the Bazel build
here; instead, use `gomock`.

Also clean up the `EXISTING_GO_GENERATE_COMMENTS` in
`build/bazelutil/check.sh`.

Release note: None

72237: kvserver: remove detritus for old change replicas and preemptive snaps r=nvanbenschoten a=tbg

See individual commits.

- kvserver: remove deprecated fields in ChangeReplicasTrigger
- kvserver: improve comment on raft send buffer size
- kvserver: improve comment on split trigger helper
- kvserver: remove TODO for removing DisableEagerReplicaRemoval
- kvserver: remove snapshot CanDecline flag
- kvserver: reflect absence of preemptive snaps in more comments
- kvserver: remove IsPreemptive()
- kvserver: bump snapshotReservationWaitWarnThreshold
- kvserver: add TODO about removing snapshot log sending code.
- kvserver: assert that local descriptor "always" contains the replica
- kvserver: remove env-defaults for snapshot rate limits

Release note: None


72246: ci: make sure `check generated files` output is dumped if check fails r=rail a=rickystewart

Closes #72245.

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>

BUILD.bazel

@@ -50,14 +50,15 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 # tell gazelle to resolve to this second target instead. See
 # pkg/kv/kvclient/rangefeed/BUILD.bazel for an annotated example.
 #
+# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/cmd/roachtest/prometheus //pkg/cmd/roachtest/prometheus:with-mocks
+# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests //pkg/cmd/roachtest/tests:with-mocks
 # gazelle:resolve go github.com/cockroachdb/cockroach/pkg/roachpb //pkg/roachpb:with-mocks
 # gazelle:resolve go github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord //pkg/kv/kvclient/kvcoord:with-mocks
 # gazelle:resolve go github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed //pkg/kv/kvclient/rangefeed:with-mocks
 # gazelle:resolve go github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangecache //pkg/kv/kvclient/rangecache:with-mocks
 
 # See pkg/roachpb/gen/BUILD.bazel for more details.
 #
-# gazelle:resolve go github.com/cockroachdb/cockroach/pkg/roachpb //pkg/roachpb:with-mocks
 # gazelle:resolve proto go roachpb/api.proto //pkg/roachpb:with-mocks
 # gazelle:resolve proto go roachpb/app_stats.proto //pkg/roachpb:with-mocks
 # gazelle:resolve proto go roachpb/data.proto //pkg/roachpb:with-mocks
@@ -117,6 +118,8 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 # gazelle:exclude pkg/testutils/**/testdata/**
 # gazelle:exclude pkg/security/securitytest/embedded.go
 # gazelle:exclude pkg/cmd/roachprod/vm/aws/embedded.go
+# gazelle:exclude pkg/cmd/roachtest/prometheus/mock_generated.go
+# gazelle:exclude pkg/cmd/roachtest/tests/drt_generated.go
 # gazelle:exclude pkg/**/*_string.go
 # gazelle:exclude pkg/geo/wkt/wkt_generated.go
 # gazelle:exclude pkg/sql/schemachanger/scop/backfill_visitor_generated.go

build/bazelutil/check.sh

@@ -3,24 +3,20 @@
 set -euo pipefail
 
 EXISTING_GO_GENERATE_COMMENTS="
-pkg/ccl/sqlproxyccl/throttler/service.go://go:generate mockgen -package=throttler -destination=mocks_generated.go -source=service.go . Service
-pkg/ccl/sqlproxyccl/denylist/service.go://go:generate mockgen -package=denylist -destination=mocks_generated.go -source=service.go . Service
-pkg/ccl/sqlproxyccl/tenant/directory.go://go:generate mockgen -package=tenant -destination=mocks_generated.go . DirectoryClient,Directory_WatchEndpointsClient
 pkg/cmd/roachprod/vm/aws/config.go://go:generate go-bindata -mode 0600 -modtime 1400000000 -pkg aws -o embedded.go config.json old.json
 pkg/cmd/roachprod/vm/aws/config.go://go:generate gofmt -s -w embedded.go
 pkg/cmd/roachprod/vm/aws/config.go://go:generate goimports -w embedded.go
 pkg/cmd/roachprod/vm/aws/config.go://go:generate terraformgen -o terraform/main.tf
-pkg/geo/wkt/wkt.go://go:generate sh generate.sh
+pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster
+pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -source drt.go -package tests -destination drt_generated.go
+pkg/kv/kvclient/kvcoord/transport.go://go:generate mockgen -package=kvcoord -destination=mocks_generated.go . Transport
+pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangecache -destination=mocks_generated.go . RangeDescriptorDB
+pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go .
 pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *lockState concurrency
 pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch
+pkg/roachpb/api.go://go:generate mockgen -package=roachpb -destination=mocks_generated.go . InternalClient,Internal_RangeFeedClient
 pkg/roachpb/batch.go://go:generate go run -tags gen-batch gen/main.go
 pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated.go -source=cert.go . Cert
-pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . cluster
-pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangecache -destination=mocks_generated.go . RangeDescriptorDB
-pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go .
-pkg/kv/kvclient/kvcoord/transport.go://go:generate mockgen -package=kvcoord -destination=mocks_generated.go . Transport
-pkg/roachpb/api.go://go:generate mockgen -package=roachpb -destination=mocks_generated.go . InternalClient,Internal_RangeFeedClient
-pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -source drt.go -package tests -destination drt_generated.go
 pkg/security/securitytest/securitytest.go://go:generate go-bindata -mode 0600 -modtime 1400000000 -pkg securitytest -o embedded.go -ignore README.md -ignore regenerate.sh test_certs
 pkg/security/securitytest/securitytest.go://go:generate gofmt -s -w embedded.go
 pkg/security/securitytest/securitytest.go://go:generate goimports -w embedded.go
@@ -75,7 +71,7 @@ git grep '//go:generate' -- './*.go' | grep -v stringer | grep -v 'add-leaktest\
     echo "$LINE"
     echo 'Please ensure that the equivalent logic to generate these files is'
     echo 'present in the Bazel build as well, then add the line to the'
-    echo 'EXISTING_GO_GENERATE_COMMENTS in build/bazelutil/check-genfiles.sh.'
+    echo 'EXISTING_GO_GENERATE_COMMENTS in build/bazelutil/check.sh.'
     echo 'Also see https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/1380090083/How+to+ensure+your+code+builds+with+Bazel'
     exit 1
 done

build/teamcity-check-genfiles.sh

@@ -22,7 +22,7 @@ tc_prepare
 
 tc_start_block "Ensure generated code is up-to-date"
 # Buffer noisy output and only print it on failure.
-if ! run run_bazel build/bazelutil/check.sh &> artifacts/buildshort.log || (cat artifacts/buildshort.log && false); then
+if ! (run run_bazel build/bazelutil/check.sh &> artifacts/buildshort.log || (cat artifacts/buildshort.log && false)); then
     # The command will output instructions on how to fix the error.
     exit 1
 fi

pkg/cmd/roachtest/BUILD.bazel

@@ -23,7 +23,7 @@ go_library(
         "//pkg/cmd/roachtest/registry",
         "//pkg/cmd/roachtest/spec",
         "//pkg/cmd/roachtest/test",
-        "//pkg/cmd/roachtest/tests",
+        "//pkg/cmd/roachtest/tests:with-mocks",
         "//pkg/internal/team",
         "//pkg/testutils/skip",
         "//pkg/util/contextutil",

pkg/cmd/roachtest/prometheus/BUILD.bazel

@@ -1,28 +1,44 @@
+load("@bazel_gomock//:gomock.bzl", "gomock")
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
-    name = "prometheus",
-    srcs = [
-        "mock_generated.go",
-        "prometheus.go",
-    ],
+    name = "with-mocks",
+    srcs = [":prometheus_mocks"],
+    embed = [":prometheus"],
     importpath = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/prometheus",
     visibility = ["//visibility:public"],
+    deps = [
+        "@com_github_golang_mock//gomock",
+    ],
+)
+
+go_library(
+    name = "prometheus",
+    srcs = ["prometheus.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/prometheus",
+    visibility = ["//visibility:private"],
     deps = [
         "//pkg/cmd/roachtest/logger",
         "//pkg/cmd/roachtest/option",
-        "@com_github_golang_mock//gomock",
         "@in_gopkg_yaml_v2//:yaml_v2",
     ],
 )
 
 go_test(
     name = "prometheus_test",
     srcs = ["prometheus_test.go"],
-    embed = [":prometheus"],
+    embed = [":with-mocks"],  # keep
     deps = [
         "//pkg/cmd/roachtest/option",
         "@com_github_golang_mock//gomock",
         "@com_github_stretchr_testify//require",
     ],
 )
+
+gomock(
+    name = "prometheus_mocks",
+    out = "mock_generated.go",
+    interfaces = ["Cluster"],
+    library = ":prometheus",
+    package = "prometheus",
+)

pkg/cmd/roachtest/prometheus/prometheus.go

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-//go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . cluster
+//go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster
 
 package prometheus
 
@@ -42,10 +42,10 @@ type Config struct {
 	ScrapeConfigs  []ScrapeConfig
 }
 
-// cluster is a subset of roachtest.Cluster.
+// Cluster is a subset of roachtest.Cluster.
 // It is abstracted to prevent a circular dependency on roachtest, as Cluster
 // requires the test interface.
-type cluster interface {
+type Cluster interface {
 	ExternalIP(context.Context, option.NodeListOption) ([]string, error)
 	Get(ctx context.Context, l *logger.Logger, src, dest string, opts ...option.Option) error
 	RunE(ctx context.Context, node option.NodeListOption, args ...string) error
@@ -63,7 +63,7 @@ type Prometheus struct {
 func Init(
 	ctx context.Context,
 	cfg Config,
-	c cluster,
+	c Cluster,
 	repeatFunc func(context.Context, option.NodeListOption, string, ...string) error,
 ) (*Prometheus, error) {
 	if err := c.RunE(
@@ -118,7 +118,7 @@ sudo systemd-run --unit prometheus --same-dir \
 
 // Snapshot takes a snapshot of prometheus and stores the snapshot in the given localPath
 func (pm *Prometheus) Snapshot(
-	ctx context.Context, c cluster, l *logger.Logger, localPath string,
+	ctx context.Context, c Cluster, l *logger.Logger, localPath string,
 ) error {
 	if err := c.RunE(
 		ctx,
@@ -148,7 +148,7 @@ const (
 )
 
 // makeYAMLConfig creates a prometheus YAML config for the server to use.
-func makeYAMLConfig(ctx context.Context, c cluster, scrapeConfigs []ScrapeConfig) (string, error) {
+func makeYAMLConfig(ctx context.Context, c Cluster, scrapeConfigs []ScrapeConfig) (string, error) {
 	type yamlStaticConfig struct {
 		Targets []string
 	}

pkg/cmd/roachtest/prometheus/prometheus_test.go

@@ -24,15 +24,15 @@ func TestMakeYAMLConfig(t *testing.T) {
 	testCases := []struct {
 		desc string
 
-		mockCluster   func(ctrl *gomock.Controller) cluster
+		mockCluster   func(ctrl *gomock.Controller) Cluster
 		scrapeConfigs []ScrapeConfig
 
 		expected string
 	}{
 		{
 			desc: "multiple scrape nodes",
-			mockCluster: func(ctrl *gomock.Controller) cluster {
-				c := NewMockcluster(ctrl)
+			mockCluster: func(ctrl *gomock.Controller) Cluster {
+				c := NewMockCluster(ctrl)
 				c.EXPECT().
 					ExternalIP(ctx, []int{1}).
 					Return([]string{"127.0.0.1"}, nil)
@@ -91,8 +91,8 @@ scrape_configs:
 		},
 		{
 			desc: "using make commands",
-			mockCluster: func(ctrl *gomock.Controller) cluster {
-				c := NewMockcluster(ctrl)
+			mockCluster: func(ctrl *gomock.Controller) Cluster {
+				c := NewMockCluster(ctrl)
 				c.EXPECT().
 					ExternalIP(ctx, []int{3, 4, 5}).
 					Return([]string{"127.0.0.3", "127.0.0.4", "127.0.0.5"}, nil)