Repository: GoogleCloudPlatform/spark-on-k8s-operator
Pull request #1783 (2023-06-27 10:03:31)

Link to helm docs fixed

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

View this [failed invocation](https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/pull/1783/checks?check_run_id=14584843357) of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Patch:
diff --git a/charts/spark-operator-chart/Chart.yaml b/charts/spark-operator-chart/Chart.yaml
index da9a2e4..437804b 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.27
+version: 1.1.28
 appVersion: v1beta2-1.3.8-3.1.1
 keywords:
   - spark
diff --git a/charts/spark-operator-chart/templates/deployment.yaml b/charts/spark-operator-chart/templates/deployment.yaml
index 127a2dd..9deacae 100644
--- a/charts/spark-operator-chart/templates/deployment.yaml
+++ b/charts/spark-operator-chart/templates/deployment.yaml
@@ -47,6 +47,8 @@ spec:
       - name: {{ .Chart.Name }}
         image: {{ .Values.image.repository }}:{{ default .Chart.AppVersion .Values.image.tag }}
         imagePullPolicy: {{ .Values.image.pullPolicy }}
+        envFrom:
+          {{- toYaml .Values.envFrom | nindent 10 }}
         securityContext:
           {{- toYaml .Values.securityContext | nindent 10 }}
         {{- if .Values.metrics.enable }}
@@ -98,6 +100,9 @@ spec:
         {{- with .Values.volumeMounts }}
         {{- toYaml . | nindent 10 }}
         {{- end }}
+      {{- with .Values.sidecars }}
+        {{- toYaml . | nindent 6 }}
+      {{- end }}
       {{- if or .Values.webhook.enable (ne (len .Values.volumes) 0 ) }}
       volumes:
       {{- end }}
diff --git a/charts/spark-operator-chart/values.yaml b/charts/spark-operator-chart/values.yaml
index 5007b7e..c7d672f 100644
--- a/charts/spark-operator-chart/values.yaml
+++ b/charts/spark-operator-chart/values.yaml
@@ -71,12 +71,18 @@ ingressUrlFormat: ""
 # -- Set higher levels for more verbose logging
 logLevel: 2
 
+# -- Pod environment variable sources
+envFrom: []
+
 # podSecurityContext -- Pod security context
 podSecurityContext: {}
 
 # securityContext -- Operator container security context
 securityContext: {}
 
+# sidecars -- Sidecar containers
+sidecars: []
+
 # volumes - Operator volumes
 volumes: []
 
