Skip to content

Commit

Permalink
[SPARK-50022][CORE][UI] Fix MasterPage to hide App UI links when UI…
Browse files Browse the repository at this point in the history
… is disabled

### What changes were proposed in this pull request?

This PR aims to fix `MasterPage` to hide App UI links when UI is disabled.

Previously, the link leads the following errors if a user clicks it.
<img width="997" alt="Screenshot 2024-10-17 at 22 06 22" src="https://github.com/user-attachments/assets/e53ba01f-533f-4d42-a488-830afaf40efa">

### Why are the changes needed?

**1. PREPARATION**

```
$ cat conf/spark-defaults.conf
spark.ui.reverseProxy true
spark.ui.reverseProxyUrl http://localhost:8080

$ sbin/start-master.sh

$ sbin/start-worker.sh spark://$(hostname):7077

$ bin/spark-shell --master spark://$(hostname):7077 -c spark.ui.enabled=false
```

**2. BEFORE**

<img width="340" alt="Screenshot 2024-10-17 at 22 01 16" src="https://github.com/user-attachments/assets/3069e43d-ba8c-4d36-8101-65e10b420879">

**3. AFTER**

<img width="345" alt="Screenshot 2024-10-17 at 22 04 12" src="https://github.com/user-attachments/assets/b9feba47-90fb-4557-803c-94eaa8ce62e1">

### Does this PR introduce _any_ user-facing change?

The previous behavior shows HTTP 500 error.

### How was this patch tested?

Pass the CIs with a newly added test case.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #48535 from dongjoon-hyun/SPARK-50022.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
  • Loading branch information
dongjoon-hyun committed Oct 18, 2024
1 parent ae87ce6 commit 9d0e31d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ private[ui] class MasterPage(parent: MasterWebUI) extends WebUIPage("") {
</td>
<td>
{
if (app.isFinished) {
if (app.isFinished || app.desc.appUiUrl.isBlank()) {
app.desc.name
} else {
<a href={UIUtils.makeHref(parent.master.reverseProxy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@

package org.apache.spark.deploy.master.ui

import java.util.Date

import scala.io.Source

import jakarta.servlet.http.HttpServletResponse.{SC_METHOD_NOT_ALLOWED, SC_OK}
import org.mockito.Mockito.{mock, when}

import org.apache.spark.{SecurityManager, SparkConf, SparkFunSuite}
import org.apache.spark.deploy.DeployMessages.{MasterStateResponse, RequestMasterState}
import org.apache.spark.deploy.master._
import org.apache.spark.deploy.master.ui.MasterWebUISuite._
import org.apache.spark.internal.config.DECOMMISSION_ENABLED
Expand All @@ -33,6 +36,8 @@ import org.apache.spark.util.Utils

class ReadOnlyMasterWebUISuite extends SparkFunSuite {

import org.apache.spark.deploy.DeployTestUtils._

val conf = new SparkConf()
.set(UI_KILL_ENABLED, false)
.set(DECOMMISSION_ENABLED, false)
Expand All @@ -45,6 +50,14 @@ class ReadOnlyMasterWebUISuite extends SparkFunSuite {
when(master.conf).thenReturn(conf)
when(master.rpcEnv).thenReturn(rpcEnv)
when(master.self).thenReturn(masterEndpointRef)
val desc1 = createAppDesc().copy(name = "WithUI")
val desc2 = desc1.copy(name = "WithoutUI", appUiUrl = "")
val app1 = new ApplicationInfo(new Date().getTime, "app1", desc1, new Date(), null, Int.MaxValue)
val app2 = new ApplicationInfo(new Date().getTime, "app2", desc2, new Date(), null, Int.MaxValue)
val state = new MasterStateResponse(
"host", 8080, None, Array.empty, Array(app1, app2), Array.empty,
Array.empty, Array.empty, RecoveryState.ALIVE)
when(masterEndpointRef.askSync[MasterStateResponse](RequestMasterState)).thenReturn(state)
val masterWebUI = new MasterWebUI(master, 0)

override def beforeAll(): Unit = {
Expand Down Expand Up @@ -97,4 +110,13 @@ class ReadOnlyMasterWebUISuite extends SparkFunSuite {
assert(result.contains("Environment Variables"))
assert(result.contains("<tr><td>SPARK_SCALA_VERSION</td><td>2.1"))
}

test("SPARK-50022: Fix 'MasterPage' to hide App UI links when UI is disabled") {
val url = s"http://${Utils.localHostNameForURI()}:${masterWebUI.boundPort}/"
val conn = sendHttpRequest(url, "GET")
assert(conn.getResponseCode === SC_OK)
val result = Source.fromInputStream(conn.getInputStream).mkString
assert(result.contains("<a href=\"appUiUrl\">WithUI</a>"))
assert(result.contains(" WithoutUI\n"))
}
}

0 comments on commit 9d0e31d

Please sign in to comment.