Replace NodePort Service with ClusterIP (#520)

* Remove NodePort Service

* Docs update

Author: akhurana001

docs/api.md

@@ -147,7 +147,7 @@ A `DriverInfo` captures information about the driver pod and the Spark web UI ru
 | ------------- | ------------- |
 | `WebUIServiceName` | Name of the service for the Spark web UI. |
 | `WebUIPort` | Port on which the Spark web UI runs on the Node. |
-| `WebUIAddress` | Address to access the web UI from outside the cluster via the Node. |
+| `WebUIAddress` | Address to access the web UI from within the cluster. |
 | `WebUIIngressName` | Name of the ingress for the Spark web UI. |
 | `WebUIIngressAddress` | Address to access the web UI via the Ingress. |
 | `PodName` | Name of the driver pod. |

docs/quick-start-guide.md

@@ -199,10 +199,10 @@ and deleting the pods outside the operator might lead to incorrect metric values
 
 ## Driver UI Access and Ingress
 
-The operator, by default, makes the Spark UI accessible by creating a service of type `NodePort` which exposes the UI via the node running the driver.
+The operator, by default, makes the Spark UI accessible by creating a service of type `ClusterIP` which exposes the UI. This is only accessible from within the cluster.
 The operator also supports creating an Ingress for the UI. This can be turned on by setting the `ingress-url-format` command-line flag. The `ingress-url-format` should be a template like `{{$appName}}.ingress.cluster.com` and the operator will replace the `{{$appName}}` with the appropriate appName.
 
-The operator also sets both `WebUIAddress` which uses the Node's public IP as well as `WebUIIngressAddress` as part of the `DriverInfo` field of the `SparkApplication`.
+The operator also sets both `WebUIAddress` which is accessible from within the cluster as well as `WebUIIngressAddress` as part of the `DriverInfo` field of the `SparkApplication`.
 
 ## About the Mutating Admission Webhook
 

pkg/apis/sparkoperator.k8s.io/v1alpha1/types.go

@@ -422,8 +422,7 @@ const (
 // DriverInfo captures information about the driver.
 type DriverInfo struct {
 	WebUIServiceName string `json:"webUIServiceName,omitempty"`
-	// UI Details for the UI created via NodePort service.
-	// TODO: Remove this in favor of UI access via Ingress.
+	// UI Details for the UI created via ClusterIP service accessible from within the cluster.
 	WebUIPort    int32  `json:"webUIPort,omitempty"`
 	WebUIAddress string `json:"webUIAddress,omitempty"`
 	// Ingress Details if an ingress for the UI was created.

pkg/apis/sparkoperator.k8s.io/v1beta1/types.go

@@ -457,8 +457,7 @@ const (
 // DriverInfo captures information about the driver.
 type DriverInfo struct {
 	WebUIServiceName string `json:"webUIServiceName,omitempty"`
-	// UI Details for the UI created via NodePort service.
-	// TODO: Remove this in favor of UI access via Ingress.
+	// UI Details for the UI created via ClusterIP service accessible from within the cluster.
 	WebUIPort    int32  `json:"webUIPort,omitempty"`
 	WebUIAddress string `json:"webUIAddress,omitempty"`
 	// Ingress Details if an ingress for the UI was created.

pkg/controller/sparkapplication/controller.go

@@ -312,11 +312,6 @@ func (c *Controller) getAndUpdateDriverState(app *v1beta1.SparkApplication) erro
 		return nil
 	}
 
-	if driverPod.Spec.NodeName != "" {
-		if nodeIP := c.getNodeIP(driverPod.Spec.NodeName); nodeIP != "" {
-			app.Status.DriverInfo.WebUIAddress = fmt.Sprintf("%s:%d", nodeIP, app.Status.DriverInfo.WebUIPort)
-		}
-	}
 	app.Status.SparkApplicationID = getSparkApplicationID(driverPod)
 
 	if driverPod.Status.Phase == apiv1.PodSucceeded || driverPod.Status.Phase == apiv1.PodFailed {
@@ -659,7 +654,8 @@ func (c *Controller) submitSparkApplication(app *v1beta1.SparkApplication) *v1be
 		glog.Errorf("failed to create UI service for SparkApplication %s/%s: %v", app.Namespace, app.Name, err)
 	} else {
 		app.Status.DriverInfo.WebUIServiceName = service.serviceName
-		app.Status.DriverInfo.WebUIPort = service.nodePort
+		app.Status.DriverInfo.WebUIPort = service.servicePort
+		app.Status.DriverInfo.WebUIAddress = fmt.Sprintf("%s:%d", service.serviceIP, app.Status.DriverInfo.WebUIPort)
 		// Create UI Ingress if ingress-format is set.
 		if c.ingressURLFormat != "" {
 			ingress, err := createSparkUIIngress(app, *service, c.ingressURLFormat, c.kubeClient)
@@ -823,27 +819,6 @@ func (c *Controller) enqueue(obj interface{}) {
 	c.queue.AddRateLimited(key)
 }
 
-// Return IP of the node. If no External IP is found, Internal IP will be returned
-func (c *Controller) getNodeIP(nodeName string) string {
-	node, err := c.kubeClient.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
-	if err != nil {
-		glog.Errorf("failed to get node %s", nodeName)
-		return ""
-	}
-
-	for _, address := range node.Status.Addresses {
-		if address.Type == apiv1.NodeExternalIP {
-			return address.Address
-		}
-	}
-	for _, address := range node.Status.Addresses {
-		if address.Type == apiv1.NodeInternalIP {
-			return address.Address
-		}
-	}
-	return ""
-}
-
 func (c *Controller) recordSparkApplicationEvent(app *v1beta1.SparkApplication) {
 	switch app.Status.AppState.State {
 	case v1beta1.NewState:

pkg/controller/sparkapplication/sparkui.go

@@ -48,7 +48,7 @@ func getSparkUIingressURL(ingressURLFormat string, appName string) string {
 type SparkService struct {
 	serviceName string
 	servicePort int32
-	nodePort    int32
+	serviceIP   string
 }
 
 // SparkIngress encapsulates information about the driver UI ingress.
@@ -124,7 +124,7 @@ func createSparkUIService(
 				config.SparkAppNameLabel: app.Name,
 				config.SparkRoleLabel:    config.SparkDriverRole,
 			},
-			Type: apiv1.ServiceTypeNodePort,
+			Type: apiv1.ServiceTypeClusterIP,
 		},
 	}
 
@@ -136,8 +136,8 @@ func createSparkUIService(
 
 	return &SparkService{
 		serviceName: service.Name,
-		servicePort: int32(port),
-		nodePort:    service.Spec.Ports[0].NodePort,
+		servicePort: service.Spec.Ports[0].Port,
+		serviceIP:   service.Spec.ClusterIP,
 	}, nil
 }
 

pkg/controller/sparkapplication/sparkui_test.go

@@ -67,8 +67,8 @@ func TestCreateSparkUIService(t *testing.T) {
 		if !reflect.DeepEqual(test.expectedSelector, service.Spec.Selector) {
 			t.Errorf("%s: for label selector wanted %s got %s", test.name, test.expectedSelector, service.Spec.Selector)
 		}
-		if service.Spec.Type != apiv1.ServiceTypeNodePort {
-			t.Errorf("%s: for service type wanted %s got %s", test.name, apiv1.ServiceTypeNodePort, service.Spec.Type)
+		if service.Spec.Type != apiv1.ServiceTypeClusterIP {
+			t.Errorf("%s: for service type wanted %s got %s", test.name, apiv1.ServiceTypeClusterIP, service.Spec.Type)
 		}
 		if len(service.Spec.Ports) != 1 {
 			t.Errorf("%s: wanted a single port got %d ports", test.name, len(service.Spec.Ports))