Don't call `Startable.start()` for already started containers

Add a new `TestcontainersStartup.start` static method and update the
existing start methods so that `Startable.start()` is only called when
the container is not already running.

Prior to this commit, we assumed that `Startable.start()` calls were
idempotent and could be safely made multiple times. Whilst this appears
to be true for stock `GenericContainer` based startables, users may have
their own `start()` method that does not expect to be called multiple
times.

The implemented detection logic will not be applied if a `Startable`
is not also a `Container`. In these cases, the implementation will need
to deal directly with multiple `start()` calls.

Fixed gh-43253
This commit is contained in:
Phillip Webb 2024-12-12 15:17:02 -08:00
parent 65a862c13c
commit ccc1b5da28
5 changed files with 253 additions and 7 deletions

View File

@ -0,0 +1,76 @@
/*
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.testcontainers.service.connection;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
import org.testcontainers.utility.DockerImageName;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration;
import org.springframework.boot.testsupport.container.TestImage;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Integration tests to ensure containers are started only once.
*
* @author Phillip Webb
*/
@SpringJUnitConfig
@Testcontainers(disabledWithoutDocker = true)
class ServiceConnectionStartsConnectionOnceIntegrationTest {
@Container
@ServiceConnection
static final StartCountingPostgreSQLContainer postgres = TestImage
.container(StartCountingPostgreSQLContainer.class);
@Test
void startedOnlyOnce() {
assertThat(postgres.startCount.get()).isOne();
}
@Configuration(proxyBeanMethods = false)
@ImportAutoConfiguration(DataSourceAutoConfiguration.class)
static class TestConfiguration {
}
static class StartCountingPostgreSQLContainer extends PostgreSQLContainer<StartCountingPostgreSQLContainer> {
final AtomicInteger startCount = new AtomicInteger();
StartCountingPostgreSQLContainer(DockerImageName dockerImageName) {
super(dockerImageName);
}
@Override
public void start() {
this.startCount.incrementAndGet();
super.start();
}
}
}

View File

@ -97,7 +97,7 @@ class TestcontainersLifecycleBeanPostProcessor
}
else if (this.startables.get() == Startables.STARTED) {
logger.trace(LogMessage.format("Starting container %s", beanName));
startableBean.start();
TestcontainersStartup.start(startableBean);
}
}
return bean;

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -17,7 +17,13 @@
package org.springframework.boot.testcontainers.lifecycle;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.testcontainers.containers.Container;
import org.testcontainers.lifecycle.Startable;
import org.testcontainers.lifecycle.Startables;
@ -40,7 +46,7 @@ public enum TestcontainersStartup {
@Override
void start(Collection<? extends Startable> startables) {
startables.forEach(Startable::start);
startables.forEach(TestcontainersStartup::start);
}
},
@ -52,7 +58,8 @@ public enum TestcontainersStartup {
@Override
void start(Collection<? extends Startable> startables) {
Startables.deepStart(startables).join();
SingleStartables singleStartables = new SingleStartables();
Startables.deepStart(startables.stream().map(singleStartables::getOrCreate)).join();
}
};
@ -91,4 +98,69 @@ public enum TestcontainersStartup {
return canonicalName.toString();
}
/**
* Start the given {@link Startable} unless is's detected as already running.
* @param startable the startable to start
* @since 3.4.1
*/
public static void start(Startable startable) {
if (!isRunning(startable)) {
startable.start();
}
}
private static boolean isRunning(Startable startable) {
try {
return (startable instanceof Container<?> container) && container.isRunning();
}
catch (Throwable ex) {
return false;
}
}
/**
* Tracks and adapts {@link Startable} instances to use
* {@link TestcontainersStartup#start(Startable)} so containers are only started once
* even when calling {@link Startables#deepStart(java.util.stream.Stream)}.
*/
private static final class SingleStartables {
private final Map<Startable, SingleStartable> adapters = new HashMap<>();
SingleStartable getOrCreate(Startable startable) {
return this.adapters.computeIfAbsent(startable, this::create);
}
private SingleStartable create(Startable startable) {
return new SingleStartable(this, startable);
}
record SingleStartable(SingleStartables singleStartables, Startable startable) implements Startable {
@Override
public Set<Startable> getDependencies() {
Set<Startable> dependencies = this.startable.getDependencies();
if (dependencies.isEmpty()) {
return dependencies;
}
return dependencies.stream()
.map(this.singleStartables::getOrCreate)
.collect(Collectors.toCollection(LinkedHashSet::new));
}
@Override
public void start() {
TestcontainersStartup.start(this.startable);
}
@Override
public void stop() {
this.startable.stop();
}
}
}
}

View File

@ -33,6 +33,7 @@ import org.springframework.boot.autoconfigure.service.connection.ConnectionDetai
import org.springframework.boot.autoconfigure.service.connection.ConnectionDetailsFactory;
import org.springframework.boot.origin.Origin;
import org.springframework.boot.origin.OriginProvider;
import org.springframework.boot.testcontainers.lifecycle.TestcontainersStartup;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.core.ResolvableType;
@ -188,7 +189,7 @@ public abstract class ContainerConnectionDetailsFactory<C extends Container<?>,
Assert.state(this.container != null,
"Container cannot be obtained before the connection details bean has been initialized");
if (this.container instanceof Startable startable) {
startable.start();
TestcontainersStartup.start(startable);
}
return this.container;
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -17,10 +17,13 @@
package org.springframework.boot.testcontainers.lifecycle;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.lifecycle.Startable;
import org.springframework.mock.env.MockEnvironment;
@ -39,6 +42,16 @@ class TestcontainersStartupTests {
private final AtomicInteger counter = new AtomicInteger();
@Test
void startSingleStartsOnlyOnce() {
TestStartable startable = new TestStartable();
assertThat(startable.startCount).isZero();
TestcontainersStartup.start(startable);
assertThat(startable.startCount).isOne();
TestcontainersStartup.start(startable);
assertThat(startable.startCount).isOne();
}
@Test
void startWhenSquentialStartsSequentially() {
List<TestStartable> startables = createTestStartables(100);
@ -49,6 +62,22 @@ class TestcontainersStartupTests {
}
}
@Test
void startWhenSquentialStartsOnlyOnce() {
List<TestStartable> startables = createTestStartables(10);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isZero();
}
TestcontainersStartup.SEQUENTIAL.start(startables);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isOne();
}
TestcontainersStartup.SEQUENTIAL.start(startables);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isOne();
}
}
@Test
void startWhenParallelStartsInParallel() {
List<TestStartable> startables = createTestStartables(100);
@ -56,6 +85,47 @@ class TestcontainersStartupTests {
assertThat(startables.stream().map(TestStartable::getThreadName)).hasSizeGreaterThan(1);
}
@Test
void startWhenParallelStartsOnlyOnce() {
List<TestStartable> startables = createTestStartables(10);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isZero();
}
TestcontainersStartup.PARALLEL.start(startables);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isOne();
}
TestcontainersStartup.PARALLEL.start(startables);
for (int i = 0; i < startables.size(); i++) {
assertThat(startables.get(i).getStartCount()).isOne();
}
}
@Test
void startWhenParallelStartsDependenciesOnlyOnce() {
List<TestStartable> dependencies = createTestStartables(10);
TestStartable first = new TestStartable(dependencies);
TestStartable second = new TestStartable(dependencies);
List<TestStartable> startables = List.of(first, second);
assertThat(first.getStartCount()).isZero();
assertThat(second.getStartCount()).isZero();
for (int i = 0; i < startables.size(); i++) {
assertThat(dependencies.get(i).getStartCount()).isZero();
}
TestcontainersStartup.PARALLEL.start(startables);
assertThat(first.getStartCount()).isOne();
assertThat(second.getStartCount()).isOne();
for (int i = 0; i < startables.size(); i++) {
assertThat(dependencies.get(i).getStartCount()).isOne();
}
TestcontainersStartup.PARALLEL.start(startables);
assertThat(first.getStartCount()).isOne();
assertThat(second.getStartCount()).isOne();
for (int i = 0; i < startables.size(); i++) {
assertThat(dependencies.get(i).getStartCount()).isOne();
}
}
@Test
void getWhenNoPropertyReturnsDefault() {
MockEnvironment environment = new MockEnvironment();
@ -93,20 +163,43 @@ class TestcontainersStartupTests {
return testStartables;
}
private final class TestStartable implements Startable {
private class TestStartable extends GenericContainer<TestStartable> {
private int startCount;
private int index;
private String threadName;
TestStartable() {
super("test");
}
TestStartable(Collection<TestStartable> startables) {
super("test");
this.dependencies.addAll(startables);
}
@Override
public Set<Startable> getDependencies() {
return this.dependencies;
}
@Override
public void start() {
this.startCount++;
this.index = TestcontainersStartupTests.this.counter.getAndIncrement();
this.threadName = Thread.currentThread().getName();
}
@Override
public void stop() {
this.startCount--;
}
@Override
public boolean isRunning() {
return this.startCount > 0;
}
int getIndex() {
@ -117,6 +210,10 @@ class TestcontainersStartupTests {
return this.threadName;
}
int getStartCount() {
return this.startCount;
}
}
}