bneradt commented on code in PR #9181: URL: https://github.com/apache/trafficserver/pull/9181#discussion_r1020584842
########## tests/gold_tests/dns/replay/server_down.replay.yaml: ########## @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. + +meta: + version: "1.0" + +sessions: +- transactions: + - client-request: + delay: 2s Review Comment: Let's add comments explaining the reasoning for the delays in the replay file. In particular I'm currently trying to think through the relationship between this first delay and the DelayStart directives in the test.py file. Shouldn't only one of those be needed? ########## tests/gold_tests/dns/dns_host_down.test.py: ########## @@ -0,0 +1,96 @@ +''' +Verify ATS handles down origin servers with domain cached correctly. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. +from ports import get_port +import os + +Test.Summary = ''' +Verify ATS handles down origin servers with cached domain correctly. +''' + + +class DownCachedOriginServerTest: + replay_file = "replay/server_down.replay.yaml" + + def __init__(self): + """Initialize the Test processes for the test runs.""" + self._dns_port = None + + def _configure_trafficserver(self): + """Configure Traffic Server.""" + self._ts = Test.MakeATSProcess("ts", enable_cache=False) + + self._ts.Disk.remap_config.AddLine( + f"map / http://resolve.this.com:{self._server.Variables.http_port}/"; + ) + + self._ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'hostdb|dns|http|socket', + 'proxy.config.http.connect_attempts_max_retries': 0, + 'proxy.config.http.connect_attempts_rr_retries': 0, + 'proxy.config.hostdb.fail.timeout': 10, + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.hostdb.ttl_mode': 1, + 'proxy.config.hostdb.timeout': 2, + 'proxy.config.hostdb.lookup_timeout': 2, + 'proxy.config.http.transaction_no_activity_timeout_in': 2, + 'proxy.config.http.connect_attempts_timeout': 2, + 'proxy.config.hostdb.host_file.interval': 1, + 'proxy.config.hostdb.host_file.path': os.path.join(Test.TestDirectory, "hosts_file"), + }) + + # Even when the origin server is down, SM will return a hit-fresh domain from HostDB. + # After request has failed, SM should mark the IP as down + def _test_host_mark_down(self): + tr = Test.AddTestRun() + self._server = tr.AddVerifierServerProcess("server", DownCachedOriginServerTest.replay_file) + self._configure_trafficserver() Review Comment: I suggest moving this to the `__init__` function since it's a Test-level rather than a TestRun-level process. ########## tests/gold_tests/dns/replay/server_down.replay.yaml: ########## @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. + +meta: + version: "1.0" + +sessions: +- transactions: + - client-request: + delay: 2s + method: "GET" + version: "1.1" + url: /dns/mark/down + headers: + fields: + - [ Host, example.com ] + - [ X-Request, request ] + - [ uuid, 1 ] + + server-response: + status: 200 + + proxy-response: + status: 502 Review Comment: Comments explaining the reasoning of these expected return codes would be helpful too. ########## tests/gold_tests/dns/dns_host_down.test.py: ########## @@ -0,0 +1,96 @@ +''' +Verify ATS handles down origin servers with domain cached correctly. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. +from ports import get_port +import os + +Test.Summary = ''' +Verify ATS handles down origin servers with cached domain correctly. +''' + + +class DownCachedOriginServerTest: + replay_file = "replay/server_down.replay.yaml" + + def __init__(self): + """Initialize the Test processes for the test runs.""" + self._dns_port = None + + def _configure_trafficserver(self): + """Configure Traffic Server.""" + self._ts = Test.MakeATSProcess("ts", enable_cache=False) + + self._ts.Disk.remap_config.AddLine( + f"map / http://resolve.this.com:{self._server.Variables.http_port}/"; + ) + + self._ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'hostdb|dns|http|socket', + 'proxy.config.http.connect_attempts_max_retries': 0, + 'proxy.config.http.connect_attempts_rr_retries': 0, + 'proxy.config.hostdb.fail.timeout': 10, + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.hostdb.ttl_mode': 1, + 'proxy.config.hostdb.timeout': 2, + 'proxy.config.hostdb.lookup_timeout': 2, + 'proxy.config.http.transaction_no_activity_timeout_in': 2, + 'proxy.config.http.connect_attempts_timeout': 2, + 'proxy.config.hostdb.host_file.interval': 1, + 'proxy.config.hostdb.host_file.path': os.path.join(Test.TestDirectory, "hosts_file"), + }) + + # Even when the origin server is down, SM will return a hit-fresh domain from HostDB. + # After request has failed, SM should mark the IP as down + def _test_host_mark_down(self): + tr = Test.AddTestRun() + self._server = tr.AddVerifierServerProcess("server", DownCachedOriginServerTest.replay_file) + self._configure_trafficserver() + + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + + tr.DelayStart = 1 + tr.AddVerifierClientProcess( + "client-1", + DownCachedOriginServerTest.replay_file, + http_ports=[self._ts.Variables.port], + other_args='--keys 1') + + tr.NotRunningAfter = self._server + + # After host has been marked down from previous test, HostDB should not return + # the host as available and DNS lookup should fail. Review Comment: Any diags.log entries we should verify with the test via ContainsExpression? (traffic.out is debug-level, and it's generally good to avoid depending upon those since they may change more regularly.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
