Add Consul module #4683

julb · 2021-11-17T14:07:23Z

No description provided.

kiview · 2021-11-17T15:28:17Z

Hi @julb, thanks a lot for contributing this module.

Looking at the logic around runInitCommands(), it seems that this module makes using Consul with Testcontainers considerably easier, so theoretically we can imagine this becoming an incubating module.

However, we are currently lacking the capacity to review and merge PRs for new modules, so I can't promise when we will be able to get to it. This does not mean, that we don't appreciate your contribution. We are currently in the process to rework our current approach around modules, to make it easier for the community to contribute them.

In case you are interested in getting it out to users as early as possible, you can always consider publishing it in your own repo (this is in no way an indication, that we don't want to add it to our repo).

eddumelendez

Hi @julb, sorry for the late reply and thanks for your contribution! The code looks good but I've added a few comments.

Are you willing to update the PR?

docs/modules/consul.md

modules/consul/src/test/java/org/testcontainers/consul/ConsulClientTest.java

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

julb · 2022-07-28T13:27:23Z

Hey @eddumelendez
It is updated.
Thanks for your feedback!

eddumelendez

just a couple of comments and LGTM. Once again, thanks!

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

eddumelendez

few more comments from @kiview and me :)

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

julb · 2022-07-29T07:23:20Z

@eddumelendez @kiview all remarks addressed! Thanks a lot.

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

eddumelendez

LGTM

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

kiview

Thanks a lot for the PR @julb, LGTM 👍

There is one open question regarding the package name, but this is something we as maintainer should align on first 🙂

kiview · 2022-08-04T14:34:03Z

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

@@ -0,0 +1,103 @@
+package org.testcontainers.consul;


