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

Bump testcontainers-java-tarantool to 1.2.0 #444

Merged
merged 2 commits into from
Dec 21, 2023
Merged

Bump testcontainers-java-tarantool to 1.2.0 #444

merged 2 commits into from
Dec 21, 2023

Conversation

nickkkccc
Copy link
Contributor

@nickkkccc nickkkccc commented Nov 16, 2023

Note the peculiarity of instance restarts in a cluster #106

I haven't forgotten about:

  • Tests
  • Changelog
  • Documentation
  • Commit messages comply with the guideline
  • Cleanup the code for review. See checklist

Related issues:
Closes #442

@nickkkccc nickkkccc linked an issue Nov 16, 2023 that may be closed by this pull request
@nickkkccc nickkkccc changed the title Bump testcontainers-java-tarantool to 1.0.2 Bump testcontainers-java-tarantool to 1.0.3 Dec 8, 2023
@nickkkccc nickkkccc changed the title Bump testcontainers-java-tarantool to 1.0.3 Bump testcontainers-java-tarantool to 1.1.0 Dec 12, 2023
pom.xml Outdated
@@ -264,7 +264,7 @@
<dependency>
<groupId>io.tarantool</groupId>
<artifactId>testcontainers-java-tarantool</artifactId>
<version>1.0.1</version>
<version>1.0.2</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<version>1.0.2</version>
<version>1.1.0</version>

@nickkkccc nickkkccc changed the title Bump testcontainers-java-tarantool to 1.1.0 Bump testcontainers-java-tarantool to 1.2.0 Dec 14, 2023
- Bump testcontainers-java-tarantool version to 1.2.0
- Bump org.testcontainers:junit-jupiter version to 1.19.3
- CHANGELOG.md

Closes #442
dkasimovskiy
dkasimovskiy previously approved these changes Dec 20, 2023
@@ -16,7 +16,7 @@ local ok, err = cartridge.cfg({
'app.roles.api_storage',
'app.roles.custom',
},
cluster_cookie = 'testapp-cluster-cookie',
cluster_cookie = 'secret-cluster-cookie',
Copy link
Contributor

Choose a reason for hiding this comment

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

it is default value and you can just remove this whole line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@@ -56,7 +56,6 @@ public static void setUp() throws URISyntaxException, SSLException {
.withUsername("test_user")
.withPassword("test_password")
.withMemtxMemory(256 * 1024 * 1024)
.withDirectoryBinding(RESOURCE_PATH + "ssl")
Copy link
Contributor

Choose a reason for hiding this comment

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

you won't get files that are placed in org/testcontainers/containers/enterprise/ssl that needed for testing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it for enterprise tests

Copy link
Contributor

@ArtDu ArtDu left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a question about container behavior

@@ -23,7 +23,6 @@ abstract class CartridgeMixedInstancesContainer {
"cartridge-java-test-mixed",
"cartridge/instances_mixed.yml",
"cartridge/topology_mixed.lua")
.withDirectoryBinding("cartridge")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get how this works. Please explain how does a container get needed files?

Copy link
Contributor Author

@nickkkccc nickkkccc Dec 21, 2023

Choose a reason for hiding this comment

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

Previous version of the test containers (e.g., 1.0.1):

  1. Dependencies are downloaded on the system inside the container (gcc and others).
  2. The cartridge folder is a mapping of the /app directory.
  3. The cartridge folder is copied into the image as the /app working directory. This step is where the image is built.
  4. Start the container and run the cartridge build command (dependency rocks are downloaded into the working directory at this point).
  5. Key point: the dependency rocks are downloaded into the /app directory related to the container. But docker looks at the cartridge folder that was mapped earlier in the build phase.

So binding works with 1.0.1

New version of testcontainers:

Copying the /cartridge directory and executing cartridge build happens during the image build phase. Key point: when using the .withDirectoryBinding method at the container startup stage, docker will look at the /cartridge directory that doesn't have the built rocks dependencies in it.

So binding doesn't work with new testcontainers.

@nickkkccc nickkkccc requested a review from ArtDu December 21, 2023 07:35
@nickkkccc nickkkccc added this pull request to the merge queue Dec 21, 2023
Merged via the queue into master with commit 9948753 Dec 21, 2023
3 checks passed
@nickkkccc nickkkccc deleted the gh-442 branch December 21, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump testcontainers-java to 1.2.0
4 participants