Skip to content

[LIVY-1071] Add driver.address.use-hostname RSC config flag - #544

Open
soumyadeeplogin wants to merge 3 commits into
apache:masterfrom
soumyadeeplogin:livy-item1-driver-hostname-flag
Open

[LIVY-1071] Add driver.address.use-hostname RSC config flag#544
soumyadeeplogin wants to merge 3 commits into
apache:masterfrom
soumyadeeplogin:livy-item1-driver-hostname-flag

Conversation

@soumyadeeplogin

@soumyadeeplogin soumyadeeplogin commented Aug 20, 2026

Copy link
Copy Markdown

What changes were proposed in this pull request?

Adds an opt-in RSCConf key, driver.address.use-hostname (default false), that makes ContextLauncher resolve the RSC driver's address from the hostname reported in RemoteDriverAddress instead of the socket's IP. Existing IP-based behavior is unchanged when the flag is left at its default.

This is useful for Kubernetes + service-mesh (Istio) deployments where pod IPs are not stable/routable but hostnames are.

Originally authored by Venugopal Allenki, extracted and rebased onto current master for upstreaming.

How was this patch tested?

Manually verified in a Kubernetes + Istio deployment with the flag enabled and disabled.

Added TestContextLauncher with unit tests covering both resolution paths (default socket-IP path, and hostname path when the flag is enabled) against a new ContextLauncher.resolveDriverAddress method extracted from RegistrationHandler.handle() for direct testability, since the handler itself is a private inner class only exercisable via a full RPC handshake. Also documented the new flag in conf/livy.conf.template.

Was this patch authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (Sonnet 5, Anthropic), used to scan/plan the extraction of local UIP patches for upstreaming and to draft/iterate on the code and tests in this PR under human review. Please refer to the ASF Generative Tooling Guidance for details.

Note on JIRA

Filed as LIVY-1071.

@soumyadeeplogin soumyadeeplogin changed the title Add driver.address.use-hostname RSC config flag [LIVY-1071] Add driver.address.use-hostname RSC config flag Aug 21, 2026
@soumyadeeplogin

Copy link
Copy Markdown
Author

Filed https://issues.apache.org/jira/browse/LIVY-1071 for this change.

@soumyadeeplogin

Copy link
Copy Markdown
Author

@gyogal @roczei could one of you approve the CI workflow run for this PR (first-time contributor gate) and take a look when you have a chance? Filed as LIVY-1071.

Adds an opt-in RSCConf key, driver.address.use-hostname (default false),
that makes ContextLauncher resolve the RSC driver's address from the
hostname reported in RemoteDriverAddress instead of the socket's IP.
Existing IP-based behavior is unchanged when the flag is left at its
default. Useful for Kubernetes + service-mesh (Istio) deployments where
pod IPs are not stable/routable but hostnames are.

Authored-by: Venugopal Allenki <vallenki@salesforce.com>
Co-authored-by: Soumyadeep Roy <soumyadeep.roy@salesforce.com>
@soumyadeeplogin
soumyadeeplogin force-pushed the livy-item1-driver-hostname-flag branch from e3796c8 to 7341e2e Compare August 25, 2026 16:06
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.82%. Comparing base (501e822) to head (7341e2e).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...main/java/org/apache/livy/rsc/ContextLauncher.java 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #544      +/-   ##
============================================
+ Coverage     68.68%   68.82%   +0.14%     
- Complexity     1218     1219       +1     
============================================
  Files           106      107       +1     
  Lines          6815     6875      +60     
  Branches        836      839       +3     
============================================
+ Hits           4681     4732      +51     
- Misses         1666     1675       +9     
  Partials        468      468              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@roczei

roczei commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

@soumyadeeplogin

Thanks for the PR! The change looks good overall. I have a few minor follow-ups, could you please address them when you have a chance?

  1. Documentation

Please add livy.rsc.driver.address.use-hostname in conf/livy.conf.template, near the other livy.rsc.jars setting.

  1. Unit test

Please add unit tests for the new flag. Two simple tests in TestSparkClient would be enough:

  • With the flag off, assert Livy uses the socket IP.
  • With the flag on, assert Livy uses the reported hostname.

For both tests, set RPC_SERVER_ADDRESS to something like spark-driver.default.svc.cluster.local, then check ContextInfo.remoteAddress.

  1. PR description — AI tooling disclosure

Please add this section to the PR description as required by the Livy PR template:

Was this patch authored or co-authored using generative AI tooling?

(If yes, include 'Generated-by: ' followed by the tool name and version. If no, write 'No'.)
Please refer to the ASF Generative Tooling Guidance for details.

soumyadeep-roy added 2 commits August 26, 2026 13:32
Codecov flagged the DRIVER_ADDRESS_USE_HOSTNAME branch in
ContextLauncher as untested when CI ran on this PR.
RegistrationHandler.handle() could not be unit-tested directly since
it depends on a live Netty ChannelHandlerContext, so the branch logic
is extracted into a new package-private static method,
ContextLauncher.resolveDriverAddress, which is directly testable.
Behavior is unchanged.
Per review feedback on apache#544, document the new
livy.rsc.driver.address.use-hostname flag next to the other
livy.rsc.* settings so operators can discover it without reading
RSCConf source.
@soumyadeeplogin

Copy link
Copy Markdown
Author

Thanks for the review, @roczei! Pushed updates addressing all three points:

1. Documentation — Added livy.rsc.driver.address.use-hostname to conf/livy.conf.template, next to livy.rsc.jars.

2. Unit test — Added TestContextLauncher covering both paths (socket-IP default, and hostname when the flag is enabled). I went with a unit test against the resolution logic itself rather than a TestSparkClient end-to-end test, for a specific reason: RSCDriver.initializeServer() explicitly nulls RPC_SERVER_ADDRESS on the driver side (see the comment there — "we need to unset this configuration since it doesn't really apply for the driver side"), and only repopulates it from spark.driver.host when spark.master starts with k8s. So setting RPC_SERVER_ADDRESS directly in a TestSparkClient config wouldn't actually reach RemoteDriverAddress.host in the non-k8s in-process test path used by the rest of that suite — and forcing spark.master to a k8s value there would make the driver try to initialize a real Kubernetes-mode SparkContext, which isn't feasible without an actual (or mocked) K8s API server.

Instead, I extracted the branch logic out of RegistrationHandler.handle() into a new package-private static method, ContextLauncher.resolveDriverAddress(RSCConf, BaseProtocol.RemoteDriverAddress, InetSocketAddress), which is directly unit-testable without touching Netty or Spark internals, and added two tests exercising both branches. Happy to also add a TestSparkClient-level test if you still want end-to-end coverage — let me know and I can look at mocking the k8s path or another approach.

3. AI tooling disclosure — Added the required section to the PR description.

@roczei

roczei commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for addressing the feedbacks, looks good from my side!

@soumyadeeplogin

Copy link
Copy Markdown
Author

Thanks @roczei for approving the PR. Could you please approve the workflows as well when you get a chance?

@roczei

roczei commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Could you please approve the workflows as well when you get a chance?

Currently I'm neither a Livy PMC member nor a committer, so I don't have the necessary permissions to run the CI.

Probably this is the actual configuration:

The default for this option is to “always require approval for external contributors”.

Source: https://infra.apache.org/github-actions-policy.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants