Switch to host names in integration tests #950
Conversation
Codecov Report
@@ Coverage Diff @@
## master #950 +/- ##
==========================================
- Coverage 55.85% 55.75% -0.11%
==========================================
Files 263 263
Lines 19541 19541
==========================================
- Hits 10915 10895 -20
- Misses 6900 6911 +11
- Partials 1726 1735 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
fln
changed the title
ti-chi-bot
added
size/XL
fln
force-pushed
the
host_names_in_integration_tests
branch
from
3a55657
to
c422d54
Compare
|
@9547 PRs are split. Just noticed that I have overlooked the request to have mixed IP and host name tests. Mixing host names and IP addresses in tests is not that trivial. If we mix host names and IP address in a single topology - Leaving some tests with host names while other test with IP addresses also causes issue because Is there any chance full test suite switch to host names would be accepted smile ? Alternatively would a simple test of just building a minimal cluster and tearing it down using only IP addresses suffice? My opinion is that if we have code that works with host names it should with high confidence work with IP addresses as well without explicit testing. |
This commit updates TLS certificate generator to detect if IP address or host name was used as host value. If host name is detected field `DNSNames` of x509 SAN extenstion is used instead of `IPAddresses`. * https://en.wikipedia.org/wiki/Subject_Alternative_Name * https://tools.ietf.org/html/rfc5280#section-4.2.1.6 This contributes towards fixing pingcap#337.
This commit updates run_pd.sh template to use `listen_host` for --peer-urls flag instead of `host`. This allows us to use server hostname instead of IP addresse. Without this change if PD service is declared using hostname it fails to start, reporting an error that it can not bind to given interface. Metadata variable `listen_host` was introduced in pingcap#495 and is already used for the --client-urls flag. This should resolve pingcap#337 and partially implement pingcap#691.
This PR updates refactors integration tests to use host names instead of IP addresses. It resolves pingcap#337. All IP literals in the integration tests are replaced with host names `n1` to `n5`. This allows us to make integration test topology files immutable and remove extra topology file templating step. Extra topology file `full_scale_in_tidb_2nd.yaml` is introduced for second tidb scaling operation to avoid topology file mutation during the test. Using host names in the integration tests will help us to maintain compatibility with services declared using host names. There should be no extra maintenance to support using IP addresses.
fln
force-pushed
the
host_names_in_integration_tests
branch
from
c422d54
to
aa451b7
Compare
ti-chi-bot
added
the
needs-rebase
indeed, they are different nodes, only the same if the dns record of the host equals the ip.
Are you run those tests locally? eq: docker exec -it tiup-control-panel
bash /tiup-cluster/tests/tiup-cluster/run.shIf so, the cmd and scale tests are running in serial, and the base env was not destroyed between tests :), but in Github Action, the environment is recreated before each test case is running. So don't worry about the shared problem. |
ti-chi-bot
removed
the
needs-rebase
|
/lgtm |
|
/merge |
ti-chi-bot
added
the
status/can-merge
|
Can merge label has been added. Git tree hash: 328c0d9
|
What problem does this PR solve?
This PR updates refactors integration tests to use host names instead of IP addresses. It resolves #337.
What is changed and how it works?
All IP literals in the integration tests are replaced with host names
n1ton5.This allows us to make integration test topology files immutable and remove extra topology file templating step.
Extra topology file
full_scale_in_tidb_2nd.yamlis introduced for second tidb scaling operation to avoid topology file mutation during the test.Using host names in the integration tests will help us to maintain compatibility with services declared using host names. There should be no extra maintenance to support using IP addresses.
Check List
Tests
Integration tests now tests functionality added in #948 and #949.
Side effects
Using host names will most likely break some services which are not yet tested with integration tests. But hopefully this will help ups to expand host name support where needed.
Related changes
Includes code from #948 and #949.
Release notes: