Multi-AZ integration tests #328
Conversation
congoamz
force-pushed
the
multi-az-integ
branch
2 times, most recently
from
64672fd to
598afbc
Compare
| self._cluster_instance_template: HostInfo | ||
| host_pattern = WrapperProperties.CLUSTER_INSTANCE_HOST_PATTERN.get(self._props) | ||
| if host_pattern: | ||
| if host_pattern.find(":") > -1: |
There was a problem hiding this comment.
I was running into a problem where the proxy tests weren't working because the host list provider was using 3306 instead of the proxy port. Talked with Sergiy and we decided to expand the cluster instance host pattern so that you can also specify a port that will override the one returned by the topology query.
| assert aurora_utility.is_db_instance_writer(current_connection_id) is True | ||
| assert current_connection_id != initial_writer_id | ||
|
|
||
| @pytest.mark.skip |
There was a problem hiding this comment.
Not sure why we originally disabled this, test seems to be passing now
|
|
||
| conn = self._open_connection(instance_info) | ||
| cursor = conn.cursor() | ||
| get_writer_id_query = self._get_multi_az_writer_sql(engine) |
There was a problem hiding this comment.
The tests depend on RdsTestUtility.get_instance_ids to return the writer in the first position. To do this with multi-az clusters we have to identify the writer before querying for the topology
| } | ||
|
|
||
| if (!excludeFailover) { | ||
| LOGGER.warning("Tests requiring database failover are not supported for multi-az clusters. It may take 1hr+" + |
There was a problem hiding this comment.
This line will be hit by our gh action integration tests because excludeFailover must not be set so that the failover tests still run for Aurora. Should I make this info() instead of warning()? Or make it a comment instead of a log message?
tests/integration/host/src/test/java/integration/host/TestEnvironmentProvider.java
Outdated
Show resolved
Hide resolved
| clusterBuilder = clusterBuilder.enableIAMDatabaseAuthentication(true); | ||
| } else if (DatabaseEngineDeployment.MULTI_AZ.equals(this.dbEngineDeployment)) { | ||
| clusterBuilder = | ||
| clusterBuilder.allocatedStorage(allocatedStorage) |
There was a problem hiding this comment.
These settings are required when creating a multi-az cluster
| # That is expected exception. Test pass. | ||
| assert True | ||
|
|
||
| @pytest.mark.skip("Currently failing, will be fixed in another PR") |
There was a problem hiding this comment.
This test was being skipped on main because of a bug. I got it to run now but it is failing with aurora for reasons unrelated to multi-az changes. I'll fix this in another PR
| r"(?P<region>[a-zA-Z0-9\-]+)\.rds\.amazonaws\.com)" | ||
| r"(?P<dns>proxy-|cluster-|cluster-ro-|cluster-custom-)?" \ | ||
| r"(?P<domain>[a-zA-Z0-9]+\." \ | ||
| r"(?P<region>[a-zA-Z0-9\-]+)\.rds\.amazonaws\.com)(?!\.cn$)" |
There was a problem hiding this comment.
I'm not sure if this regular expression is right. That's what implemented in JDBC:
<instance>.cluster-<xyz>.<region>.rds.amazonaws.com
<instance>.cluster-<xyz>.rds.<region>.amazonaws.com.cn
region and rds are taking different places in url.
There was a problem hiding this comment.
Ya we don't actually know what is the correct pattern, we are confirming the pattern with Dave @sergiyvamz
There was a problem hiding this comment.
Also, the jdbc file mixes both patterns you mention, which is probably incorrect, its probably one or the other. I made this change to unify them to the same pattern, although we admittedly don't know which pattern is correct yet
congoamz
force-pushed
the
multi-az-integ
branch
from
9ea1d7e to
dd8e381
Compare
congoamz
force-pushed
the
multi-az-integ
branch
from
64c92a8 to
aaacc91
Compare
5 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.