Copilot commented on code in PR #13872:
URL: https://github.com/apache/trafficserver/pull/13872#discussion_r4321596146
##########
tests/gold_tests/pluginTest/stek_share/stek_share.test.py:
##########
@@ -311,6 +311,15 @@ def check_session(ev, test):
tr1.StillRunningAfter += ts4
tr1.StillRunningAfter += ts5
+# Each node checks for the leader's STEK when it joins the Raft cluster and
then
+# only every 10 seconds. A node whose first check comes before the key arrives
+# installs it about 10 seconds later, and a leader elected after its own first
+# check sends its key only on the next one. Wait until every node is using the
+# shared key rather than sleeping a fixed time.
+for i, ts in enumerate((ts1, ts2, ts3, ts4, ts5), 1):
+ Test.AddAwaitFileContainsTestRun(
+ f'Await ts{i} using the shared STEK', ts.Disk.traffic_out.Name,
'(Received new|Using initial|Using new) STEK')
Review Comment:
The await regex includes `Using initial` which can plausibly occur during
normal startup before a node has installed the *leader-shared* STEK. If that
happens, the await can pass too early and reintroduce the original race
(handshakes starting before the shared key is installed everywhere). Consider
tightening the match to only log messages that unambiguously indicate the
shared key was received/installed (e.g., `Received new STEK` / a log that
indicates installation from the cluster), or use separate leader vs follower
expectations if their log lines differ.
##########
tests/gold_tests/pluginTest/stek_share/stek_share.test.py:
##########
@@ -289,8 +289,8 @@ def check_session(ev, test):
return (retval, "Check that session ids match", err)
-tr1 = Test.AddTestRun('Basic Curl test, and give it enough time for all ATS to
start up and sync STEK')
-tr1.Processes.Default.Command = 'sleep 10 && curl https://127.0.0.1:{0}
-k'.format(ts1.Variables.ssl_port)
+tr1 = Test.AddTestRun('Basic Curl test')
+tr1.Processes.Default.Command = 'curl https://127.0.0.1:{0}
-k'.format(ts1.Variables.ssl_port)
Review Comment:
This change removes the only explicit delay before the first network
request. Since `StartBefore(ts1)` ensures process start ordering but not
readiness, `curl` may intermittently race ATS binding/listening (connection
refused / handshake failure), creating a new source of flakiness. To keep the
test robust, consider gating `tr1` on a readiness signal (e.g., await a
'ready/listening' log line or a port-open check) or moving this `curl` run
after the per-node STEK awaits (since those awaits already imply the nodes are
up and logging).
--
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]
