From c42273f29d9c0d8606796e96063f4ee9e6233d7e Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Tue, 29 Oct 2024 23:25:27 +0000 Subject: [PATCH 01/10] feat: use recommended cluster settings for cockroachdb --- modules/cockroachdb/cockroachdb.go | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index 4d412f04d3..706a3ea976 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "database/sql" "encoding/pem" "fmt" "net" @@ -90,6 +91,11 @@ func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustom return addTLS(ctx, container, o) }, }, + PostReadies: []testcontainers.ContainerHook{ + func(ctx context.Context, container testcontainers.Container) error { + return setRecommendedSettings(ctx, container, o) + }, + }, }, }, }, @@ -233,6 +239,46 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option return nil } +// setRecommendedSettings applies the cluster settings recommended by cockroachlabs for testing clusters. +// See https://www.cockroachlabs.com/docs/stable/local-testing for more information. +func setRecommendedSettings(ctx context.Context, container testcontainers.Container, opts options) error { + port, err := container.MappedPort(ctx, defaultSQLPort) + if err != nil { + return err + } + + host, err := container.Host(ctx) + if err != nil { + return err + } + + db, err := sql.Open("pgx/v5", connString(opts, host, port)) + if err != nil { + return err + } + defer db.Close() + + stmts := []string{ + "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'", + "SET CLUSTER SETTING jobs.registry.interval.gc = '30s'", + "SET CLUSTER SETTING jobs.registry.interval.cancel = '180s'", + "SET CLUSTER SETTING jobs.retention_time = '15s'", + "SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false", + "SET CLUSTER SETTING kv.range_split.by_load_merge_delay = '5s'", + `ALTER RANGE default CONFIGURE ZONE USING "gc.ttlseconds" = 600`, + `ALTER DATABASE system CONFIGURE ZONE USING "gc.ttlseconds" = 600`, + } + + for _, stmt := range stmts { + _, err = db.Exec(stmt) + if err != nil { + return err + } + } + + return nil +} + func connString(opts options, host string, port nat.Port) string { user := url.User(opts.User) if opts.Password != "" { From 1fe0c72961b00f4cf6e001841b7dd7a7b7ff349d Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 12:15:25 +0000 Subject: [PATCH 02/10] fix: wrap errors --- modules/cockroachdb/cockroachdb.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index 706a3ea976..d9cfb9cb36 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -244,17 +244,17 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option func setRecommendedSettings(ctx context.Context, container testcontainers.Container, opts options) error { port, err := container.MappedPort(ctx, defaultSQLPort) if err != nil { - return err + return fmt.Errorf("mapped port: %w", err) } host, err := container.Host(ctx) if err != nil { - return err + return fmt.Errorf("host: %w", err) } db, err := sql.Open("pgx/v5", connString(opts, host, port)) if err != nil { - return err + return fmt.Errorf("sql.Open: %w", err) } defer db.Close() @@ -272,7 +272,7 @@ func setRecommendedSettings(ctx context.Context, container testcontainers.Contai for _, stmt := range stmts { _, err = db.Exec(stmt) if err != nil { - return err + return fmt.Errorf("db.Exec: %w", err) } } From fe3d3ee33f41a21979ecdcf1ee7be3a1263a77be Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 13:25:39 +0000 Subject: [PATCH 03/10] feat: add WithStatements option and expose recommended settings as a list of statements --- modules/cockroachdb/cockroachdb.go | 24 ++++++++--------------- modules/cockroachdb/options.go | 31 +++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index d9cfb9cb36..38f0eae839 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -93,7 +93,7 @@ func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustom }, PostReadies: []testcontainers.ContainerHook{ func(ctx context.Context, container testcontainers.Container) error { - return setRecommendedSettings(ctx, container, o) + return runStatements(ctx, container, o) }, }, }, @@ -239,9 +239,12 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option return nil } -// setRecommendedSettings applies the cluster settings recommended by cockroachlabs for testing clusters. -// See https://www.cockroachlabs.com/docs/stable/local-testing for more information. -func setRecommendedSettings(ctx context.Context, container testcontainers.Container, opts options) error { +// runStatements runs the configured statements against the CockroachDB container. +func runStatements(ctx context.Context, container testcontainers.Container, opts options) error { + if len(opts.Statements) == 0 { + return nil + } + port, err := container.MappedPort(ctx, defaultSQLPort) if err != nil { return fmt.Errorf("mapped port: %w", err) @@ -258,18 +261,7 @@ func setRecommendedSettings(ctx context.Context, container testcontainers.Contai } defer db.Close() - stmts := []string{ - "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'", - "SET CLUSTER SETTING jobs.registry.interval.gc = '30s'", - "SET CLUSTER SETTING jobs.registry.interval.cancel = '180s'", - "SET CLUSTER SETTING jobs.retention_time = '15s'", - "SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false", - "SET CLUSTER SETTING kv.range_split.by_load_merge_delay = '5s'", - `ALTER RANGE default CONFIGURE ZONE USING "gc.ttlseconds" = 600`, - `ALTER DATABASE system CONFIGURE ZONE USING "gc.ttlseconds" = 600`, - } - - for _, stmt := range stmts { + for _, stmt := range opts.Statements { _, err = db.Exec(stmt) if err != nil { return fmt.Errorf("db.Exec: %w", err) diff --git a/modules/cockroachdb/options.go b/modules/cockroachdb/options.go index a2211d77e7..2acde60ee6 100644 --- a/modules/cockroachdb/options.go +++ b/modules/cockroachdb/options.go @@ -3,11 +3,12 @@ package cockroachdb import "github.com/testcontainers/testcontainers-go" type options struct { - Database string - User string - Password string - StoreSize string - TLS *TLSConfig + Database string + User string + Password string + StoreSize string + TLS *TLSConfig + Statements []string } func defaultOptions() options { @@ -67,3 +68,23 @@ func WithTLS(cfg *TLSConfig) Option { o.TLS = cfg } } + +// ClusterDefaults are the settings recommended by Cockroach Labs for testing clusters. +// See https://www.cockroachlabs.com/docs/stable/local-testing for more information. +var ClusterDefaults []string = []string{ + "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'", + "SET CLUSTER SETTING jobs.registry.interval.gc = '30s'", + "SET CLUSTER SETTING jobs.registry.interval.cancel = '180s'", + "SET CLUSTER SETTING jobs.retention_time = '15s'", + "SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false", + "SET CLUSTER SETTING kv.range_split.by_load_merge_delay = '5s'", + `ALTER RANGE default CONFIGURE ZONE USING "gc.ttlseconds" = 600`, + `ALTER DATABASE system CONFIGURE ZONE USING "gc.ttlseconds" = 600`, +} + +// WithStatements sets the statements to run on the CockroachDB cluster once the container is ready. +func WithStatements(statements ...string) Option { + return func(o *options) { + o.Statements = statements + } +} From fb1139ac3fe1fffa356b33ec6ce60f1d16aa4ed1 Mon Sep 17 00:00:00 2001 From: martskins Date: Wed, 30 Oct 2024 14:16:52 +0000 Subject: [PATCH 04/10] Update modules/cockroachdb/options.go Co-authored-by: Steven Hartland --- modules/cockroachdb/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cockroachdb/options.go b/modules/cockroachdb/options.go index 2acde60ee6..fee22f3cc2 100644 --- a/modules/cockroachdb/options.go +++ b/modules/cockroachdb/options.go @@ -71,7 +71,7 @@ func WithTLS(cfg *TLSConfig) Option { // ClusterDefaults are the settings recommended by Cockroach Labs for testing clusters. // See https://www.cockroachlabs.com/docs/stable/local-testing for more information. -var ClusterDefaults []string = []string{ +var ClusterDefaults = []string{ "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'", "SET CLUSTER SETTING jobs.registry.interval.gc = '30s'", "SET CLUSTER SETTING jobs.registry.interval.cancel = '180s'", From dc03f27e48213073760468a54c67d08262e94a77 Mon Sep 17 00:00:00 2001 From: martskins Date: Wed, 30 Oct 2024 14:16:57 +0000 Subject: [PATCH 05/10] Update modules/cockroachdb/cockroachdb.go Co-authored-by: Steven Hartland --- modules/cockroachdb/cockroachdb.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index 38f0eae839..03bc5a6cfe 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -262,8 +262,7 @@ func runStatements(ctx context.Context, container testcontainers.Container, opts defer db.Close() for _, stmt := range opts.Statements { - _, err = db.Exec(stmt) - if err != nil { + if _, err = db.Exec(stmt); err != nil { return fmt.Errorf("db.Exec: %w", err) } } From fee481ad98571409ded55d7a4c30793a67c66220 Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 14:29:51 +0000 Subject: [PATCH 06/10] fix: return close error on defer --- modules/cockroachdb/cockroachdb.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index 03bc5a6cfe..a3b4952548 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -240,7 +240,7 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option } // runStatements runs the configured statements against the CockroachDB container. -func runStatements(ctx context.Context, container testcontainers.Container, opts options) error { +func runStatements(ctx context.Context, container testcontainers.Container, opts options) (err error) { if len(opts.Statements) == 0 { return nil } @@ -259,7 +259,12 @@ func runStatements(ctx context.Context, container testcontainers.Container, opts if err != nil { return fmt.Errorf("sql.Open: %w", err) } - defer db.Close() + defer func() { + cerr := db.Close() + if err == nil { + err = cerr + } + }() for _, stmt := range opts.Statements { if _, err = db.Exec(stmt); err != nil { From 2553165d884a92a0d4afc7283a68331b70cbe040 Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 15:29:02 +0000 Subject: [PATCH 07/10] chore: improvement documentation and set default value for statements in defaultOptions --- modules/cockroachdb/examples_test.go | 59 ++++++++++++++++++++++++++++ modules/cockroachdb/options.go | 11 ++++-- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/modules/cockroachdb/examples_test.go b/modules/cockroachdb/examples_test.go index c06c97596b..2f6c5ea1a5 100644 --- a/modules/cockroachdb/examples_test.go +++ b/modules/cockroachdb/examples_test.go @@ -2,6 +2,7 @@ package cockroachdb_test import ( "context" + "database/sql" "fmt" "log" "net/url" @@ -50,3 +51,61 @@ func ExampleRun() { // true // postgres://root@localhost:xxx/defaultdb?sslmode=disable } + +func ExampleRun_withRecommendedSettings() { + ctx := context.Background() + + cockroachdbContainer, err := cockroachdb.Run(ctx, "cockroachdb/cockroach:latest-v23.1", cockroachdb.WithStatements(cockroachdb.ClusterDefaults...)) + defer func() { + if err := testcontainers.TerminateContainer(cockroachdbContainer); err != nil { + log.Printf("failed to terminate container: %s", err) + } + }() + if err != nil { + log.Printf("failed to start container: %s", err) + return + } + + state, err := cockroachdbContainer.State(ctx) + if err != nil { + log.Printf("failed to get container state: %s", err) + return + } + fmt.Println(state.Running) + + addr, err := cockroachdbContainer.ConnectionString(ctx) + if err != nil { + log.Printf("failed to get connection string: %s", err) + return + } + + db, err := sql.Open("pgx/v5", addr) + if err != nil { + log.Printf("failed to open connection: %s", err) + return + } + defer func() { + if err := db.Close(); err != nil { + log.Printf("failed to close connection: %s", err) + } + }() + + var queueInterval string + if err := db.QueryRow("SHOW CLUSTER SETTING kv.range_merge.queue_interval").Scan(&queueInterval); err != nil { + log.Printf("failed to scan row: %s", err) + return + } + fmt.Println(queueInterval) + + var statsCollectionEnabled bool + if err := db.QueryRow("SHOW CLUSTER SETTING sql.stats.automatic_collection.enabled").Scan(&statsCollectionEnabled); err != nil { + log.Printf("failed to scan row: %s", err) + return + } + fmt.Println(statsCollectionEnabled) + + // Output: + // true + // 00:00:00.05 + // false +} diff --git a/modules/cockroachdb/options.go b/modules/cockroachdb/options.go index fee22f3cc2..192693825e 100644 --- a/modules/cockroachdb/options.go +++ b/modules/cockroachdb/options.go @@ -13,10 +13,11 @@ type options struct { func defaultOptions() options { return options{ - User: defaultUser, - Password: defaultPassword, - Database: defaultDatabase, - StoreSize: defaultStoreSize, + User: defaultUser, + Password: defaultPassword, + Database: defaultDatabase, + StoreSize: defaultStoreSize, + Statements: ClusterDefaults, } } @@ -83,6 +84,8 @@ var ClusterDefaults = []string{ } // WithStatements sets the statements to run on the CockroachDB cluster once the container is ready. +// This, in combination with ClusterDefaults, can be used to configure the cluster with the settings +// recommended by Cockroach Labs. func WithStatements(statements ...string) Option { return func(o *options) { o.Statements = statements From 096672b369eb28592bfa6e4ebce151f565f30a7a Mon Sep 17 00:00:00 2001 From: martskins Date: Wed, 30 Oct 2024 15:31:57 +0000 Subject: [PATCH 08/10] Update modules/cockroachdb/cockroachdb.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- modules/cockroachdb/cockroachdb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index a3b4952548..a33ac9fee0 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -240,7 +240,7 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option } // runStatements runs the configured statements against the CockroachDB container. -func runStatements(ctx context.Context, container testcontainers.Container, opts options) (err error) { +func runStatements(ctx context.Context, container testcontainers.Container, opts options) (err error) { //nolint:nonamedreturns // Needed for error checking. if len(opts.Statements) == 0 { return nil } From f7dc17fbd3224114036bb8560f235f5080874751 Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 15:59:27 +0000 Subject: [PATCH 09/10] chore: remove unnecessary nolint directive and rename ClusterDefaults to DefaultStatements --- modules/cockroachdb/cockroachdb.go | 2 +- modules/cockroachdb/examples_test.go | 2 +- modules/cockroachdb/options.go | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/cockroachdb/cockroachdb.go b/modules/cockroachdb/cockroachdb.go index a33ac9fee0..a3b4952548 100644 --- a/modules/cockroachdb/cockroachdb.go +++ b/modules/cockroachdb/cockroachdb.go @@ -240,7 +240,7 @@ func addTLS(ctx context.Context, container testcontainers.Container, opts option } // runStatements runs the configured statements against the CockroachDB container. -func runStatements(ctx context.Context, container testcontainers.Container, opts options) (err error) { //nolint:nonamedreturns // Needed for error checking. +func runStatements(ctx context.Context, container testcontainers.Container, opts options) (err error) { if len(opts.Statements) == 0 { return nil } diff --git a/modules/cockroachdb/examples_test.go b/modules/cockroachdb/examples_test.go index 2f6c5ea1a5..9a8fb12881 100644 --- a/modules/cockroachdb/examples_test.go +++ b/modules/cockroachdb/examples_test.go @@ -55,7 +55,7 @@ func ExampleRun() { func ExampleRun_withRecommendedSettings() { ctx := context.Background() - cockroachdbContainer, err := cockroachdb.Run(ctx, "cockroachdb/cockroach:latest-v23.1", cockroachdb.WithStatements(cockroachdb.ClusterDefaults...)) + cockroachdbContainer, err := cockroachdb.Run(ctx, "cockroachdb/cockroach:latest-v23.1", cockroachdb.WithStatements(cockroachdb.DefaultStatements...)) defer func() { if err := testcontainers.TerminateContainer(cockroachdbContainer); err != nil { log.Printf("failed to terminate container: %s", err) diff --git a/modules/cockroachdb/options.go b/modules/cockroachdb/options.go index 192693825e..90e67a1c2d 100644 --- a/modules/cockroachdb/options.go +++ b/modules/cockroachdb/options.go @@ -17,7 +17,7 @@ func defaultOptions() options { Password: defaultPassword, Database: defaultDatabase, StoreSize: defaultStoreSize, - Statements: ClusterDefaults, + Statements: DefaultStatements, } } @@ -70,9 +70,9 @@ func WithTLS(cfg *TLSConfig) Option { } } -// ClusterDefaults are the settings recommended by Cockroach Labs for testing clusters. +// DefaultStatements are the settings recommended by Cockroach Labs for testing clusters. // See https://www.cockroachlabs.com/docs/stable/local-testing for more information. -var ClusterDefaults = []string{ +var DefaultStatements = []string{ "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'", "SET CLUSTER SETTING jobs.registry.interval.gc = '30s'", "SET CLUSTER SETTING jobs.registry.interval.cancel = '180s'", @@ -84,7 +84,7 @@ var ClusterDefaults = []string{ } // WithStatements sets the statements to run on the CockroachDB cluster once the container is ready. -// This, in combination with ClusterDefaults, can be used to configure the cluster with the settings +// This, in combination with DefaultStatements, can be used to configure the cluster with the settings // recommended by Cockroach Labs. func WithStatements(statements ...string) Option { return func(o *options) { From ff63c4c3670823b2a7ba22347b3de48f32fce662 Mon Sep 17 00:00:00 2001 From: Martin Asquino Date: Wed, 30 Oct 2024 22:06:44 +0000 Subject: [PATCH 10/10] fix: only use recommended settings in root user tests --- modules/cockroachdb/cockroachdb_test.go | 9 +++++++++ modules/cockroachdb/options.go | 1 + 2 files changed, 10 insertions(+) diff --git a/modules/cockroachdb/cockroachdb_test.go b/modules/cockroachdb/cockroachdb_test.go index cc355e9168..45df7909bb 100644 --- a/modules/cockroachdb/cockroachdb_test.go +++ b/modules/cockroachdb/cockroachdb_test.go @@ -28,6 +28,9 @@ func TestCockroach_NotRoot(t *testing.T) { url: "postgres://test@localhost:xxxxx/defaultdb?sslmode=disable", opts: []testcontainers.ContainerCustomizer{ cockroachdb.WithUser("test"), + // Do not run the default statements as the user used on this test is + // lacking the needed MODIFYCLUSTERSETTING privilege to run them. + cockroachdb.WithStatements(), }, }) } @@ -38,6 +41,9 @@ func TestCockroach_Password(t *testing.T) { opts: []testcontainers.ContainerCustomizer{ cockroachdb.WithUser("foo"), cockroachdb.WithPassword("bar"), + // Do not run the default statements as the user used on this test is + // lacking the needed MODIFYCLUSTERSETTING privilege to run them. + cockroachdb.WithStatements(), }, }) } @@ -50,6 +56,9 @@ func TestCockroach_TLS(t *testing.T) { url: "postgres://root@localhost:xxxxx/defaultdb?sslmode=verify-full", opts: []testcontainers.ContainerCustomizer{ cockroachdb.WithTLS(tlsCfg), + // Do not run the default statements as the user used on this test is + // lacking the needed MODIFYCLUSTERSETTING privilege to run them. + cockroachdb.WithStatements(), }, }) } diff --git a/modules/cockroachdb/options.go b/modules/cockroachdb/options.go index 90e67a1c2d..eba101834e 100644 --- a/modules/cockroachdb/options.go +++ b/modules/cockroachdb/options.go @@ -71,6 +71,7 @@ func WithTLS(cfg *TLSConfig) Option { } // DefaultStatements are the settings recommended by Cockroach Labs for testing clusters. +// Note that to use these defaults the user needs to have MODIFYCLUSTERSETTING privilege. // See https://www.cockroachlabs.com/docs/stable/local-testing for more information. var DefaultStatements = []string{ "SET CLUSTER SETTING kv.range_merge.queue_interval = '50ms'",