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

chore: refactor AuroraTestUtility #1252

Merged
merged 23 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
482f5ad
test: update mysql and testcontainers dependencies to fix flaky tests
aaron-congo Jan 14, 2025
db91c85
fix: test if host is allowed using hostname equality
aaron-congo Jan 15, 2025
6f31328
test: close topology monitors before each test
aaron-congo Jan 16, 2025
e277287
Add note about org.testcontainers dependencies requiring the same ver…
aaron-congo Jan 16, 2025
3454081
catch ClusterTopologyMonitorImpl.createHost exceptions, make sure pro…
aaron-congo Jan 17, 2025
515e2d7
wip
aaron-congo Jan 16, 2025
83775e8
Address warnings
aaron-congo Jan 17, 2025
0188dab
wip
aaron-congo Jan 20, 2025
fe81eb9
Rename TestEnvironment classes
aaron-congo Jan 20, 2025
1cc6436
wip
aaron-congo Jan 20, 2025
1eb072d
Rename auroraUtil to testUtil
aaron-congo Jan 20, 2025
2ed579e
wip javadocs
aaron-congo Jan 20, 2025
92a2a2b
Remove instances parameter from createCluster functions
aaron-congo Jan 21, 2025
b0f6236
Finish javadocs
aaron-congo Jan 21, 2025
ce05def
cleanup
aaron-congo Jan 22, 2025
558fb00
Merge branch 'main' into test-utility
aaron-congo Jan 22, 2025
d0af9fd
Rename HostEnvironment and ContainerEnvironment back to TestEnvironment
aaron-congo Jan 22, 2025
1adaa8d
Rename TestUtility back to AuroraTestUtility
aaron-congo Jan 22, 2025
9990eab
Rename testUtil variable back to auroraUtil
aaron-congo Jan 22, 2025
98a1b7f
Merge branch 'main' into test-utility
aaron-congo Jan 22, 2025
435626b
Fix checkstyle
aaron-congo Jan 22, 2025
1bbe5ee
cleanup
aaron-congo Jan 22, 2025
c486682
Fix mistake in formation of test host suffix
aaron-congo Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
import software.amazon.jdbc.ConnectionProviderManager;
import software.amazon.jdbc.Driver;
import software.amazon.jdbc.HikariPoolConfigurator;
import software.amazon.jdbc.HikariPooledConnectionProvider;
import software.amazon.jdbc.PropertyDefinition;
Expand Down Expand Up @@ -116,7 +117,7 @@ public void test_pooledConnectionAutoScaling_setReadOnlyOnOldConnection()
null,
poolExpirationNanos,
TimeUnit.MINUTES.toNanos(10));
ConnectionProviderManager.setConnectionProvider(provider);
Driver.setCustomConnectionProvider(provider);

final List<Connection> connections = new ArrayList<>();
try {
Expand All @@ -127,8 +128,11 @@ public void test_pooledConnectionAutoScaling_setReadOnlyOnOldConnection()
}

final Connection newInstanceConn;
final String instanceClass =
auroraUtil.getDbInstanceClass(TestEnvironment.getCurrent().getInfo().getRequest());
final TestInstanceInfo newInstance =
auroraUtil.createInstance("auto-scaling-instance");
auroraUtil.createInstance(instanceClass, "auto-scaling-instance");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before these changes, the new instance was added to the TestEnvironment in a convoluted way: an earlier call to AuroraTestUtility would pass in the TestEnvironment instance list, which the class would then assign to an internal instance variable. Then, in this call, the instance list would be updated. In this PR we removed the internal instance variable. Should I change the method signature to require the TestEnvironment instance list to be passed in so that we can update it inside the function instead of relying on callers to update it themselves? It would be convenient but it also would be a side effect of calling the function

instances.add(newInstance);
try {
newInstanceConn =
DriverManager.getConnection(ConnectionStringHelper.getWrapperUrl(), props);
Expand All @@ -144,6 +148,7 @@ public void test_pooledConnectionAutoScaling_setReadOnlyOnOldConnection()
newInstanceConn.setReadOnly(false);
} finally {
auroraUtil.deleteInstance(newInstance);
instances.remove(newInstance);
}

