Repository: GoogleCloudPlatform/spark-on-k8s-operator
Pull request #1763 (2023-05-25 14:14:04)

Updating webhook docs to also mention eks
As mentioned in the comment https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/issues/1708#issuecomment-1523442278, as well as several issues (e.g. https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/issues/1708, https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/issues/1725), the webhook default port change also affects AWS EKS clusters since March 2023. 

This PR is to update the docs to mention AWS EKS along with GKE for webhook port update. 

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/1763/checks?check_run_id=13758952751) 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..01dd103 100644
--- a/docs/quick-start-guide.md
+++ b/docs/quick-start-guide.md
@@ -18,7 +18,7 @@ For a more detailed guide on how to use, compose, and work with `SparkApplicatio
       - [Work Queue Metrics](#work-queue-metrics)
   - [Driver UI Access and Ingress](#driver-ui-access-and-ingress)
   - [About the Mutating Admission Webhook](#about-the-mutating-admission-webhook)
-    - [Mutating Admission Webhooks on a private GKE cluster](#mutating-admission-webhooks-on-a-private-gke-cluster)
+    - [Mutating Admission Webhooks on a private GKE or EKS cluster](#mutating-admission-webhooks-on-a-private-gke-or-eks-cluster)
 
 ## Installation
 
@@ -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.
@@ -257,9 +267,9 @@ $ kubectl apply -f manifest/spark-operator-with-webhook.yaml
 
 This will create a Deployment named `sparkoperator` and a Service named `spark-webhook` for the webhook in namespace `spark-operator`.
 
-### Mutating Admission Webhooks on a private GKE cluster
+### Mutating Admission Webhooks on a private GKE or EKS cluster
 
-If you are deploying the operator on a GKE cluster with the [Private cluster](https://cloud.google.com/kubernetes-engine/docs/how-to/private-clusters) setting enabled, and you wish to deploy the cluster with the [Mutating Admission Webhook](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/), then make sure to change the `webhookPort` to `443`. Alternatively you can choose to allow connections to the default port (8080).
+If you are deploying the operator on a GKE cluster with the [Private cluster](https://cloud.google.com/kubernetes-engine/docs/how-to/private-clusters) setting enabled, or on an enterprise AWS EKS cluster and you wish to deploy the cluster with the [Mutating Admission Webhook](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/), then make sure to change the `webhookPort` to `443`. Alternatively you can choose to allow connections to the default port (8080).
 
 > By default, firewall rules restrict your cluster master to only initiate TCP connections to your nodes on ports 443 (HTTPS) and 10250 (kubelet). For some Kubernetes features, you might need to add firewall rules to allow access on additional ports. For example, in Kubernetes 1.9 and older, kubectl top accesses heapster, which needs a firewall rule to allow TCP connections on port 8080. To grant such access, you can add firewall rules.
 [From the docs](https://cloud.google.com/kubernetes-engine/docs/how-to/private-clusters#add_firewall_rules)
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()")
+		})
+	}
+}