dongjoon-hyun commented on PR #803: URL: https://github.com/apache/spark-kubernetes-operator/pull/803#issuecomment-5455660721
I reviewed this PR and compared it against the upstream one-liner, https://github.com/apache/spark/pull/58190. I'd prefer that we land the fix there instead. The root cause is that `BasicDriverFeatureStep` ignores `conf.proxyUser` and stamps `Utils.getCurrentUserName()` into `ENV_SPARK_USER`. Fixing that one line upstream covers the operator as a side effect, and it also fixes a case this PR cannot reach: `Utils.getCurrentUserName()` prefers the `SPARK_USER` environment variable over the UGI short name, so a plain `bin/spark-submit --proxy-user alice` launched from an environment that exports `SPARK_USER` produces the same wrong value, even though `SparkSubmit` wraps `runMain` in `proxyUser.doAs(...)`. The `doAs` wrapper never gets a chance to matter there. So this is not operator-specific. One data point in favor of the upstream fix being sufficient: `BasicExecutorFeatureStep` evaluates `Utils.getCurrentUserName()` inside the driver JVM, so executors inherit the driver container's `SPARK_USER`. Correcting the driver env alone is enough to make driver and executor identity agree. On the operator side, this is ~60 lines of post-processing plus a 35-line Javadoc on a private method, and parts of it are not quite right: - The `!replaced` fallback branch is unreachable. `BasicDriverFeatureStep` unconditionally adds `SPARK_USER` before the operator ever sees the pod, so that branch exists only for the mocked unit tests and is not covered by them either. - The early return on `environment().contains(ENV_SPARK_USER)` is justified in the test comment as preventing driver/executor identity divergence, but that is not what happens. When a user sets `spark.kubernetes.driverEnv.SPARK_USER=carol`, the container ends up with two `SPARK_USER` entries (the feature step's, then the custom one), the later one wins at runtime, and executors inherit `carol` regardless of whether we patch the first entry. The `sparkUserValue()` helper asserts `SPARK_USER` appears exactly once, which the mock-built container satisfies but a real feature-step container would not. All of this disappears once the upstream change is in. @otterc Could we close this in favor of reopening https://github.com/apache/spark/pull/58190? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
