Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(modules.clickhouse): Add zookeeper for clickhouse clusterization #1995

Merged
merged 11 commits into from
Feb 2, 2024

Conversation

laskoviymishka
Copy link
Contributor

@laskoviymishka laskoviymishka commented Dec 15, 2023

If you would like to perform queries against clusterized ClickHouse you need to start zookeeper near your cluster and pass link to it via config.xml

API example:

	zkhost, zkport, err := RunZookeeper(ctx)
	if err != nil {
		t.Fatal(err)
	}

	container, err := RunContainer(ctx,
		WithUsername(user),
		WithPassword(password),
		WithDatabase(dbname),
		WithZookeeper(zkhost, zkport),
	)
	if err != nil {
		t.Fatal(err)
	}

What does this PR do?

Add option: WithZookeeper to clickhouse module.

Why is it important?

ClickHouse queries (especially DDLs) vary a lot between single node and cluster. Having it a clusterized allows you to apply exactly same code to real prod clusters and in your tests.

Related issues

@laskoviymishka laskoviymishka requested a review from a team as a code owner December 15, 2023 17:03
Copy link

netlify bot commented Dec 15, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit e1e52d6
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/65b7a4b3caa0e300085eab73
😎 Deploy Preview https://deploy-preview-1995--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. I've left some comments but in general I would rather prefer to add an example about how to do this instead of hardcoding the configuration file. Also, wonder if it is possible to configure the zookeeper section to do not assume other config values. If so, then using templates would be great, redpanda module is doing so here.

With Testcontainers, containers can be composed programmatically and with the upcoming network support in #1993 modules would improve this experience. That's why I don't see having zookeeper as part of clickhouse.

modules/clickhouse/zookeeper.go Outdated Show resolved Hide resolved
modules/clickhouse/zookeeper.go Outdated Show resolved Hide resolved
modules/clickhouse/mounts/zk_config.xml.tpl Show resolved Hide resolved
modules/clickhouse/clickhouse.go Outdated Show resolved Hide resolved
modules/clickhouse/clickhouse.go Outdated Show resolved Hide resolved
docs/modules/clickhouse.md Outdated Show resolved Hide resolved
If you would like to perform queries against clusterized ClickHouse you need to start zookeeper near your cluster and pass link to it via `config.xml`

API example:

```
	zk, err := RunZookeeper(ctx)
	if err != nil {
		t.Fatal(err)
	}

	container, err := RunContainer(ctx,
		WithUsername(user),
		WithPassword(password),
		WithDatabase(dbname),
		WithZookeeper(zk),
	)
	if err != nil {
		t.Fatal(err)
	}
```
Pass clickhouse config as separate config file
Move xml config into go-tmpl
@laskoviymishka
Copy link
Contributor Author

@eddumelendez gentle ping on this one <3

eddumelendez
eddumelendez previously approved these changes Jan 22, 2024
Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will approve it once the comments for the docs are addressed.

Thank you so much for your work here! And @eddumelendez, thanks for the review too!

