Repository: GoogleCloudPlatform/spark-on-k8s-operator
Pull request #1272 (2021-05-25 13:41:25)

add crds drift check between chart/ and manifest/
Hello,

This PR address the issue of crds drift between chart/ and manifest/ encountered in #1262 
I've also delete redundant element in `.github/workflows/main.yaml`

This PR comes with an open question: 
Should we keep Travis-ci, since everything that it does is also check by GitHub workflow ?


Patch:
diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 301ea68..9b9e3b5 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -29,9 +29,6 @@ jobs:
       - name: unit tests
         run: make test
 
-      - name: integration tests
-        run: make test
-
   it:
     runs-on: ubuntu-latest
     steps:
@@ -68,6 +65,9 @@ jobs:
       - name: Run chart-testing (lint)
         run: ct lint
 
+      - name: Detect CRDs drift between chart and manifest
+        run: make detect-crds-drift
+
       - name: Create kind cluster
         uses: helm/kind-action@v1.0.0
 
diff --git a/Makefile b/Makefile
index 92cc18d..7f70f84 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,9 @@ fmt-check: clean
 	@echo "running fmt check"
 	./.travis.gofmt.sh
 
+detect-crds-drift:
+	diff -q charts/spark-operator-chart/crds manifest/crds
+
 clean:
 	@echo "cleaning up caches and output"
 	go clean -cache -testcache -r -x ./... 2>&1 >/dev/null
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,