We normally use the org.testcontainers.containers package for Testcontainers container implementation, but AFAIK this makes Testcontainers suffer from the Split-Package problem when using the modulepath (as described here: https://stackoverflow.com/questions/53245628/jdk9-automatic-modules-and-split-packages-dependencies).

We also have other modules, that follow a package convention of org.testcontainers.{module-name}, the same as here. So I am inclined to say, we should leave it as provided in the PR.

WDYT @bsideup?

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

Although we have some AUTHORS files in different modules, this is generally not what we do and we prefer using the Git history to identify authors.

…ontainer.java

eddumelendez · 2022-08-04T20:11:20Z

@julb can you please add consul to

.github/ISSUE_TEMPLATE/bug_report.yaml
.github/ISSUE_TEMPLATE/enhancement.yaml
.github/ISSUE_TEMPLATE/feature.yaml
.github/dependabot.yml
.github/labeler.yml

Index: .github/ISSUE_TEMPLATE/bug_report.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/bug_report.yaml b/.github/ISSUE_TEMPLATE/bug_report.yaml
--- a/.github/ISSUE_TEMPLATE/bug_report.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/bug_report.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/ISSUE_TEMPLATE/enhancement.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/enhancement.yaml b/.github/ISSUE_TEMPLATE/enhancement.yaml
--- a/.github/ISSUE_TEMPLATE/enhancement.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/enhancement.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/ISSUE_TEMPLATE/feature.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/feature.yaml b/.github/ISSUE_TEMPLATE/feature.yaml
--- a/.github/ISSUE_TEMPLATE/feature.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/feature.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/dependabot.yml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
--- a/.github/dependabot.yml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/dependabot.yml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -43,6 +43,11 @@
       interval: "monthly"
     open-pull-requests-limit: 10
   - package-ecosystem: "gradle"
+    directory: "/modules/consul"
+    schedule:
+      interval: "monthly"
+    open-pull-requests-limit: 10
+  - package-ecosystem: "gradle"
     directory: "/modules/couchbase"
     schedule:
       interval: "monthly"
Index: .github/labeler.yml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/labeler.yml b/.github/labeler.yml
--- a/.github/labeler.yml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/labeler.yml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -15,6 +15,8 @@
   - modules/clickhouse/*
 "modules/cockroachdb":
   - modules/cockroachdb/*
+"modules/consul":
+  - modules/consul/*
 "modules/couchbase":
   - modules/couchbase/*
 "modules/db2":

julb · 2022-08-05T07:19:16Z

@eddumelendez @kiview done!
Thanks for your reviews and your time.

kiview · 2022-08-05T08:17:23Z

Thanks @julb, we will merge once we internally decided on how to proceed with package names for container modules 🙂

eddumelendez

one last thing @julb ! We are not able to push to your branch so here we are again asking you for some changes 😬 about moving to assertj

Thank you so much!

eddumelendez · 2022-08-06T00:15:53Z

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

+        // Write operation
+        properties.forEach((key, value) -> {
+            Response<Boolean> writeResponse = consulClient.setKVValue(key, value);
+            Assertions.assertThat(writeResponse.getValue()).isTrue();


import static

eddumelendez · 2022-08-06T00:21:47Z

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

+    public void readFirstPropertyPathWithCli() throws IOException, InterruptedException {
+        GenericContainer.ExecResult result = consulContainer.execInContainer("consul", "kv", "get", "config/testing1");
+        final String output = result.getStdout().replaceAll("\\r?\\n", "");
+        MatcherAssert.assertThat(output, CoreMatchers.containsString("value123"));


move to assertThat from assertj, please

eddumelendez · 2022-08-06T00:25:03Z

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

+        // Read operation
+        properties.forEach((key, value) -> {
+            Response<GetValue> readResponse = consulClient.getKVValue(key);
+            Assertions.assertThat(readResponse.getValue().getDecodedValue()).isEqualTo(value);


import static

eddumelendez · 2022-08-06T00:26:05Z

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

+        RestAssured
+            .given()
+            .when()
+            .get("http://" + getHostAndPort() + "/v1/kv/config/testing1")
+            .then()
+            .assertThat()
+            .body(
+                "[0].Value",
+                CoreMatchers.equalTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)))
+            );


can we do something like

Suggested change

RestAssured

.given()

.when()

.get("http://" + getHostAndPort() + "/v1/kv/config/testing1")

.then()

.assertThat()

.body(

"[0].Value",

CoreMatchers.equalTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)))

);

io.restassured.response.Response response = RestAssured

.given()

.when()

.get("http://" + getHostAndPort() + "/v1/kv/config/testing1")

.andReturn();

assertThat(response.body().jsonPath().getString("[0].Value"))

.isEqualTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)));

eddumelendez · 2022-08-15T14:33:02Z

Thanks @julb ! PR has been merged and now we have a consul module!

julb · 2022-08-15T19:00:16Z

OMG!
Thanks a lot for your support, that's awesome !!
Looking forward for the official release!

julb requested review from bsideup, kiview and rnorth as code owners November 17, 2021 14:07

julb changed the title ~~Provide testcontainers consul module~~ New module: consul Nov 17, 2021

kiview added the type/new module label Nov 17, 2021

eddumelendez requested changes Jul 26, 2022

View reviewed changes

eddumelendez assigned kiview and eddumelendez Jul 26, 2022

julb requested a review from a team as a code owner July 28, 2022 13:01

github-actions bot added the type/docs label Jul 28, 2022

eddumelendez reviewed Jul 28, 2022

View reviewed changes

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated Show resolved Hide resolved

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated Show resolved Hide resolved

eddumelendez reviewed Jul 28, 2022

View reviewed changes

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated Show resolved Hide resolved

eddumelendez requested changes Jul 28, 2022

View reviewed changes

julb requested a review from eddumelendez July 29, 2022 07:24

eddumelendez reviewed Jul 29, 2022

View reviewed changes

modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java Show resolved Hide resolved

eddumelendez added this to the next milestone Jul 29, 2022

julb requested a review from eddumelendez August 3, 2022 09:37

eddumelendez approved these changes Aug 3, 2022

View reviewed changes

kiview reviewed Aug 4, 2022

View reviewed changes

modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated Show resolved Hide resolved

kiview approved these changes Aug 4, 2022

View reviewed changes

julb and others added 7 commits August 4, 2022 17:11

Provide testcontainers consul module

9e8b9d1

Take PR feedback into account for Consul module

bcc0ed5

consul-module: take additional remarks into account

aa24a61

consul-module: fix checkstyle issues

36536f1

Delete AUTHORS file

02fa7bf

Although we have some AUTHORS files in different modules, this is generally not what we do and we prefer using the Git history to identify authors.

Update modules/consul/src/main/java/org/testcontainers/consul/ConsulC…

951c270

…ontainer.java

consul: reference consul in github config files

8fe111f

eddumelendez reviewed Aug 6, 2022

View reviewed changes

eddumelendez changed the title ~~New module: consul~~ Add Consul module Aug 6, 2022

eddumelendez merged commit 625947f into testcontainers:master Aug 15, 2022

eddumelendez mentioned this pull request Aug 31, 2022

Add module for Hashicorp Consul #4860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Consul module #4683

Add Consul module #4683

julb commented Nov 17, 2021

kiview commented Nov 17, 2021

eddumelendez left a comment

julb commented Jul 28, 2022

eddumelendez left a comment

eddumelendez left a comment

julb commented Jul 29, 2022 •

edited

eddumelendez left a comment

kiview left a comment

kiview Aug 4, 2022

eddumelendez commented Aug 4, 2022 •

edited

julb commented Aug 5, 2022

kiview commented Aug 5, 2022

eddumelendez left a comment

eddumelendez Aug 6, 2022

eddumelendez Aug 6, 2022

eddumelendez Aug 6, 2022

eddumelendez Aug 6, 2022

eddumelendez commented Aug 15, 2022

julb commented Aug 15, 2022 •

edited

Add Consul module #4683

Add Consul module #4683

Conversation

julb commented Nov 17, 2021

kiview commented Nov 17, 2021

eddumelendez left a comment

Choose a reason for hiding this comment

julb commented Jul 28, 2022

eddumelendez left a comment

Choose a reason for hiding this comment

eddumelendez left a comment

Choose a reason for hiding this comment

julb commented Jul 29, 2022 • edited

eddumelendez left a comment

Choose a reason for hiding this comment

kiview left a comment

Choose a reason for hiding this comment

kiview Aug 4, 2022

Choose a reason for hiding this comment

eddumelendez commented Aug 4, 2022 • edited

julb commented Aug 5, 2022

kiview commented Aug 5, 2022

eddumelendez left a comment

Choose a reason for hiding this comment

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

eddumelendez commented Aug 15, 2022

julb commented Aug 15, 2022 • edited

julb commented Jul 29, 2022 •

edited

eddumelendez commented Aug 4, 2022 •

edited

julb commented Aug 15, 2022 •

edited