Add AuTest for connect_attempts_max_retries_down_server#12922
Add AuTest for connect_attempts_max_retries_down_server#12922masaori335 wants to merge 1 commit intoapache:masterfrom
Conversation
60af02a to
8e43f53
Compare
8e43f53 to
473012f
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new AuTest (ATSReplay-based gold test) intended to exercise proxy.config.http.connect_attempts_max_retries_down_server, and extends the ATSReplay harness to allow DNS records to be specified directly in replay YAML.
Changes:
- Added a new replay scenario
connect_attempts_rr_down_server.replay.yamlundertests/gold_tests/dns/replay/. - Added a new gold test driver
tests/gold_tests/dns/connect_attempts.test.pyto run the replay. - Extended
ATSReplayTestto acceptautest.dns.recordsand apply them to the DNS server process viaaddRecords.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/gold_tests/dns/replay/connect_attempts_rr_down_server.replay.yaml | New replay scenario configuring connect-attempt retry records and expected proxy responses. |
| tests/gold_tests/dns/connect_attempts.test.py | New gold test entrypoint invoking the replay via Test.ATSReplayTest. |
| tests/gold_tests/autest-site/ats_replay.test.ext | ATSReplay harness enhancement: allow replay YAML to inject DNS records into the test DNS server. |
| # limitations under the License. | ||
|
|
||
| # | ||
| # This replay file assumes that caching is enabled. |
There was a problem hiding this comment.
The header comment says this replay assumes caching is enabled, but autest.ats.process_config.enable_cache is set to false. Please update/remove the comment to match the actual configuration (or enable cache if that was intended).
| # This replay file assumes that caching is enabled. | |
| # This replay file runs with caching disabled. |
| proxy-response: | ||
| status: 502 | ||
|
|
||
| # request expected hit down_server cache and immidiately get 500 |
There was a problem hiding this comment.
Typo in comment: immidiately should be immediately.
| # request expected hit down_server cache and immidiately get 500 | |
| # request expected hit down_server cache and immediately get 500 |
| proxy-response: | ||
| status: 500 |
There was a problem hiding this comment.
This transaction expects a 500 response, but ATS's "origin down" path returns 502 (see HttpTransact::OriginDown), and with proxy.config.http.connect_attempts_max_retries_down_server: 1 ATS will still attempt a connection to a marked-down origin rather than short-circuiting. The expected proxy-response.status here should be updated accordingly (likely 502), and the accompanying comment should be adjusted to reflect what behavior is being asserted.
This is an effort to verify
connect_attempts_max_retries_down_serverbehavior.The doc for
proxy.config.http.connect_attempts_max_retries_down_serversays below.Also doc of
proxy.config.http.connect_attempts_max_retriessays belowHowever, I don't see ATS makes connection attempts against down servers. From my understanding,
HostDBRecord::select_best_httpdoesn't returnHostDBInfowhich is in down state. HowHttpSMcan try to down servers?