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

Replace deprecated method #1746

Closed
wants to merge 12 commits into from

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented Sep 27, 2024

No description provided.

@wind57 wind57 changed the base branch from main to 3.1.x September 27, 2024 06:00
@@ -12,15 +12,29 @@
<artifactId>spring-cloud-kubernetes-integration-tests</artifactId>
<packaging>pom</packaging>

<!-- we need this fix : https://github.com/testcontainers/testcontainers-java/issues/4203,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will create an issue for this one, so that we do not forget about it

@@ -51,8 +50,6 @@ private KubernetesClientCatalogWatchNamespacesDelegate() {

private static final String NAMESPACE_B = "namespaceb";

private static final K3sContainer K3S = Commons.container();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nothing changes in this test, just use the container that was created previously.


private static final K3sContainer CONTAINER = new FixedPortsK3sContainer(DockerImageName.parse(Commons.RANCHER))
.configureFixedPorts(EXPOSED_PORTS)
.withFileSystemBind(TEMP_FOLDER, TEMP_FOLDER)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the core of this change.

withFileSystemBind is deprecated and we are supposed to replace it with withCopyFileToContainer.

When I tried to replace it verbatim, it meant that we need to copy to the k3s container everything that is under two directories:

  • /tmp/docker/image, these are images like wiremock, busybox, etc
  • TEMP_FOLDER (whatever that path is), and there we keep all of our docker images for integration tests.

When I tried to do that, the process failed with an OOM. The issue is that those directories have lots of images, too much for the JVM to cope with. There is a partial fix for this from testcontainers (testcontainers/testcontainers-java#4203), but it did not help in our case.

As such, I decided to do something different. In each integration test, instead of copying the whole directory, copy only tar files (images) that are needed for that particular integration test.

So the the method that we used to get a hold of the running k3s container (Commons::container) was changed to :

public static K3sContainer container(List<String> ourImages, List<String> images) { ... }

i.e.: pass the images that are strictly needed for a test. In this manner, we do not load the entire directory, but just a few images (most often just one). This in turn means that all callers have to change: all integration tests now need to pass this information further. Not a big deal, of course, but triggers lots of changes

@wind57 wind57 closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants