Add configuration for SparkUI service type (kubeflow#1100)

* Add support for sparkUI service type

* Update docs and CRD based on newer version of controller-gen

* Add protocol back to ports required fields for 1.18 compatibility. Document this step in developer guide.

docs/developer-guide.md

@@ -38,11 +38,11 @@ To update the auto-generated code, run the following command. (This step is only
 $ hack/update-codegen.sh
 ```
 
-To update the auto-generated CRD definitions, run the following command:
+To update the auto-generated CRD definitions, run the following command. After doing so, you must update the list of required fields under each `ports` field to add the `protocol` field to the list. Skipping this step will make the CRDs incompatible with Kubernetes v1.18+.
 
 ```bash
 $ GO111MODULE=off go get -u sigs.k8s.io/controller-tools/cmd/controller-gen
-$ controller-gen crd:trivialVersions=true,maxDescLen=0 paths="./pkg/apis/sparkoperator.k8s.io/v1beta2" output:crd:artifacts:config=./manifest/crds/
+$ controller-gen crd:trivialVersions=true,maxDescLen=0,crdVersions=v1beta1 paths="./pkg/apis/sparkoperator.k8s.io/v1beta2" output:crd:artifacts:config=./manifest/crds/
 ```
 
 You can verify the current auto-generated code is up to date with:

manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml

@@ -3686,6 +3686,8 @@ spec:
                     servicePort:
                       format: int32
                       type: integer
+                    serviceType:
+                      type: string
                   type: object
                 sparkVersion:
                   type: string
@@ -4340,5 +4342,5 @@ status:
   acceptedNames:
     kind: ""
     plural: ""
-  conditions: null
-  storedVersions: null
+  conditions: []
+  storedVersions: []

manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml

@@ -3672,6 +3672,8 @@ spec:
                 servicePort:
                   format: int32
                   type: integer
+                serviceType:
+                  type: string
               type: object
             sparkVersion:
               type: string
@@ -4349,5 +4351,5 @@ status:
   acceptedNames:
     kind: ""
     plural: ""
-  conditions: null
-  storedVersions: null
+  conditions: []
+  storedVersions: []

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

@@ -306,6 +306,9 @@ type SparkUIConfiguration struct {
 	// TargetPort should be the same as the one defined in spark.ui.port
 	// +optional
 	ServicePort *int32 `json:"servicePort"`
+	// ServiceType allows configuring the type of the service. Defaults to ClusterIP.
+	// +optional
+	ServiceType *apiv1.ServiceType `json:"serviceType"`
 	// IngressAnnotations is a map of key,value pairs of annotations that might be added to the ingress object. i.e. specify nginx as ingress.class
 	// +optional
 	IngressAnnotations map[string]string `json:"ingressAnnotations,omitempty"`

pkg/controller/sparkapplication/sparkapp_util.go

@@ -57,6 +57,13 @@ func getDriverPodName(app *v1beta2.SparkApplication) string {
 	return fmt.Sprintf("%s-driver", app.Name)
 }
 
+func getUIServiceType(app *v1beta2.SparkApplication) apiv1.ServiceType {
+	if app.Spec.SparkUIOptions != nil && app.Spec.SparkUIOptions.ServiceType != nil {
+		return *app.Spec.SparkUIOptions.ServiceType
+	}
+	return apiv1.ServiceTypeClusterIP
+}
+
 func getDefaultUIServiceName(app *v1beta2.SparkApplication) string {
 	return fmt.Sprintf("%s-ui-svc", app.Name)
 }

pkg/controller/sparkapplication/sparkui.go

@@ -49,6 +49,7 @@ func getSparkUIingressURL(ingressURLFormat string, appName string, appNamespace
 // SparkService encapsulates information about the driver UI service.
 type SparkService struct {
 	serviceName string
+	serviceType apiv1.ServiceType
 	servicePort int32
 	targetPort  intstr.IntOrString
 	serviceIP   string
@@ -160,7 +161,7 @@ func createSparkUIService(
 				config.SparkAppNameLabel: app.Name,
 				config.SparkRoleLabel:    config.SparkDriverRole,
 			},
-			Type: apiv1.ServiceTypeClusterIP,
+			Type: getUIServiceType(app),
 		},
 	}
 
@@ -172,6 +173,7 @@ func createSparkUIService(
 
 	return &SparkService{
 		serviceName: service.Name,
+		serviceType: service.Spec.Type,
 		servicePort: service.Spec.Ports[0].Port,
 		targetPort:  service.Spec.Ports[0].TargetPort,
 		serviceIP:   service.Spec.ClusterIP,

pkg/controller/sparkapplication/sparkui_test.go

@@ -66,8 +66,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.ServiceTypeClusterIP {
-			t.Errorf("%s: for service type wanted %s got %s", test.name, apiv1.ServiceTypeClusterIP, service.Spec.Type)
+		if service.Spec.Type != test.expectedService.serviceType {
+			t.Errorf("%s: for service type wanted %s got %s", test.name, test.expectedService.serviceType, 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))
@@ -141,12 +141,30 @@ func TestCreateSparkUIService(t *testing.T) {
 			SparkApplicationID: "foo-3",
 		},
 	}
+	var serviceTypeNodePort apiv1.ServiceType = apiv1.ServiceTypeNodePort
+	app5 := &v1beta2.SparkApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "default",
+			UID:       "foo-123",
+		},
+		Spec: v1beta2.SparkApplicationSpec{
+			SparkUIOptions: &v1beta2.SparkUIConfiguration{
+				ServiceType: &serviceTypeNodePort,
+			},
+		},
+		Status: v1beta2.SparkApplicationStatus{
+			SparkApplicationID: "foo-2",
+			ExecutionAttempts:  2,
+		},
+	}
 	testcases := []testcase{
 		{
 			name: "service with custom serviceport and serviceport and target port are same",
 			app:  app1,
 			expectedService: SparkService{
 				serviceName: fmt.Sprintf("%s-ui-svc", app1.GetName()),
+				serviceType: apiv1.ServiceTypeClusterIP,
 				servicePort: 4041,
 				targetPort: intstr.IntOrString{
 					Type:   intstr.Int,
@@ -164,6 +182,7 @@ func TestCreateSparkUIService(t *testing.T) {
 			app:  app2,
 			expectedService: SparkService{
 				serviceName: fmt.Sprintf("%s-ui-svc", app2.GetName()),
+				serviceType: apiv1.ServiceTypeClusterIP,
 				servicePort: int32(defaultPort),
 			},
 			expectedSelector: map[string]string{
@@ -177,6 +196,7 @@ func TestCreateSparkUIService(t *testing.T) {
 			app:  app4,
 			expectedService: SparkService{
 				serviceName: fmt.Sprintf("%s-ui-svc", app4.GetName()),
+				serviceType: apiv1.ServiceTypeClusterIP,
 				servicePort: 80,
 				targetPort: intstr.IntOrString{
 					Type:   intstr.Int,
@@ -189,6 +209,20 @@ func TestCreateSparkUIService(t *testing.T) {
 			},
 			expectError: false,
 		},
+		{
+			name: "service with custom servicetype",
+			app:  app5,
+			expectedService: SparkService{
+				serviceName: fmt.Sprintf("%s-ui-svc", app4.GetName()),
+				serviceType: apiv1.ServiceTypeNodePort,
+				servicePort: int32(defaultPort),
+			},
+			expectedSelector: map[string]string{
+				config.SparkAppNameLabel: "foo",
+				config.SparkRoleLabel:    config.SparkDriverRole,
+			},
+			expectError: false,
+		},
 		{
 			name:        "service with bad port configurations",
 			app:         app3,