Repository: GoogleCloudPlatform/spark-on-k8s-operator
Pull request #1264 (2021-05-18 05:22:03)

Allow user to specify service annotation on Spark UI service
Currently, there's no way to specify annotations on Spark UI services. One example of how this can be useful, is that we can add annotation on Spark UI services such that it can be ignored by Istio Pilot.

google-cla[bot]: 
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: **Please visit <https://cla.developers.google.com/> to sign.**

Once you've signed (or fixed any issues), please reply here with `@googlebot I signed it!` and we'll verify it.

----

#### What to do if you already signed the CLA

##### Individual signers

*   It's possible we don't have your GitHub username or you're using a different email address on your commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).

##### Corporate signers

*   Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to [go/cla#troubleshoot](http://go/cla#troubleshoot) ([Public version](https://opensource.google/docs/cla/#troubleshoot)).
*   The email used to register you as an authorized contributor must be the email used for the Git commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).
*   The email used to register you as an authorized contributor must also be [attached to your GitHub account](https://github.com/settings/emails).
		

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2FGoogleCloudPlatform%2Fspark-on-k8s-operator%2Fpull%2F1264) for more info**.

<!-- need_sender_cla -->
khorshuheng: @googlebot I signed it!
google-cla[bot]: All (the pull request submitter and all commit authors) CLAs are signed, **but** one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project.  Please have them confirm that by leaving a comment that contains only `@googlebot I consent.` in this pull request.

*Note to project maintainer:* There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent.  In those cases, you can manually confirm consent of the commit author(s), and set the `cla` label to `yes` (if enabled on your project).

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2FGoogleCloudPlatform%2Fspark-on-k8s-operator%2Fpull%2F1264) for more info**.

<!-- need_author_consent -->
khorshuheng: @googlebot I consent.
khorshuheng: > Please bump up the chart version and resolve the conflict.

Updated to 1.1.3, conflict resolved.

Patch:
diff --git a/charts/spark-operator-chart/Chart.yaml b/charts/spark-operator-chart/Chart.yaml
index a701432..c0b0635 100644
--- a/charts/spark-operator-chart/Chart.yaml
+++ b/charts/spark-operator-chart/Chart.yaml
@@ -1,7 +1,7 @@
 apiVersion: v2
 name: spark-operator
 description: A Helm chart for Spark on Kubernetes operator
-version: 1.1.2
+version: 1.1.3
 appVersion: v1beta2-1.2.3-3.1.1
 keywords:
   - spark
diff --git a/charts/spark-operator-chart/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml b/charts/spark-operator-chart/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
index 863d984..2a5176a 100644
--- a/charts/spark-operator-chart/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
+++ b/charts/spark-operator-chart/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
@@ -3695,6 +3695,10 @@ spec:
                   type: string
                 sparkUIOptions:
                   properties:
+                    serviceAnnotations:
+                      additionalProperties:
+                        type: string
+                      type: object
                     ingressAnnotations:
                       additionalProperties:
                         type: string
diff --git a/charts/spark-operator-chart/crds/sparkoperator.k8s.io_sparkapplications.yaml b/charts/spark-operator-chart/crds/sparkoperator.k8s.io_sparkapplications.yaml
index 68abbf0..f338d03 100644
--- a/charts/spark-operator-chart/crds/sparkoperator.k8s.io_sparkapplications.yaml
+++ b/charts/spark-operator-chart/crds/sparkoperator.k8s.io_sparkapplications.yaml
@@ -3681,6 +3681,10 @@ spec:
               type: string
             sparkUIOptions:
               properties:
+                serviceAnnotations:
+                  additionalProperties:
+                    type: string
+                  type: object
                 ingressAnnotations:
                   additionalProperties:
                     type: string
diff --git a/docs/api-docs.md b/docs/api-docs.md
index 0eaad6a..f5d76ff 100644
--- a/docs/api-docs.md
+++ b/docs/api-docs.md
@@ -2958,6 +2958,18 @@ Kubernetes core/v1.ServiceType
 </tr>
 <tr>
 <td>
