Skip to content

Commit 8dd8db4

Browse files
Tom-NewtonTom-Newton
authored
Make default ingress tls and annotations congurable in the helm config (#2513)
* Initial attempt at passing the info through the CLI Signed-off-by: Thomas Newton <[email protected]> * Working getting ingressTLS config from helm yaml into a `[]networkingv1.IngressTLS` Signed-off-by: Thomas Newton <[email protected]> * Vaguely correct adding TLS options to UI ingress Signed-off-by: Thomas Newton <[email protected]> * First test passing Signed-off-by: Thomas Newton <[email protected]> * Pass through argument where I had forgotten Signed-off-by: Thomas Newton <[email protected]> * Implement IngressAnnotations Signed-off-by: Thomas Newton <[email protected]> * Add "default" to some variable names Signed-off-by: Thomas Newton <[email protected]> * Update helm and CLI parsing, including adding annotations Signed-off-by: Thomas Newton <[email protected]> * Sufficient unit tests Signed-off-by: Thomas Newton <[email protected]> * Tests and documentation for helm Signed-off-by: Thomas Newton <[email protected]> * Minor adjustments to test strings Signed-off-by: Thomas Newton <[email protected]> * PR comments Signed-off-by: Thomas Newton <[email protected]> * Fix rebase Signed-off-by: Thomas Newton <[email protected]> * Avoid manually constructing the expected ingress name in tests Signed-off-by: Thomas Newton <[email protected]> * Quote and rename on the helm side Signed-off-by: Thomas Newton <[email protected]> * Avoid using pointers Signed-off-by: Thomas Newton <[email protected]> * More renaming to remove "default" Signed-off-by: Thomas Newton <[email protected]> * Revert helm quote on json strings Signed-off-by: Thomas Newton <[email protected]> * Tidy imports Signed-off-by: Thomas Newton <[email protected]> * Fix tests after #2554 moved the ingress creation to be on submitted spark applications Signed-off-by: Thomas Newton <[email protected]> * Re-generate helm docs Signed-off-by: Thomas Newton <[email protected]> --------- Signed-off-by: Thomas Newton <[email protected]>
1 parent 718e3a0 commit 8dd8db4

File tree

9 files changed

+294
-19
lines changed

9 files changed

+294
-19
lines changed

‎charts/spark-operator-chart/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ See [helm uninstall](https://helm.sh/docs/helm/helm_uninstall) for command docum
9393
| controller.uiIngress.enable | bool | `false` | Specifies whether to create ingress for Spark web UI. `controller.uiService.enable` must be `true` to enable ingress. |
9494
| controller.uiIngress.urlFormat | string | `""` | Ingress URL format. Required if `controller.uiIngress.enable` is true. |
9595
| controller.uiIngress.ingressClassName | string | `""` | Optionally set the ingressClassName. |
96+
| controller.uiIngress.tls | list | `[]` | Optionally set default TLS configuration for the Spark UI's ingress. `ingressTLS` in the SparkApplication spec overrides this. |
97+
| controller.uiIngress.annotations | object | `{}` | Optionally set default ingress annotations for the Spark UI's ingress. `ingressAnnotations` in the SparkApplication spec overrides this. |
9698
| controller.batchScheduler.enable | bool | `false` | Specifies whether to enable batch scheduler for spark jobs scheduling. If enabled, users can specify batch scheduler name in spark application. |
9799
| controller.batchScheduler.kubeSchedulerNames | list | `[]` | Specifies a list of kube-scheduler names for scheduling Spark pods. |
98100
| controller.batchScheduler.default | string | `""` | Default batch scheduler to be used if not specified by the user. If specified, this value must be either "volcano" or "yunikorn". Specifying any other value will cause the controller to error on startup. |

‎charts/spark-operator-chart/templates/controller/deployment.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ spec:
7272
{{- with .Values.controller.uiIngress.ingressClassName }}
7373
- --ingress-class-name={{ . }}
7474
{{- end }}
75+
{{- with .Values.controller.uiIngress.tls }}
76+
- --ingress-tls={{ . | toJson }}
77+
{{- end }}
78+
{{- with .Values.controller.uiIngress.annotations }}
79+
- --ingress-annotations={{ . | toJson }}
80+
{{- end }}
7581
{{- end }}
7682
{{- if .Values.controller.batchScheduler.enable }}
7783
- --enable-batch-scheduler=true

‎charts/spark-operator-chart/tests/controller/deployment_test.yaml‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,37 @@ tests:
184184
path: spec.template.spec.containers[?(@.name=="spark-operator-controller")].args
185185
content: --ingress-class-name=nginx
186186

187+
- it: Should contain `--ingress-tls` arg if `controller.uiIngress.enable` is set to `true` and `controller.uiIngress.tls` is set
188+
set:
189+
controller:
190+
uiService:
191+
enable: true
192+
uiIngress:
193+
enable: true
194+
tls:
195+
- hosts:
196+
- "*.test.com"
197+
secretName: test-secret
198+
asserts:
199+
- contains:
200+
path: spec.template.spec.containers[?(@.name=="spark-operator-controller")].args
201+
content: '--ingress-tls=[{"hosts":["*.test.com"],"secretName":"test-secret"}]'
202+
203+
- it: Should contain `--ingress-annotations` arg if `controller.uiIngress.enable` is set to `true` and `controller.uiIngress.annotations` is set
204+
set:
205+
controller:
206+
uiService:
207+
enable: true
208+
uiIngress:
209+
enable: true
210+
annotations:
211+
cert-manager.io/cluster-issuer: "letsencrypt"
212+
kubernetes.io/ingress.class: nginx
213+
asserts:
214+
- contains:
215+
path: spec.template.spec.containers[?(@.name=="spark-operator-controller")].args
216+
content: '--ingress-annotations={"cert-manager.io/cluster-issuer":"letsencrypt","kubernetes.io/ingress.class":"nginx"}'
217+
187218
- it: Should contain `--enable-batch-scheduler` arg if `controller.batchScheduler.enable` is `true`
188219
set:
189220
controller:
@@ -246,7 +277,7 @@ tests:
246277
- contains:
247278
path: spec.template.spec.containers[?(@.name=="spark-operator-controller")].args
248279
content: --leader-election-lock-namespace=spark-operator
249-
280+
250281
- it: Should disable leader election if `controller.leaderElection.enable` is set to `false`
251282
set:
252283
controller:

‎charts/spark-operator-chart/values.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,15 @@ controller:
7474
urlFormat: ""
7575
# -- Optionally set the ingressClassName.
7676
ingressClassName: ""
77+
# -- Optionally set default TLS configuration for the Spark UI's ingress. `ingressTLS` in the SparkApplication spec overrides this.
78+
tls: []
79+
# - hosts:
80+
# - "*.example.com"
81+
# secretName: "example-secret"
82+
# -- Optionally set default ingress annotations for the Spark UI's ingress. `ingressAnnotations` in the SparkApplication spec overrides this.
83+
annotations: {}
84+
# key1: value1
85+
# key2: value2
7786

7887
batchScheduler:
7988
# -- Specifies whether to enable batch scheduler for spark jobs scheduling.

‎cmd/operator/controller/start.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ package controller
1818

1919
import (
2020
"crypto/tls"
21+
"encoding/json"
2122
"flag"
23+
"fmt"
2224
"os"
2325
"slices"
2426
"time"
@@ -33,6 +35,7 @@ import (
3335
"go.uber.org/zap/zapcore"
3436
"golang.org/x/time/rate"
3537
corev1 "k8s.io/api/core/v1"
38+
networkingv1 "k8s.io/api/networking/v1"
3639
"k8s.io/apimachinery/pkg/labels"
3740
"k8s.io/apimachinery/pkg/runtime"
3841
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -87,9 +90,11 @@ var (
8790
defaultBatchScheduler string
8891

8992
// Spark web UI service and ingress
90-
enableUIService bool
91-
ingressClassName string
92-
ingressURLFormat string
93+
enableUIService bool
94+
ingressClassName string
95+
ingressURLFormat string
96+
ingressTLS []networkingv1.IngressTLS
97+
ingressAnnotations map[string]string
9398

9499
// Leader election
95100
enableLeaderElection bool
@@ -126,11 +131,25 @@ func init() {
126131
}
127132

128133
func NewStartCommand() *cobra.Command {
134+
var ingressTLSstring string
135+
var ingressAnnotationsString string
129136
var command = &cobra.Command{
130137
Use: "start",
131138
Short: "Start controller and webhook",
132-
PreRun: func(_ *cobra.Command, args []string) {
139+
PreRunE: func(_ *cobra.Command, args []string) error {
133140
development = viper.GetBool("development")
141+
142+
if ingressTLSstring != "" {
143+
if err := json.Unmarshal([]byte(ingressTLSstring), &ingressTLS); err != nil {
144+
return fmt.Errorf("failed parsing ingress-tls JSON string from CLI: %v", err)
145+
}
146+
}
147+
if ingressAnnotationsString != "" {
148+
if err := json.Unmarshal([]byte(ingressAnnotationsString), &ingressAnnotations); err != nil {
149+
return fmt.Errorf("failed parsing ingress-annotations JSON string from CLI: %v", err)
150+
}
151+
}
152+
return nil
134153
},
135154
Run: func(_ *cobra.Command, args []string) {
136155
sparkoperator.PrintVersion(false)
@@ -154,6 +173,8 @@ func NewStartCommand() *cobra.Command {
154173
command.Flags().BoolVar(&enableUIService, "enable-ui-service", true, "Enable Spark Web UI service.")
155174
command.Flags().StringVar(&ingressClassName, "ingress-class-name", "", "Set ingressClassName for ingress resources created.")
156175
command.Flags().StringVar(&ingressURLFormat, "ingress-url-format", "", "Ingress URL format.")
176+
command.Flags().StringVar(&ingressTLSstring, "ingress-tls", "", "JSON format string for the default TLS config on the Spark UI ingresses. e.g. '[{\"hosts\":[\"*.example.com\"],\"secretName\":\"example-secret\"}]'. `ingressTLS` in the SparkApplication spec will override this value.")
177+
command.Flags().StringVar(&ingressAnnotationsString, "ingress-annotations", "", "JSON format string for the default ingress annotations for the Spark UI ingresses. e.g. '[{\"cert-manager.io/cluster-issuer\": \"letsencrypt\"}]'. `ingressAnnotations` in the SparkApplication spec will override this value.")
157178

158179
command.Flags().BoolVar(&enableLeaderElection, "leader-election", false, "Enable leader election for controller manager. "+
159180
"Enabling this will ensure there is only one active controller manager.")
@@ -403,6 +424,8 @@ func newSparkApplicationReconcilerOptions() sparkapplication.Options {
403424
EnableUIService: enableUIService,
404425
IngressClassName: ingressClassName,
405426
IngressURLFormat: ingressURLFormat,
427+
IngressTLS: ingressTLS,
428+
IngressAnnotations: ingressAnnotations,
406429
DefaultBatchScheduler: defaultBatchScheduler,
407430
DriverPodCreationGracePeriod: driverPodCreationGracePeriod,
408431
SparkApplicationMetrics: sparkApplicationMetrics,

‎internal/controller/sparkapplication/controller.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ type Options struct {
6161
EnableUIService bool
6262
IngressClassName string
6363
IngressURLFormat string
64+
IngressTLS []networkingv1.IngressTLS
65+
IngressAnnotations map[string]string
6466
DefaultBatchScheduler string
6567

6668
DriverPodCreationGracePeriod time.Duration
@@ -323,7 +325,7 @@ func (r *Reconciler) reconcileSubmittedSparkApplication(ctx context.Context, req
323325
app.Spec.SparkConf[common.SparkUIProxyBase] = ingressURL.Path
324326
app.Spec.SparkConf[common.SparkUIProxyRedirectURI] = "/"
325327
}
326-
ingress, err := r.createWebUIIngress(app, *service, ingressURL, r.options.IngressClassName)
328+
ingress, err := r.createWebUIIngress(app, *service, ingressURL, r.options.IngressClassName, r.options.IngressTLS, r.options.IngressAnnotations)
327329
if err != nil {
328330
return fmt.Errorf("failed to create web UI ingress: %v", err)
329331
}

0 commit comments

Comments
 (0)