Skip to content

[SPARK-27996][WEBUI] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy#27311

Closed
ishikin wants to merge 3 commits intoapache:masterfrom
ishikin:master
Closed

[SPARK-27996][WEBUI] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy#27311
ishikin wants to merge 3 commits intoapache:masterfrom
ishikin:master

Conversation

@ishikin
Copy link

@ishikin ishikin commented Jan 21, 2020

What changes were proposed in this pull request?

This PR adds method updateProtocolFromHeader(request: HttpServletRequest, url: String): String to JettyUtils.scala and calls this method from when Spark History Server is generating a redirect response.

Why are the changes needed?

This change is needed to address bug SPARK-27996 in History Server. Please note that similar change is likely needed in Spark UI. The scope of this PR is only to address the problem in History Server.

Does this PR introduce any user-facing change?

Yes, the Spark History server deployed in Kubernetes now correctly opens the job page.

How was this patch tested?

The patch was tested by building and deploying Spark History Server to Kubernetes cluster and then observing the behavior of the History Server UI. The regression testing was not performed. I will appreciate is someone can can confirm the Spark History Server still works when deployed on regular infrastructure (e.g. not behind the reverse proxy).

@dongjoon-hyun
Copy link
Member

ok to test

@dongjoon-hyun
Copy link
Member

Thank you for making a PR against master branch.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-27996][K8S] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy #27083 [SPARK-27996][K8S] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy Jan 23, 2020
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-27996][K8S] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy [SPARK-27996][WEBUI] Send HTTP redirect using the correct protocol when Spark History server is deployed behind the reverse proxy Jan 23, 2020
@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117269 has finished for PR 27311 at commit 4896e19.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @ishikin .
The UT failure looks relevant one. Could you take a look at ApplicationCacheSuite?

org.apache.spark.deploy.history.ApplicationCacheSuite.redirect includes query params

@ishikin
Copy link
Author

ishikin commented Jan 24, 2020

Hi @dongjoon-hyun,
Sorry, didn't realize master has unit test coverage for History Server now. I have originally submitted this PR against 2.4 branch and there was none.
I have corrected the unit test for redirect and added unit test covering the function I am adding.
Thanks!

@SparkQA
Copy link

SparkQA commented Jan 24, 2020

Test build #117363 has finished for PR 27311 at commit 48f6bc5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 24, 2020

Test build #117373 has finished for PR 27311 at commit d072798.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Jan 25, 2020

Test build #117390 has finished for PR 27311 at commit d072798.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ishikin ishikin requested a review from dongjoon-hyun January 28, 2020 20:56
@ishikin
Copy link
Author

ishikin commented Jan 28, 2020

Hi @dongjoon-hyun,
Not sure what is the next step since this is my first PR for Spark.
Please let me know if anything else is needed from my side.
Thanks!

@dongjoon-hyun
Copy link
Member

@ishikin . Sorry. I also forgot this PR due to the other community work items(2.4.5 RC2 preparation and some correctness issues). I'll revisit this PR as soon as possible.
BTW, if there is a delay on your PRs, you can ping another committer, too. Every committer can review and merge.

@dongjoon-hyun
Copy link
Member

BTW, the following might be a risk at the other Spark resource managers, especially, YARN/MESOS.

The regression testing was not performed. I will appreciate is someone can can confirm the Spark History Server still works when deployed on regular infrastructure (e.g. not behind the reverse proxy).

@github-actions
Copy link

github-actions bot commented May 9, 2020

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label May 9, 2020
@github-actions github-actions bot closed this May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants