You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/10/09 11:49:09 UTC
[GitHub] [incubator-kyuubi] pan3793 opened a new pull request, #3597: Engine should prefer to use ip for registering on K8s cluster mode
pan3793 opened a new pull request, #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
When Kyuubi runs outside of K8s, and w/o enhanced DNS infrastructure, Kyuubi can not access the Pod by using the hostname, so it blocks the user to run Spark on K8s w/ cluster mode out-of-box.
Kyuubi provided a configuration `kyuubi.frontend.connection.url.use.hostname`, turn it off could address this issue, but we can not change the default value because of https://github.com/apache/incubator-kyuubi/issues/2266
To improve user experience, we can detect if the Driver is running inside the Pod, and if yes,
change `kyuubi.frontend.connection.url.use.hostname` default value to `false`.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#discussion_r990789422
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -164,9 +164,13 @@ object SparkSQLEngine extends Logging {
val defaultCat = if (KyuubiSparkUtil.hiveClassesArePresent) "hive" else "in-memory"
_sparkConf.setIfMissing("spark.sql.catalogImplementation", defaultCat)
- kyuubiConf.setIfMissing(KyuubiConf.FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
+ kyuubiConf.setIfMissing(FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
kyuubiConf.setIfMissing(HA_ZK_CONN_RETRY_POLICY, RetryPolicies.N_TIME.toString)
+ if (Utils.isOnK8s) {
+ kyuubiConf.setIfMissing(FRONTEND_CONNECTION_URL_USE_HOSTNAME, false)
Review Comment:
make sense, updated
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3597: Engine should prefer to use ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#discussion_r990778643
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/Utils.scala:
##########
@@ -329,4 +329,6 @@ object Utils extends Logging {
*/
def getContextOrKyuubiClassLoader: ClassLoader =
Option(Thread.currentThread().getContextClassLoader).getOrElse(getKyuubiClassLoader)
+
+ def isOnK8s: Boolean = Files.exists(Paths.get("/var/run/secrets/kubernetes.io"))
Review Comment:
the solution is found at https://stackoverflow.com/a/49045575, and got confirmed by our internal K8s team. but I don't know if there is an official recommended way to do that.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3597: Engine should prefer to use ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#discussion_r990778643
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/Utils.scala:
##########
@@ -329,4 +329,6 @@ object Utils extends Logging {
*/
def getContextOrKyuubiClassLoader: ClassLoader =
Option(Thread.currentThread().getContextClassLoader).getOrElse(getKyuubiClassLoader)
+
+ def isOnK8s: Boolean = Files.exists(Paths.get("/var/run/secrets/kubernetes.io"))
Review Comment:
the solution is found at https://stackoverflow.com/a/49045575, and got confirmed by our internal K8s team. but I don't if there is an official recommended way to do that.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3597: Engine should prefer to use ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#issuecomment-1272526880
cc @zwangsheng @jiaoqingbo @cxzl25
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] Yikf commented on pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
Yikf commented on PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#issuecomment-1272708583
What happen if currently env is k8s cluster mode w/ kerberos?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#issuecomment-1272756128
> What happen if currently env is k8s cluster mode w/ kerberos?
#2266 occurs on the server side, this change only affects the engine side. that is why I say we can not change the default value GLOBALLY.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#issuecomment-1272757836
Thanks, merging to master/1.6
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 closed pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #3597: Engine should prefer ip for registering on K8s cluster mode
URL: https://github.com/apache/incubator-kyuubi/pull/3597
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cfmcgrady commented on a diff in pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on code in PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#discussion_r990781911
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -164,9 +164,13 @@ object SparkSQLEngine extends Logging {
val defaultCat = if (KyuubiSparkUtil.hiveClassesArePresent) "hive" else "in-memory"
_sparkConf.setIfMissing("spark.sql.catalogImplementation", defaultCat)
- kyuubiConf.setIfMissing(KyuubiConf.FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
+ kyuubiConf.setIfMissing(FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
kyuubiConf.setIfMissing(HA_ZK_CONN_RETRY_POLICY, RetryPolicies.N_TIME.toString)
+ if (Utils.isOnK8s) {
+ kyuubiConf.setIfMissing(FRONTEND_CONNECTION_URL_USE_HOSTNAME, false)
Review Comment:
Maybe we should point it out in the documentation?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3597: Engine should prefer ip for registering on K8s cluster mode
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3597:
URL: https://github.com/apache/incubator-kyuubi/pull/3597#issuecomment-1272534432
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3597](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0df15e7) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/f76834415a30eacbf2bfa702c7ae34ed6c814e7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f768344) will **decrease** coverage by `0.01%`.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## master #3597 +/- ##
============================================
- Coverage 51.76% 51.75% -0.02%
Complexity 13 13
============================================
Files 483 483
Lines 27020 27023 +3
Branches 3776 3777 +1
============================================
- Hits 13988 13986 -2
- Misses 11668 11672 +4
- Partials 1364 1365 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1NRTEVuZ2luZS5zY2FsYQ==) | `76.82% <33.33%> (-0.95%)` | :arrow_down: |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `75.88% <100.00%> (+0.17%)` | :arrow_up: |
| [...apache/kyuubi/engine/JpsApplicationOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvSnBzQXBwbGljYXRpb25PcGVyYXRpb24uc2NhbGE=) | `77.41% <0.00%> (-3.23%)` | :arrow_down: |
| [...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvamRiYy9KREJDTWV0YWRhdGFTdG9yZS5zY2FsYQ==) | `89.27% <0.00%> (-0.70%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `80.74% <0.00%> (-0.63%)` | :arrow_down: |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `91.17% <0.00%> (-0.30%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `73.10% <0.00%> (+0.84%)` | :arrow_up: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3597/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `80.00% <0.00%> (+2.00%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org