+<code>serviceAnnotations</code></br>
+<em>
+map[string]string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ServiceAnnotations is a map of key,value pairs of annotations that might be added to the service object.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>ingressAnnotations</code></br>
 <em>
 map[string]string
@@ -2987,5 +2999,5 @@ map[string]string
 <hr/>
 <p><em>
 Generated with <code>gen-crd-api-reference-docs</code>
-on git commit <code>e616816</code>.
+on git commit <code>24a8842</code>.
 </em></p>
diff --git a/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml b/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
index 863d984..2a5176a 100644
--- a/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
+++ b/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
@@ -3695,6 +3695,10 @@ spec:
                   type: string
                 sparkUIOptions:
                   properties:
+                    serviceAnnotations:
+                      additionalProperties:
+                        type: string
+                      type: object
                     ingressAnnotations:
                       additionalProperties:
                         type: string
diff --git a/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml b/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
index 68abbf0..f338d03 100644
--- a/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
+++ b/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
@@ -3681,6 +3681,10 @@ spec:
               type: string
             sparkUIOptions:
               properties:
+                serviceAnnotations:
+                  additionalProperties:
+                    type: string
+                  type: object
                 ingressAnnotations:
                   additionalProperties:
                     type: string
diff --git a/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go b/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
index 618e5ba..8583664 100644
--- a/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
+++ b/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
@@ -314,6 +314,9 @@ type SparkUIConfiguration struct {
 	// ServiceType allows configuring the type of the service. Defaults to ClusterIP.
 	// +optional
 	ServiceType *apiv1.ServiceType `json:"serviceType"`
+	// ServiceAnnotations is a map of key,value pairs of annotations that might be added to the service object.
+	// +optional
+	ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"`
 	// 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"`
diff --git a/pkg/controller/sparkapplication/sparkapp_util.go b/pkg/controller/sparkapplication/sparkapp_util.go
index 12afdb5..3ced4c4 100644
--- a/pkg/controller/sparkapplication/sparkapp_util.go
+++ b/pkg/controller/sparkapplication/sparkapp_util.go
@@ -80,6 +80,16 @@ func getResourceLabels(app *v1beta2.SparkApplication) map[string]string {
 	return labels
 }
 
+func getServiceAnnotations(app *v1beta2.SparkApplication) map[string]string {
+	serviceAnnotations := map[string]string{}
+	if app.Spec.SparkUIOptions != nil && app.Spec.SparkUIOptions.ServiceAnnotations != nil {
+		for key, value := range app.Spec.SparkUIOptions.ServiceAnnotations {
+			serviceAnnotations[key] = value
+		}
+	}
+	return serviceAnnotations
+}
+
 func getIngressResourceAnnotations(app *v1beta2.SparkApplication) map[string]string {
 	ingressAnnotations := map[string]string{}
 	if app.Spec.SparkUIOptions != nil && app.Spec.SparkUIOptions.IngressAnnotations != nil {
diff --git a/pkg/controller/sparkapplication/sparkui.go b/pkg/controller/sparkapplication/sparkui.go
index 80e1537..b62fc99 100644
--- a/pkg/controller/sparkapplication/sparkui.go
+++ b/pkg/controller/sparkapplication/sparkui.go
@@ -50,12 +50,13 @@ 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
-	servicePortName string
-	targetPort      intstr.IntOrString
-	serviceIP       string
+	serviceName        string
+	serviceType        apiv1.ServiceType
+	servicePort        int32
+	servicePortName    string
+	targetPort         intstr.IntOrString
+	serviceIP          string
+	serviceAnnotations map[string]string
 }
 
 // SparkIngress encapsulates information about the driver UI ingress.
@@ -169,6 +170,11 @@ func createSparkUIService(
 		},
 	}
 
+	serviceAnnotations := getServiceAnnotations(app)
+	if len(serviceAnnotations) != 0 {
+		service.ObjectMeta.Annotations = serviceAnnotations
+	}
+
 	glog.Infof("Creating a service %s for the Spark UI for application %s", service.Name, app.Name)
 	service, err = kubeClient.CoreV1().Services(app.Namespace).Create(context.TODO(), service, metav1.CreateOptions{})
 	if err != nil {
@@ -176,12 +182,13 @@ func createSparkUIService(
 	}
 
 	return &SparkService{
-		serviceName:     service.Name,
-		serviceType:     service.Spec.Type,
-		servicePort:     service.Spec.Ports[0].Port,
-		servicePortName: service.Spec.Ports[0].Name,
-		targetPort:      service.Spec.Ports[0].TargetPort,
-		serviceIP:       service.Spec.ClusterIP,
+		serviceName:        service.Name,
+		serviceType:        service.Spec.Type,
+		servicePort:        service.Spec.Ports[0].Port,
+		servicePortName:    service.Spec.Ports[0].Name,
+		targetPort:         service.Spec.Ports[0].TargetPort,
+		serviceIP:          service.Spec.ClusterIP,
+		serviceAnnotations: serviceAnnotations,
 	}, nil
 }
 


Test patch:
diff --git a/pkg/controller/sparkapplication/sparkui_test.go b/pkg/controller/sparkapplication/sparkui_test.go
index 81b92a8..a23c80e 100644
--- a/pkg/controller/sparkapplication/sparkui_test.go
+++ b/pkg/controller/sparkapplication/sparkui_test.go
@@ -80,6 +80,10 @@ func TestCreateSparkUIService(t *testing.T) {
 		if port.Name != test.expectedService.servicePortName {
 			t.Errorf("%s: unexpected port name wanted %s got %s", test.name, test.expectedService.servicePortName, port.Name)
 		}
+		serviceAnnotations := service.ObjectMeta.Annotations
+		if !reflect.DeepEqual(serviceAnnotations, test.expectedService.serviceAnnotations) {
+			t.Errorf("%s: unexpected annotations wanted %s got %s", test.name, test.expectedService.serviceAnnotations, serviceAnnotations)
+		}
 	}
 	defaultPort := defaultSparkWebUIPort
 	defaultPortName := defaultSparkWebUIPortName
@@ -180,6 +184,24 @@ func TestCreateSparkUIService(t *testing.T) {
 			SparkApplicationID: "foo-6",
 		},
 	}
