[GitHub] [cassandra] jonmeredith commented on a diff in pull request #1706: Fixing flaky tests in InternodeEncryptionEnforcementTest

["jonmeredith (via GitHub)" <gi...@apache.org>]

jonmeredith commented on code in PR #1706:
URL: https://github.com/apache/cassandra/pull/1706#discussion_r1107893477


##########
test/distributed/org/apache/cassandra/distributed/test/InternodeEncryptionEnforcementTest.java:
##########
@@ -164,7 +149,9 @@ public void testOutboundConnectionsAreInterruptedWhenAuthFails() throws IOExcept
                 assertTrue(!outbound.small.isConnected() && !outbound.large.isConnected() && !outbound.urgent.isConnected());
             };
             cluster.get(1).runOnInstance(runnable);
+            cluster.get(1).shutdown();

Review Comment:
   Why shutdown before the try block completes and closes the cluster?



##########
src/java/org/apache/cassandra/net/OutboundConnectionInitiator.java:
##########
@@ -147,6 +147,7 @@ private Future<Result<SuccessType>> initiate(EventLoop eventLoop)
         {
             // interrupt other connections, so they must attempt to re-authenticate
             MessagingService.instance().interruptOutbound(settings.to);
+            logger.error("authentication failed to " + settings.connectToId());

Review Comment:
   We normally capitalize log messages - `Authentication failed`



##########
test/distributed/org/apache/cassandra/distributed/test/InternodeEncryptionEnforcementTest.java:
##########
@@ -432,7 +426,8 @@ public static class AllowFirstAndRejectOtherOutboundAuthenticator extends Reject
         @Override
         public boolean authenticate(InetAddress remoteAddress, int remotePort, Certificate[] certificates, InternodeConnectionDirection connectionType)
         {
-            if (connectionType == InternodeConnectionDirection.OUTBOUND)
+            if (connectionType == InternodeConnectionDirection.OUTBOUND ||
+                connectionType == InternodeConnectionDirection.OUTBOUND_PRECONNECT)

Review Comment:
   why add `OUTBOUND_PRECONNECT`? Doesn't that mean you don't exercise the outbound auth failure path as it always preconnects before the real auth attempt.



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org