[incubator-uniffle] branch master updated: [MINOR] Fix flaky test testGetHostIp (#141)

[ro...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 04cbdbb  [MINOR] Fix flaky test testGetHostIp (#141)
04cbdbb is described below

commit 04cbdbb6eec98cdd02fcc770e0cf1298db281c4f
Author: Chen Zhang <67...@users.noreply.github.com>
AuthorDate: Mon Aug 8 16:35:05 2022 +0800

    [MINOR] Fix flaky test testGetHostIp (#141)
    
    ### What changes were proposed in this pull request?
    
    Do not use `InetAddress.getLocalHost().getHostAddress()`  in test because it is indeterminate. The return value may be different on a multi-NIC machine or on different system.
    
    ### Why are the changes needed?
    
    In addition to 127.0.0.1, the loopback address 127.0.1.1 can also be returned, which happens on Debian.This will make UT fail.
    
    ```
    org.opentest4j.AssertionFailedError:
    Expected :127.0.1.1
    Actual   :10.0.191.230
    
    at org.apache.uniffle.common.util.RssUtilsTest.testGetHostIp(RssUtilsTest.java:64)
    ```
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    UT.
---
 .../main/java/org/apache/uniffle/common/util/RssUtils.java |  8 ++++----
 .../java/org/apache/uniffle/common/util/RssUtilsTest.java  | 14 ++++++++------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java b/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
index 8b7f500..1224d8a 100644
--- a/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
+++ b/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
@@ -97,10 +97,10 @@ public class RssUtils {
     return result;
   }
 
-  // `InetAddress.getLocalHost().getHostAddress()` could return 127.0.0.1. To avoid
-  // this situation, we can get current ip through network interface (filtered ipv6,
-  // loop back, etc.). If the network interface in the machine is more than one, we
-  // will choose the first IP.
+  // `InetAddress.getLocalHost().getHostAddress()` could return
+  // 127.0.0.1 (127.0.1.1 on Debian). To avoid this situation, we can get current
+  // ip through network interface (filtered ipv6, loop back, etc.).
+  // If the network interface in the machine is more than one, we will choose the first IP.
   public static String getHostIp() throws Exception {
     // For K8S, there are too many IPs, it's hard to decide which we should use.
     // So we use the environment variable to tell RSS to use which one.
diff --git a/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java b/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
index 3c946eb..97e732f 100644
--- a/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
+++ b/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
@@ -18,7 +18,9 @@
 package org.apache.uniffle.common.util;
 
 import java.lang.reflect.Field;
+import java.net.Inet4Address;
 import java.net.InetAddress;
+import java.net.NetworkInterface;
 import java.nio.ByteBuffer;
 import java.util.Collections;
 import java.util.List;
@@ -35,7 +37,7 @@ import org.apache.uniffle.common.ShuffleDataSegment;
 import org.apache.uniffle.common.ShuffleIndexResult;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
@@ -57,12 +59,12 @@ public class RssUtilsTest {
   @Test
   public void testGetHostIp() {
     try {
-      String address = InetAddress.getLocalHost().getHostAddress();
       String realIp = RssUtils.getHostIp();
-      assertNotEquals("127.0.0.1", realIp);
-      if (!address.equals("127.0.0.1")) {
-        assertEquals(address, realIp);
-      }
+      InetAddress ia = InetAddress.getByName(realIp);
+      assertTrue(ia instanceof Inet4Address);
+      assertFalse(ia.isLinkLocalAddress() || ia.isAnyLocalAddress() || ia.isLoopbackAddress());
+      assertTrue(NetworkInterface.getByInetAddress(ia) != null);
+      assertTrue(ia.isReachable(5000));
       setEnv("RSS_IP", "8.8.8.8");
       assertEquals("8.8.8.8", RssUtils.getHostIp());
       setEnv("RSS_IP", "xxxx");