You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/01/21 12:26:49 UTC
[GitHub] [knox] iain-buclaw-sociomantic opened a new pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
iain-buclaw-sociomantic opened a new pull request #399:
URL: https://github.com/apache/knox/pull/399
## What changes were proposed in this pull request?
The Kill Application button is broken, KNOX matches and rewrites the URL twice into an invalid URL.
This change just copies what is done for `/proxy/` urls.
```
<filter name="YARNUI/yarn/outbound/apps">
<content type="*/html">
<apply path="(https?://[^/':,]+:[\d]+)?/proxy/[^']*" rule="YARNUI/yarn/outbound/apps/history"/>
<!-- snip -->
</content>
</filter>
<filter name="YARNUI/yarn/outbound/apps1">
<content type="*/html">
<apply path="/proxy/[^']*" rule="YARNUI/yarn/outbound/apps/history1"/>
<!-- snip -->
</content>
</filter>
```
It seems to work, though I'm not sure if it silently breaks other URLs just yet.
## How was this patch tested?
Tested in local production cluster. Spawned up a new spark job and killed it via the Kill Application button in YARN.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r562017634
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
That's probably why it works for me then. :-)
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
That's probably why it works for me then. :-)
Edit: There is no matching route for `YARNUI/yarn/outbound/filter/cluster` either, but it is clearly executed by KNOX as per the debug logs in the issue ticket.
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
@moresandeep going to try altering the path for the `YARNUI/yarn/outbound/apps/cluster1` then. Currently rebuilding and deploying locally.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r565929389
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -207,7 +207,7 @@
</filter>
<filter name="YARNUI/yarn/outbound/filter/cluster">
<content type="*/html">
- <apply path="(https?://[^/':,]+:[\d]+)?/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
+ <apply path="https?://[^/':,]+:[\d]+/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
Review comment:
So, the following two regexes:
```
(https?://[^/':,]+:[\d]+)?/ws/v1/cluster/apps/application
/ws/v1/.*
```
Both match the following link of the kill button in YARN.
```
/ws/v1/cluster/apps/application_1611223368889_0003/state
```
You can confirm this here https://regex101.com/r/QkGNeb/1
Because of this, both the `outbound/apps/cluster1` and `outbound/ws1` rules got applied on the link, resulting in a broken URL.
What this change does is make `https?://[^/':,]+:[\d]+` an _explicit_ match, rather than an optional one, so the `outbound/apps/cluster1` rule will now only ever be applied to links in the form of:
```
http://some-internal-domain:1234/ws/v1/cluster/apps/application
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r759023524
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -207,7 +207,7 @@
</filter>
<filter name="YARNUI/yarn/outbound/filter/cluster">
<content type="*/html">
- <apply path="(https?://[^/':,]+:[\d]+)?/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
+ <apply path="https?://[^/':,]+:[\d]+/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
Review comment:
@moresandeep @iain-buclaw-sociomantic 's explanation seems reasonable to me. Shall we merge this PR?
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r562111053
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
@moresandeep going to try altering the path for the `YARNUI/yarn/outbound/apps/cluster1` then. Currently rebuilding and deploying locally.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r562469416
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
> @moresandeep going to try altering the path for the `YARNUI/yarn/outbound/apps/cluster1` then. Currently rebuilding and deploying locally.
So I see no problems yet running with this patch. The Kill button works, and everything else seems functional (except for the history url noted in KNOX-2528/KNOX-1032).
If I understand the rules and patterns correctly, I can't imagine anything should really break as a result of this, because `/ws/v1/.*` catches and handles `/ws/v1/cluster/apps/application` correctly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r562017634
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
That's probably why it works for me then. :-)
Edit: There is no matching route for `YARNUI/yarn/outbound/filter/cluster` either, but it is clearly executed by KNOX as per the debug logs in the issue ticket.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r561965961
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
So, how is this filter called? I don't see any matching route for `YARNUI/yarn/outbound/filter/cluster1` in service.xml
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r565549485
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -207,7 +207,7 @@
</filter>
<filter name="YARNUI/yarn/outbound/filter/cluster">
<content type="*/html">
- <apply path="(https?://[^/':,]+:[\d]+)?/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
+ <apply path="https?://[^/':,]+:[\d]+/ws/v1/cluster/apps/application" rule="YARNUI/yarn/outbound/apps/cluster1"/>
Review comment:
My regex skills are rusty :( can you explain briefly what changed here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r561965961
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
So, how is this filter called? I don't see any matching route for `YARNUI/yarn/outbound/filter/cluster1` in service.xml
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 merged pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #399:
URL: https://github.com/apache/knox/pull/399
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] iain-buclaw-sociomantic commented on a change in pull request #399: KNOX-2531 Kill Application button in YARN does not work through KNOX
Posted by GitBox <gi...@apache.org>.
iain-buclaw-sociomantic commented on a change in pull request #399:
URL: https://github.com/apache/knox/pull/399#discussion_r562017634
##########
File path: gateway-service-definitions/src/main/resources/services/yarnui/2.7.0/rewrite.xml
##########
@@ -219,6 +218,11 @@
<apply path="https?://[^/':,]+:[\d][^']*" rule="YARNUI/yarn/outbound/node2"/>
</content>
</filter>
+<filter name="YARNUI/yarn/outbound/filter/cluster1">
Review comment:
That's probably why it works for me then. :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org