Skip to content
This repository has been archived by the owner on Jan 9, 2020. It is now read-only.

Optionally expose the driver UI port as NodePort #131

Merged
merged 3 commits into from
Feb 22, 2017
Merged

Optionally expose the driver UI port as NodePort #131

merged 3 commits into from
Feb 22, 2017

Conversation

kimoonkim
Copy link
Member

@kimoonkim kimoonkim commented Feb 21, 2017 •
edited
Loading

What changes were proposed in this pull request?

Addresses #129 by optionally exposing the driver Web UI as NodePort.

How was this patch tested?

Tested again my k8s cluster. I was able to look at the Web UI using my browser.

Ran the integration test successfully:

[INFO] Spark Project Kubernetes Integration Tests ......... SUCCESS [15:47 min]

@kimoonkim kimoonkim changed the title Optionally expose driver UI as a NodePort Optionally expose the driver UI port as NodePort Feb 21, 2017
@mccheah
Copy link

mccheah commented Feb 21, 2017

Also modify docs/running-on-kubernetes.md - the table at the bottom of the page should include this configuration.

@kimoonkim
Copy link
Member Author

Thanks @mccheah for taking a look. Updated docs/running-on-kubernetes.md.

@foxish
Copy link
Member

foxish commented Feb 21, 2017 •
edited
Loading

Can we work out a better scheme for setting the type of the services we create? We have a similar need in #116 as well.

ash211
ash211 reviewed Feb 22, 2017
View reviewed changes
Copy link

@ash211 ash211 left a comment

Choose a reason for hiding this comment

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

For a short-term addition this certainly gets the job done of exposing the driver UI port to outside the cluster. For longer term I think we might want to have one flag for each service's type, in addition to a flag that controls all of them at once.

I'm not sure we should block this PR on that though.

@foxish do you have any strawman suggestions for a better scheme doing this?

...urce-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/Client.scala Outdated
@@ -226,13 +226,15 @@ private[spark] class Client(
logInfo("Successfully submitted local resources and driver configuration to" +
" driver pod.")
// After submitting, adjust the service to only expose the Spark UI
val serviceType = if (sparkConf.get(EXPOSE_KUBERNETES_DRIVER_SERVICE_UI_PORT)) "NodePort"
Copy link

Choose a reason for hiding this comment

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

rename serviceType to uiServiceType

@foxish
Copy link
Member

foxish commented Feb 22, 2017

I think we might want to have one flag for each service's type, in addition to a flag that controls all of them at once.

I was thinking along these same lines. This change makes sense for the short term.

@ash211
Copy link

ash211 commented Feb 22, 2017

Did the quick rename, good to merge when the build is green

@ash211 ash211 merged commit 25a209b into apache-spark-on-k8s:k8s-support-alternate-incremental Feb 22, 2017
@ash211 ash211 mentioned this pull request Feb 22, 2017
Closed
@kimoonkim
Copy link
Member Author

Thanks for merging in this change.

@foxish, @ash211 It occurred to me that we may want to consider hierarchical config options for services that may be formatted in json or yaml, much like k8s resources. Otherwise, the flat key=value options may become messy pretty soon.

I think we might want to have one flag for each service's type, in addition to a flag that controls all of them at once.

ash211 pushed a commit that referenced this pull request Mar 8, 2017
@kimoonkim @ash211
Optionally expose the driver UI port as NodePort (#131)
cad5dd3 
* Optionally expose driver UI as a NodePort

* Update the usage doc

* Rename serviceType -> uiServiceType
@kimoonkim kimoonkim deleted the node-port-driver-ui branch July 14, 2017 17:28
foxish pushed a commit that referenced this pull request Jul 24, 2017
@kimoonkim @foxish
Optionally expose the driver UI port as NodePort (#131)
e7b3569 
* Optionally expose driver UI as a NodePort

* Update the usage doc

* Rename serviceType -> uiServiceType
ifilonenko pushed a commit to ifilonenko/spark that referenced this pull request Feb 25, 2019
@kimoonkim @ash211
Optionally expose the driver UI port as NodePort (apache-spark-on-k8s…
8ca3c26 
…#131)

* Optionally expose driver UI as a NodePort

* Update the usage doc

* Rename serviceType -> uiServiceType

(cherry picked from commit 25a209b)
ifilonenko pushed a commit to ifilonenko/spark that referenced this pull request Feb 25, 2019
@sjrand @robert3005
Fix Apache Curator version in hadoop-palantir profile (apache-spark-o…
3e7e0a1 
…n-k8s#131)
puneetloya pushed a commit to puneetloya/spark that referenced this pull request Mar 11, 2019
@kimoonkim
Optionally expose the driver UI port as NodePort (apache-spark-on-k8s…
96ea147 
…#131)

* Optionally expose driver UI as a NodePort

* Update the usage doc

* Rename serviceType -> uiServiceType
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ash211 ash211 ash211 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kimoonkim @mccheah @foxish @ash211