diff --git a/docs/quick-start-guide.md b/docs/quick-start-guide.md
index 22e8cbb..b473e35 100644
--- a/docs/quick-start-guide.md
+++ b/docs/quick-start-guide.md
@@ -162,7 +162,7 @@ To upgrade the the operator, e.g., to use a newer version container image with a
 $ helm upgrade <YOUR-HELM-RELEASE-NAME> --set image.repository=org/image --set image.tag=newTag
 ```
 
-Refer to the Helm [documentation](https://docs.helm.sh/helm/#helm-upgrade) for more details on `helm upgrade`.
+Refer to the Helm [documentation](https://helm.sh/docs/helm/helm_upgrade/) for more details on `helm upgrade`.
 
 ## About the Spark Job Namespace
 
@@ -238,10 +238,20 @@ 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 `ClusterIP` which exposes the UI. This is only accessible from within the cluster.
+
 The operator also supports creating an optional 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_suffix}/{{$appNamespace}}/{{$appName}}`. The `{ingress_suffix}` should be replaced by the user to indicate the cluster's ingress url and the operator will replace the `{{$appName}}` & `{{$appNamespace}}` with the appropriate value. Please note that Ingress support requires that cluster's ingress url routing is correctly set-up. For e.g. if the `ingress-url-format` is `{{$appName}}.ingress.cluster.com`, it requires that anything `*ingress.cluster.com` should be routed to the ingress-controller on the K8s cluster.
 
 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`.
 
+The operator generates ingress resources intended for use with the [Ingress NGINX Controller](https://kubernetes.github.io/ingress-nginx/). Include this in your application spec for the controller to ensure it recognizes the ingress and provides appropriate routes to your Spark UI.
+
+```yaml
+spec:
+  sparkUIOptions:
+    ingressAnnotations:
+        kubernetes.io/ingress.class: nginx
+```
+
 ## About the Mutating Admission Webhook
 
 The Kubernetes Operator for Apache Spark comes with an optional mutating admission webhook for customizing Spark driver and executor pods based on the specification in `SparkApplication` objects, e.g., mounting user-specified ConfigMaps and volumes, and setting pod affinity/anti-affinity, and adding tolerations.
diff --git a/docs/who-is-using.md b/docs/who-is-using.md
index f0c5759..c9c2fd4 100644
--- a/docs/who-is-using.md
+++ b/docs/who-is-using.md
@@ -41,3 +41,4 @@
 | [C2FO](https://www.c2fo.com/) | @vanhoale | Production | Data Platform / Data Infrastructure |
 | [Kognita](https://kognita.com.br/) | @andreclaudino | Production | MLOps, Data Platform / Data Infrastructure, ML/AI |
 | [Molex](https://www.molex.com/) | @AshishPushpSingh | Evaluation/Production | Data Platform |
+| [Roblox](https://www.roblox.com/) | @matschaffer-roblox | Evaluation | Data Infrastructure |
diff --git a/pkg/controller/sparkapplication/submission.go b/pkg/controller/sparkapplication/submission.go
index b8e4e1b..98bb882 100644
--- a/pkg/controller/sparkapplication/submission.go
+++ b/pkg/controller/sparkapplication/submission.go
@@ -207,6 +207,10 @@ func getMasterURL() (string, error) {
 	if kubernetesServicePort == "" {
 		return "", fmt.Errorf("environment variable %s is not found", kubernetesServicePortEnvVar)
 	}
+	// check if the host is IPv6 address
+	if strings.Contains(kubernetesServiceHost, ":") && !strings.HasPrefix(kubernetesServiceHost, "[") {
+		return fmt.Sprintf("k8s://https://[%s]:%s", kubernetesServiceHost, kubernetesServicePort), nil
+	}
 	return fmt.Sprintf("k8s://https://%s:%s", kubernetesServiceHost, kubernetesServicePort), nil
 }
 


Test patch:
diff --git a/pkg/controller/sparkapplication/submission_test.go b/pkg/controller/sparkapplication/submission_test.go
index 9f32bbb..20e247a 100644
--- a/pkg/controller/sparkapplication/submission_test.go
+++ b/pkg/controller/sparkapplication/submission_test.go
@@ -583,3 +583,54 @@ func TestProxyUserArg(t *testing.T) {
 	assert.Equal(t, "--proxy-user", args[4])
 	assert.Equal(t, "foo", args[5])
 }
+
+func Test_getMasterURL(t *testing.T) {
+	setEnv := func(host string, port string) {
+		if err := os.Setenv(kubernetesServiceHostEnvVar, host); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Setenv(kubernetesServicePortEnvVar, port); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	tests := []struct {
+		name    string
+		host    string
+		port    string
+		want    string
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name:    "should return a valid master url when IPv4 address is used",
+			host:    "localhost",
+			port:    "6443",
+			want:    "k8s://https://localhost:6443",
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "should return a valid master url when IPv6 address is used",
+			host:    "::1",
+			port:    "6443",
+			want:    "k8s://https://[::1]:6443",
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "should throw an error when the host is empty",
+			host:    "",
+			port:    "6443",
+			want:    "",
+			wantErr: assert.Error,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			setEnv(tt.host, tt.port)
+			got, err := getMasterURL()
+			if !tt.wantErr(t, err, fmt.Sprintf("getMasterURL()")) {
+				return
+			}
+			assert.Equalf(t, tt.want, got, "getMasterURL()")
+		})
+	}
+}