Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/20327#discussion_r176584238
--- Diff:
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
---
@@ -136,6 +135,39 @@ class YarnClusterSuite extends BaseYarnClusterSuite {
checkResult(finalState, result)
}
+ private def testClusterDriverBind(
+ uiEnabled: Boolean,
+ localHost: String,
+ localIp: String,
+ success: Boolean): Unit = {
+ val result = File.createTempFile("result", null, tempDir)
+ val finalState = runSpark(false,
mainClassName(YarnClusterDriver.getClass),
+ appArgs = Seq(result.getAbsolutePath()),
+ extraConf = Map(
+ "spark.yarn.appMasterEnv.SPARK_LOCAL_HOSTNAME" -> localHost,
+ "spark.yarn.appMasterEnv.SPARK_LOCAL_IP" -> localIp,
+ "spark.ui.enabled" -> uiEnabled.toString
+ ))
+ if (success) {
+ checkResult(finalState, result, "success")
+ } else {
+ finalState should be (SparkAppHandle.State.FAILED)
+ }
+ }
+
+ test("yarn-cluster driver should be able to bind listeners to MM_HOST") {
--- End diff --
> So I'm not sure the tests are actually that useful. The way they're
written, as yarn apps, actually makes them very expensive, and this is testing
basic networking config that we know will not work if the IPs are invalid.
I agree the tests are not cheap. However, they show
- the regression of not being able to bind webUI to a specific interface is
fixed
- they demonstrate how to bind RPC and webUI to different interfaces
> The actual change you're introducing - using the bind address as the
address of the NM, is actually wrong if you think about it. It just happens
that the default value of that config is the local host name.
It's not wrong. it's one of the reasonable choices. Moreover it's one that
is consistent with the setting executors use to bind RPC. Obviously there can
be others.
> So basically if setting those env variables in the AM fixes the issue I'm
not sure there's any need to change anything in Spark.
We need the fix in JettyUtils. After thinking more the change
YarnRMClient.scala should use NM_HOST for registerApplicationMaster because
it's the right host part of YARN's NodeId.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