+	app7 := &v1beta2.SparkApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "foo7",
+			Namespace: "default",
+			UID:       "foo-123",
+		},
+		Spec: v1beta2.SparkApplicationSpec{
+			SparkUIOptions: &v1beta2.SparkUIConfiguration{
+				ServiceAnnotations: map[string]string{
+					"key": "value",
+				},
+			},
+		},
+		Status: v1beta2.SparkApplicationStatus{
+			SparkApplicationID: "foo-7",
+			ExecutionAttempts:  1,
+		},
+	}
 	testcases := []testcase{
 		{
 			name: "service with custom serviceport and serviceport and target port are same",
@@ -264,6 +286,28 @@ func TestCreateSparkUIService(t *testing.T) {
 			},
 			expectError: false,
 		},
+		{
+			name: "service with annotation",
+			app:  app7,
+			expectedService: SparkService{
+				serviceName:     fmt.Sprintf("%s-ui-svc", app7.GetName()),
+				serviceType:     apiv1.ServiceTypeClusterIP,
+				servicePortName: defaultPortName,
+				servicePort:     defaultPort,
+				serviceAnnotations: map[string]string{
+					"key": "value",
+				},
+				targetPort: intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: int32(4041),
+				},
+			},
+			expectedSelector: map[string]string{
+				config.SparkAppNameLabel: "foo7",
+				config.SparkRoleLabel:    config.SparkDriverRole,
+			},
+			expectError: false,
+		},
 		{
 			name:        "service with bad port configurations",
 			app:         app3,