-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix driver host configuration to handle IPv6 addresses #2703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hi maintainers, I'm working on a fix for #2679 to properly enclose IPv6 addresses in brackets for Would it make sense to generalize or reuse this condition for setting values such as spark.driver.host, to ensure consistent IPv6 handling across the operator codebase? Any feedback or suggestions would be appreciated! Thanks! |
|
@dbbvitor: changing LGTM is restricted to collaborators In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I am afraid not. Since we cannot determine the server pod IP before creating it, the operator cannot construct the final paramater |
I think he is suggesting something like having the dynamic bracket wrapping logic in go instead of bash. Maybe the snippet he quoted could be its own function so it could be used standalone or inside the Maybe something like: func driverConfOption(...) {
//...
podIp, err := util.dynamicIPv6Brackets(POD_IP)
if err != nil {
return nil, fmt.Errorf("failed to proccess IPv6 address: %v", err)
}
args = append(args, "--conf",
fmt.Sprintf("spark.driver.host=%s", podIp)
)
} |
|
It is good to create a util function to bracket IP address as needed. But this function cannot be reused in the Spark connect case because the code must be executed in server pod, not in operator itself. |
… Spark Connect Signed-off-by: tiago.matos <tiago.matos@gympass.com>
Signed-off-by: tiago.matos <tiago.matos@gympass.com>
9c4041d to
dfdc10e
Compare
ChenYi015
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiagotxm Thanks for fixing the IPv6 issue.
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ChenYi015 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
When the driver pod IP is an IPv6 address, Spark expects the address to be enclosed in square brackets (e.g.,
[2001:db8::1]). The previous approach setspark.driver.hostunconditionally via:This fails Spark's strict hostname checks for IPv6 addresses.
Purpose of this PR
Proposed changes:
Fix
spark.driver.hostto Enclose IPv6 Addresses in Brackets to Prevent AssertionError (#2679)Change Category
Rationale
Checklist
Additional Notes
AssertionErroron IPv6 clusters by ensuring Spark receives a valid host identifier.