final long deletionCheckTimeout = System.nanoTime() + TimeUnit.MINUTES.toNanos(5);
Expand Down Expand Up @@ -200,8 +205,11 @@ public void test_pooledConnectionAutoScaling_failoverFromDeletedReader()
}

final Connection newInstanceConn;
final String instanceClass =
auroraUtil.getDbInstanceClass(TestEnvironment.getCurrent().getInfo().getRequest());
final TestInstanceInfo newInstance =
auroraUtil.createInstance("auto-scaling-instance");
auroraUtil.createInstance(instanceClass, "auto-scaling-instance");
instances.add(newInstance);
try {
newInstanceConn =
DriverManager.getConnection(ConnectionStringHelper.getWrapperUrl(newInstance), props);
Expand All @@ -214,6 +222,7 @@ public void test_pooledConnectionAutoScaling_failoverFromDeletedReader()
.anyMatch((url) -> url.equals(newInstance.getUrl())));
} finally {
auroraUtil.deleteInstance(newInstance);
instances.remove(newInstance);
}

auroraUtil.assertFirstQueryThrows(newInstanceConn, FailoverSuccessSQLException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
public class HikariTests {

private static final Logger LOGGER = Logger.getLogger(HikariTests.class.getName());
protected static final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();

@TestTemplate
public void testOpenConnectionWithUrl() throws SQLException {
Expand Down Expand Up @@ -176,8 +177,6 @@ public void testOpenConnectionWithDataSourceClassName() throws SQLException {
@EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED})
@EnableOnNumOfInstances(min = 3)
public void testFailoverLostConnection() throws SQLException {

final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();
final Properties customProps = new Properties();
PLUGINS.set(customProps, "failover");
FAILOVER_TIMEOUT_MS.set(customProps, Integer.toString(1));
Expand Down Expand Up @@ -218,8 +217,6 @@ public void testFailoverLostConnection() throws SQLException {
@EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED})
@EnableOnNumOfInstances(min = 3)
public void testEFMFailover() throws SQLException {

final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();
ProxyHelper.disableAllConnectivity();

final List<TestInstanceInfo> instances = TestEnvironment.getCurrent()
Expand Down Expand Up @@ -270,7 +267,6 @@ public void testEFMFailover() throws SQLException {
@EnableOnNumOfInstances(min = 2)
public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation()
throws SQLException, InterruptedException {
final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();
final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo();
final List<TestInstanceInfo> instances = proxyInfo.getInstances();
final TestInstanceInfo reader = instances.get(1);
Expand Down Expand Up @@ -311,7 +307,6 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation()
@EnableOnNumOfInstances(min = 2)
public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation()
throws SQLException, InterruptedException {
final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();
final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo();
final List<TestInstanceInfo> instances = proxyInfo.getInstances();
final TestInstanceInfo writer = instances.get(0);
Expand Down Expand Up @@ -360,7 +355,6 @@ public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation()
@EnableOnNumOfInstances(max = 1)
public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_singleInstance()
throws SQLException, InterruptedException {
final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility();
final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo();
final List<TestInstanceInfo> instances = proxyInfo.getInstances();
final TestInstanceInfo writer = instances.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package integration.container.tests;

import com.zaxxer.hikari.HikariConfig;
import integration.DriverHelper;
import integration.TestEnvironmentFeatures;
import integration.container.ConnectionStringHelper;
import integration.container.TestDriverProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,8 @@ public void test_failoverReaderToWriter_setReadOnlyTrueFalse()

ProxyHelper.enableAllConnectivity();

// During failover above some of readers have been tried to connect to and failed since they were not
// available. We should expect that some of readers in topology are marked as UNAVAILABLE.
// During failover above some of the readers have been tried to connect to and failed since they were not
// available. We should expect that some of the readers in topology are marked as UNAVAILABLE.
// The following code reset node availability and make them AVAILABLE.
// That is important for further steps.
TestPluginServiceImpl.clearHostAvailabilityCache();
Expand Down Expand Up @@ -857,7 +857,7 @@ public void test_pooledConnection_differentUsers() throws SQLException {

assertThrows(
HikariPool.PoolInitializationException.class, () -> {
try (final Connection conn = DriverManager.getConnection(
try (final Connection ignored = DriverManager.getConnection(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just fixing an intellij warning here

ConnectionStringHelper.getWrapperUrl(), wrongUserRightPasswordProps)) {
// Do nothing (close connection automatically)
}
Expand Down
77 changes: 33 additions & 44 deletions wrapper/src/test/java/integration/host/TestEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import org.testcontainers.shaded.org.apache.commons.lang3.NotImplementedException;
import org.testcontainers.utility.MountableFile;
import software.amazon.awssdk.services.rds.model.DBCluster;
import software.amazon.awssdk.services.rds.model.DBInstance;
import software.amazon.jdbc.util.StringUtils;

public class TestEnvironment implements AutoCloseable {
Expand Down Expand Up @@ -174,8 +175,7 @@ public static TestEnvironment build(TestEnvironmentRequest request) throws IOExc
return env;
}

private static TestEnvironment createAuroraOrMultiAzEnvironment(TestEnvironmentRequest request)
throws URISyntaxException {
private static TestEnvironment createAuroraOrMultiAzEnvironment(TestEnvironmentRequest request) {

EnvPreCreateInfo preCreateInfo =
TestEnvironmentProvider.preCreateInfos.get(request.getEnvPreCreateIndex());
Expand Down Expand Up @@ -325,7 +325,7 @@ private static void createDatabaseContainers(TestEnvironment env) {
}
}

private static void createDbCluster(TestEnvironment env) throws URISyntaxException {
private static void createDbCluster(TestEnvironment env) {

switch (env.info.getRequest().getDatabaseInstances()) {
case SINGLE_INSTANCE:
Expand All @@ -351,7 +351,7 @@ private static void createDbCluster(TestEnvironment env) throws URISyntaxExcepti
}
}

private static void createDbCluster(TestEnvironment env, int numOfInstances) throws URISyntaxException {
private static void createDbCluster(TestEnvironment env, int numOfInstances) {

env.info.setRegion(
!StringUtils.isNullOrEmpty(config.rdsDbRegion)
Expand All @@ -371,9 +371,6 @@ private static void createDbCluster(TestEnvironment env, int numOfInstances) thr
env.awsAccessKeyId,
env.awsSecretAccessKey,
env.awsSessionToken);

ArrayList<TestInstanceInfo> instances = new ArrayList<>();

if (env.reuseAuroraDbCluster) {
if (StringUtils.isNullOrEmpty(env.auroraClusterDomain)) {
throw new RuntimeException("Environment variable RDS_CLUSTER_DOMAIN is required.");
Expand Down Expand Up @@ -403,8 +400,6 @@ private static void createDbCluster(TestEnvironment env, int numOfInstances) thr

env.info.setDatabaseEngine(clusterInfo.engine());
env.info.setDatabaseEngineVersion(clusterInfo.engineVersion());
instances.addAll(env.auroraUtil.getClusterInstanceIds(env.auroraClusterName));

} else {
if (StringUtils.isNullOrEmpty(env.auroraClusterName)) {
int remainingTries = 5;
Expand All @@ -431,23 +426,30 @@ private static void createDbCluster(TestEnvironment env, int numOfInstances) thr
if (StringUtils.isNullOrEmpty(engineVersion)) {
throw new RuntimeException("Failed to get engine version.");
}
String instanceClass = getDbInstanceClass(env.info.getRequest());
String instanceClass = env.auroraUtil.getDbInstanceClass(env.info.getRequest());

LOGGER.finer(
"Using " + engine + " " + engineVersion);

env.auroraClusterDomain =
env.auroraUtil.createCluster(
env.info.getDatabaseInfo().getUsername(),
env.info.getDatabaseInfo().getPassword(),
env.info.getDatabaseInfo().getDefaultDbName(),
env.auroraClusterName,
env.info.getRequest().getDatabaseEngineDeployment(),
engine,
instanceClass,
engineVersion,
numOfInstances,
instances);
env.auroraUtil.createCluster(
env.info.getDatabaseInfo().getUsername(),
env.info.getDatabaseInfo().getPassword(),
env.info.getDatabaseInfo().getDefaultDbName(),
env.auroraClusterName,
env.info.getRequest().getDatabaseEngineDeployment(),
env.info.getRegion(),
engine,
instanceClass,
engineVersion,
numOfInstances);

List<DBInstance> dbInstances = env.auroraUtil.getDBInstances(env.auroraClusterName);
if (dbInstances.isEmpty()) {
throw new RuntimeException("Failed to get instance information for cluster " + env.auroraClusterName);
}

final String instanceEndpoint = dbInstances.get(0).endpoint().address();
env.auroraClusterDomain = instanceEndpoint.substring(instanceEndpoint.indexOf(".") + 1);
env.info.setDatabaseEngine(engine);
env.info.setDatabaseEngineVersion(engineVersion);
LOGGER.finer(
Expand All @@ -458,7 +460,7 @@ private static void createDbCluster(TestEnvironment env, int numOfInstances) thr

// remove cluster and instances
LOGGER.finer("Deleting cluster " + env.auroraClusterName);
env.auroraUtil.deleteCluster(env.auroraClusterName);
env.auroraUtil.deleteCluster(env.auroraClusterName, env.info.getRequest().getDatabaseEngineDeployment());
LOGGER.finer("Deleted cluster " + env.auroraClusterName);

throw new RuntimeException(e);
Expand All @@ -478,6 +480,7 @@ private static void createDbCluster(TestEnvironment env, int numOfInstances) thr
env.auroraClusterName + ".cluster-ro-" + env.auroraClusterDomain, port);
env.info.getDatabaseInfo().setInstanceEndpointSuffix(env.auroraClusterDomain, port);

List<TestInstanceInfo> instances = env.auroraUtil.getTestInstancesInfo(env.auroraClusterName);
env.info.getDatabaseInfo().getInstances().clear();
env.info.getDatabaseInfo().getInstances().addAll(instances);

Expand Down Expand Up @@ -586,18 +589,6 @@ private static String findEngineVersion(
}
}

private static String getDbInstanceClass(TestEnvironmentRequest request) {
switch (request.getDatabaseEngineDeployment()) {
case AURORA:
return "db.r5.large";
case RDS:
case RDS_MULTI_AZ_CLUSTER:
return "db.m5d.large";
default:
throw new NotImplementedException(request.getDatabaseEngine().toString());
}
}

private static int getPort(TestEnvironmentRequest request) {
switch (request.getDatabaseEngine()) {
case MYSQL:
Expand Down Expand Up @@ -979,16 +970,14 @@ private String[] buildHibernateCommands(boolean debugMode) {

@Override
public void close() throws Exception {
if (this.databaseContainers != null) {
for (GenericContainer<?> container : this.databaseContainers) {
try {
container.stop();
} catch (Exception ex) {
// ignore
}
for (GenericContainer<?> container : this.databaseContainers) {
try {
container.stop();
} catch (Exception ex) {
// ignore
}
this.databaseContainers.clear();
}
this.databaseContainers.clear();

if (this.telemetryXRayContainer != null) {
this.telemetryXRayContainer.stop();
Expand Down Expand Up @@ -1035,7 +1024,7 @@ private void deleteDbCluster() {

if (!this.reuseAuroraDbCluster) {
LOGGER.finest("Deleting cluster " + this.auroraClusterName + ".cluster-" + this.auroraClusterDomain);
auroraUtil.deleteCluster(this.auroraClusterName);
auroraUtil.deleteCluster(this.auroraClusterName, this.info.getRequest().getDatabaseEngineDeployment());
LOGGER.finest("Deleted cluster " + this.auroraClusterName + ".cluster-" + this.auroraClusterDomain);
}
}
Expand Down
Loading
Loading