You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ge...@apache.org on 2021/04/09 13:20:06 UTC

[spark] branch branch-3.1 updated: [SPARK-35004][TEST] Fix Incorrect assertion of `master/worker web ui available behind front-end reverseProxy` in MasterSuite

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

gengliang pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new c5ba6bb  [SPARK-35004][TEST] Fix Incorrect assertion of `master/worker web ui available behind front-end reverseProxy` in MasterSuite
c5ba6bb is described below

commit c5ba6bb3eb724db7ee455c56a00f0043220af864
Author: yangjie01 <ya...@baidu.com>
AuthorDate: Fri Apr 9 21:18:49 2021 +0800

    [SPARK-35004][TEST] Fix Incorrect assertion of `master/worker web ui available behind front-end reverseProxy` in MasterSuite
    
    ### What changes were proposed in this pull request?
    Line 425 in `MasterSuite` is considered as unused expression by Intellij IDE,
    
    https://github.com/apache/spark/blob/bfba7fadd2e65c853971fb2983bdea1c52d1ed7f/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala#L421-L426
    
    If we merge lines 424 and 425 into one as:
    
    ```
    System.getProperty("spark.ui.proxyBase") should startWith (s"$reverseProxyUrl/proxy/worker-")
    ```
    
    this assertion will fail:
    
    ```
    - master/worker web ui available behind front-end reverseProxy *** FAILED ***
      The code passed to eventually never returned normally. Attempted 45 times over 5.091914027 seconds. Last failure message: "http://proxyhost:8080/path/to/spark" did not start with substring "http://proxyhost:8080/path/to/spark/proxy/worker-". (MasterSuite.scala:405)
    ```
    
    `System.getProperty("spark.ui.proxyBase")` should be `reverseProxyUrl` because `Master#onStart` and `Worker#handleRegisterResponse` have not changed it.
    
    So the main purpose of this pr is to fix the condition of this assertion.
    
    ### Why are the changes needed?
    Bug fix.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    
    - Pass the Jenkins or GitHub Action
    
    - Manual test:
    
    1. merge lines 424 and 425 in `MasterSuite` into one to eliminate the unused expression:
    
    ```
    System.getProperty("spark.ui.proxyBase") should startWith (s"$reverseProxyUrl/proxy/worker-")
    ```
    
    2. execute `mvn clean test -pl core -Dtest=none -DwildcardSuites=org.apache.spark.deploy.master.MasterSuite`
    
    **Before**
    
    ```
    - master/worker web ui available behind front-end reverseProxy *** FAILED ***
      The code passed to eventually never returned normally. Attempted 45 times over 5.091914027 seconds. Last failure message: "http://proxyhost:8080/path/to/spark" did not start with substring "http://proxyhost:8080/path/to/spark/proxy/worker-". (MasterSuite.scala:405)
    
    Run completed in 1 minute, 14 seconds.
    Total number of tests run: 32
    Suites: completed 2, aborted 0
    Tests: succeeded 31, failed 1, canceled 0, ignored 0, pending 0
    *** 1 TEST FAILED ***
    
    ```
    
    **After**
    
    ```
    Run completed in 1 minute, 11 seconds.
    Total number of tests run: 32
    Suites: completed 2, aborted 0
    Tests: succeeded 32, failed 0, canceled 0, ignored 0, pending 0
    All tests passed.
    ```
    
    Closes #32105 from LuciferYang/SPARK-35004.
    
    Authored-by: yangjie01 <ya...@baidu.com>
    Signed-off-by: Gengliang Wang <lt...@gmail.com>
    (cherry picked from commit c06758834e6192b1888b4a885c612a151588b390)
    Signed-off-by: Gengliang Wang <lt...@gmail.com>
---
 .../test/scala/org/apache/spark/deploy/master/MasterSuite.scala    | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
index 9296274..07ef46d 100644
--- a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
@@ -418,12 +418,7 @@ class MasterSuite extends SparkFunSuite
           (workerResponse \ "masterwebuiurl").extract[String] should be (reverseProxyUrl + "/")
         }
 
-        // with LocalCluster, we have masters and workers in the same JVM, each overwriting
-        // system property spark.ui.proxyBase.
-        // so we need to manage this property explicitly for test
-        System.getProperty("spark.ui.proxyBase") should startWith
-          (s"$reverseProxyUrl/proxy/worker-")
-        System.setProperty("spark.ui.proxyBase", reverseProxyUrl)
+        System.getProperty("spark.ui.proxyBase") should be (reverseProxyUrl)
         val html = Utils
           .tryWithResource(Source.fromURL(s"$masterUrl/"))(_.getLines().mkString("\n"))
         html should include ("Spark Master at spark://")

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org