modules/clickhouse/clickhouse.go Show resolved Hide resolved
modules/clickhouse/clickhouse.go Show resolved Hide resolved
* main:
  Adding inbucket module (testcontainers#2142)
  testifylint: enable compares rule (testcontainers#2143)
  Bump containerd version to v1.7.12 (testcontainers#2137)
  feat: Add Minio module (testcontainers#2132)
  Adding LogConsumers start as part of the ContainerRequest (testcontainers#2073)
  chore: bring back assertion for network aliases for bridge in rootless mode (testcontainers#2141)
  chore(deps): bump github.com/docker/compose/v2 from 2.23.3 to 2.24.0 in /modules/compose (testcontainers#2096)
@mdelapenya mdelapenya changed the title feat(modules.clickhouse): Add zookeeper for clikchouse clusterization feat(modules.clickhouse): Add zookeeper for clickhouse clusterization Jan 29, 2024
@mdelapenya mdelapenya self-assigned this Jan 29, 2024
@mdelapenya mdelapenya added the feature New functionality or new behaviors on the existing one label Jan 29, 2024
@mdelapenya mdelapenya merged commit bc67dc3 into testcontainers:main Feb 2, 2024
80 checks passed
@mdelapenya
Copy link
Member

Thanks @laskoviymishka for submitting this new feature! Much appreciated 🙇

mdelapenya added a commit to gene-redpanda/testcontainers-go that referenced this pull request Feb 2, 2024
* main:
  feat(modules.clickhouse): Add zookeeper for clickhouse clusterization (testcontainers#1995)
  redpanda: allow using SASL and TLS together (testcontainers#2140)
  chore: do not panic in testable examples (testcontainers#2193)
  fix: all mounts should contain the testcontainers labels (testcontainers#2191)
  [redpanda] sasl test for wrong mechanism (testcontainers#2048)
  fix: deprecate BindMounts correctly (testcontainers#2190)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Feb 14, 2024
* main: (33 commits)
  feat (postgres): support for creating and restoring Snapshots (testcontainers#2199)
  fix: apply volume options only to volumes (testcontainers#2201)
  redpanda/test: add admin client call (testcontainers#2200)
  chore(deps): bump cloud.google.com/go/spanner from 1.55.0 to 1.56.0 in /modules/gcloud, cloud.google.com/go/pubsub from 1.35.0 to 1.36.1 in /modules/gcloud, cloud.google.com/go/bigquery from 1.57.1 to 1.58.0 in /modules/gcloud (testcontainers#2197)
  chore(deps): bump github.com/docker/docker from 25.0.1+incompatible to 25.0.2+incompatible (testcontainers#2196)
  fix: go doc reference broken image (testcontainers#2195)
  Add Support for WASM Transforms to Redpanda Module (testcontainers#2170)
  feat(modules.clickhouse): Add zookeeper for clickhouse clusterization (testcontainers#1995)
  redpanda: allow using SASL and TLS together (testcontainers#2140)
  chore: do not panic in testable examples (testcontainers#2193)
  fix: all mounts should contain the testcontainers labels (testcontainers#2191)
  [redpanda] sasl test for wrong mechanism (testcontainers#2048)
  fix: deprecate BindMounts correctly (testcontainers#2190)
  chore(docker_mounts): stop doing misleading logging (testcontainers#2178)
  fix: Add HTTPStrategy WithForcedIPv4LocalHost To Fix Docker Port Map (testcontainers#1775)
  chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162)
  feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131)
  chore(deps): bump golang.org/x/crypto in /modules/minio (testcontainers#2161)
  chore(deps): bump golang.org/x/crypto in /modules/openldap (testcontainers#2165)
  chore(deps): bump github.com/google/uuid from 1.5.0 to 1.6.0 (testcontainers#2169)
  ...
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Feb 14, 2024
* main: (33 commits)
  feat (postgres): support for creating and restoring Snapshots (testcontainers#2199)
  fix: apply volume options only to volumes (testcontainers#2201)
  redpanda/test: add admin client call (testcontainers#2200)
  chore(deps): bump cloud.google.com/go/spanner from 1.55.0 to 1.56.0 in /modules/gcloud, cloud.google.com/go/pubsub from 1.35.0 to 1.36.1 in /modules/gcloud, cloud.google.com/go/bigquery from 1.57.1 to 1.58.0 in /modules/gcloud (testcontainers#2197)
  chore(deps): bump github.com/docker/docker from 25.0.1+incompatible to 25.0.2+incompatible (testcontainers#2196)
  fix: go doc reference broken image (testcontainers#2195)
  Add Support for WASM Transforms to Redpanda Module (testcontainers#2170)
  feat(modules.clickhouse): Add zookeeper for clickhouse clusterization (testcontainers#1995)
  redpanda: allow using SASL and TLS together (testcontainers#2140)
  chore: do not panic in testable examples (testcontainers#2193)
  fix: all mounts should contain the testcontainers labels (testcontainers#2191)
  [redpanda] sasl test for wrong mechanism (testcontainers#2048)
  fix: deprecate BindMounts correctly (testcontainers#2190)
  chore(docker_mounts): stop doing misleading logging (testcontainers#2178)
  fix: Add HTTPStrategy WithForcedIPv4LocalHost To Fix Docker Port Map (testcontainers#1775)
  chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162)
  feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131)
  chore(deps): bump golang.org/x/crypto in /modules/minio (testcontainers#2161)
  chore(deps): bump golang.org/x/crypto in /modules/openldap (testcontainers#2165)
  chore(deps): bump github.com/google/uuid from 1.5.0 to 1.6.0 (testcontainers#2169)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or new behaviors on the existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants