diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 815c1442..96debfcb 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -304,3 +304,27 @@ jobs: - name: Validate helm chart linting run: | helm lint --strict build-tools/helm/spark-kubernetes-operator + - name: Validate deprecated helm values are still honored + run: | + helm template spark build-tools/helm/spark-kubernetes-operator \ + --set operatorDeployment.networkPolicy.enable=true \ + | grep -q 'kind: NetworkPolicy' + helm template spark build-tools/helm/spark-kubernetes-operator \ + --set operatorConfiguration.dynamicConfig.enable=true \ + | grep -q 'spark.kubernetes.operator.dynamicConfig.enabled=true' + - name: Validate helm values resolve to disabled + run: | + if helm template spark build-tools/helm/spark-kubernetes-operator \ + | grep -q 'kind: NetworkPolicy'; then + echo "NetworkPolicy rendered with the default values"; exit 1 + fi + if helm template spark build-tools/helm/spark-kubernetes-operator \ + --set operatorDeployment.networkPolicy.enable=false \ + | grep -q 'kind: NetworkPolicy'; then + echo "NetworkPolicy rendered with enable=false"; exit 1 + fi + if helm template spark build-tools/helm/spark-kubernetes-operator \ + --set operatorConfiguration.dynamicConfig.enable=false \ + | grep -q 'spark.kubernetes.operator.dynamicConfig.enabled=true'; then + echo "dynamicConfig enabled with enable=false"; exit 1 + fi diff --git a/build-tools/helm/spark-kubernetes-operator/templates/_helpers.tpl b/build-tools/helm/spark-kubernetes-operator/templates/_helpers.tpl index 94452ca9..9bf397cd 100644 --- a/build-tools/helm/spark-kubernetes-operator/templates/_helpers.tpl +++ b/build-tools/helm/spark-kubernetes-operator/templates/_helpers.tpl @@ -106,6 +106,26 @@ List of Spark workload namespaces. If not provied in values, use the same namesp {{- end }} {{- end }} +{{/* +Whether the operator pod NetworkPolicy is enabled. The legacy key +{operatorDeployment.networkPolicy.enable} is deprecated but still honored: the feature is +enabled when either key is true. +*/}} +{{- define "spark-operator.networkPolicy.enabled" -}} +{{- $np := .Values.operatorDeployment.networkPolicy -}} +{{- if or $np.enabled $np.enable }}true{{ else }}false{{ end -}} +{{- end }} + +{{/* +Whether dynamic config (hot properties loading) is enabled. The legacy key +{operatorConfiguration.dynamicConfig.enable} is deprecated but still honored: the feature is +enabled when either key is true. +*/}} +{{- define "spark-operator.dynamicConfig.enabled" -}} +{{- $dc := .Values.operatorConfiguration.dynamicConfig -}} +{{- if or $dc.enabled $dc.enable }}true{{ else }}false{{ end -}} +{{- end }} + {{/* Default property overrides */}} @@ -113,7 +133,7 @@ Default property overrides # Runtime resolved properties spark.kubernetes.operator.namespace={{ .Release.Namespace }} spark.kubernetes.operator.name={{- include "spark-operator.name" . }} -spark.kubernetes.operator.dynamicConfig.enabled={{ .Values.operatorConfiguration.dynamicConfig.enable }} +spark.kubernetes.operator.dynamicConfig.enabled={{ include "spark-operator.dynamicConfig.enabled" . }} spark.kubernetes.operator.dynamicConfig.source={{ .Values.operatorConfiguration.dynamicConfig.source }} spark.kubernetes.operator.metrics.port={{ include "spark-operator.metricsPort" . }} spark.kubernetes.operator.health.probePort={{ include "spark-operator.probePort" . }} diff --git a/build-tools/helm/spark-kubernetes-operator/templates/network-policy.yaml b/build-tools/helm/spark-kubernetes-operator/templates/network-policy.yaml index cf557ca3..74a79e96 100644 --- a/build-tools/helm/spark-kubernetes-operator/templates/network-policy.yaml +++ b/build-tools/helm/spark-kubernetes-operator/templates/network-policy.yaml @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -{{- if .Values.operatorDeployment.networkPolicy.enable }} +{{- if eq (include "spark-operator.networkPolicy.enabled" .) "true" }} --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy diff --git a/build-tools/helm/spark-kubernetes-operator/templates/operator-rbac.yaml b/build-tools/helm/spark-kubernetes-operator/templates/operator-rbac.yaml index ec433579..50d0511a 100644 --- a/build-tools/helm/spark-kubernetes-operator/templates/operator-rbac.yaml +++ b/build-tools/helm/spark-kubernetes-operator/templates/operator-rbac.yaml @@ -173,7 +173,7 @@ metadata: {{- template "spark-operator.operatorRbacRules" $ }} --- {{- end }} -{{- if and .Values.operatorConfiguration.dynamicConfig.enable (eq .Values.operatorConfiguration.dynamicConfig.source "configMap") }} +{{- if and (eq (include "spark-operator.dynamicConfig.enabled" .) "true") (eq .Values.operatorConfiguration.dynamicConfig.source "configMap") }} apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: diff --git a/build-tools/helm/spark-kubernetes-operator/templates/spark-operator.yaml b/build-tools/helm/spark-kubernetes-operator/templates/spark-operator.yaml index 046464ff..a49a88a8 100644 --- a/build-tools/helm/spark-kubernetes-operator/templates/spark-operator.yaml +++ b/build-tools/helm/spark-kubernetes-operator/templates/spark-operator.yaml @@ -162,7 +162,7 @@ spec: mountPath: /opt/spark-operator/logs - name: tmp-volume mountPath: /tmp - {{- if and .Values.operatorConfiguration.dynamicConfig.enable (eq .Values.operatorConfiguration.dynamicConfig.source "file") }} + {{- if and (eq (include "spark-operator.dynamicConfig.enabled" .) "true") (eq .Values.operatorConfiguration.dynamicConfig.source "file") }} - name: spark-operator-dynamic-config-volume mountPath: /opt/spark-operator/dynamic-conf readOnly: true @@ -191,7 +191,7 @@ spec: emptyDir: { } - name: tmp-volume emptyDir: { } - {{- if and .Values.operatorConfiguration.dynamicConfig.enable (eq .Values.operatorConfiguration.dynamicConfig.source "file") }} + {{- if and (eq (include "spark-operator.dynamicConfig.enabled" .) "true") (eq .Values.operatorConfiguration.dynamicConfig.source "file") }} - name: spark-operator-dynamic-config-volume configMap: name: spark-kubernetes-operator-dynamic-configuration diff --git a/build-tools/helm/spark-kubernetes-operator/templates/tests/test-network-policy.yaml b/build-tools/helm/spark-kubernetes-operator/templates/tests/test-network-policy.yaml index 9ba24b8b..cfbf7844 100644 --- a/build-tools/helm/spark-kubernetes-operator/templates/tests/test-network-policy.yaml +++ b/build-tools/helm/spark-kubernetes-operator/templates/tests/test-network-policy.yaml @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -{{- if .Values.operatorDeployment.networkPolicy.enable }} +{{- if eq (include "spark-operator.networkPolicy.enabled" .) "true" }} apiVersion: v1 kind: Pod metadata: diff --git a/build-tools/helm/spark-kubernetes-operator/values.schema.json b/build-tools/helm/spark-kubernetes-operator/values.schema.json index 9d85fb8e..60fa41fa 100644 --- a/build-tools/helm/spark-kubernetes-operator/values.schema.json +++ b/build-tools/helm/spark-kubernetes-operator/values.schema.json @@ -396,13 +396,17 @@ "type": "object", "description": "NetworkPolicy for the operator pod", "required": [ - "enable" + "enabled" ], "properties": { - "enable": { + "enabled": { "type": "boolean", "description": "Whether to create a NetworkPolicy for the operator pod" }, + "enable": { + "type": "boolean", + "description": "Deprecated, use 'enabled' instead; still honored (the NetworkPolicy is created when either key is true) and will be removed in chart 2.0.0 (SPARK-59533)" + }, "metricsIngress": { "type": ["array", "null"], "description": "List of NetworkPolicyPeer(s) allowed to reach the metrics port; when empty, ingress to the metrics port is denied", @@ -818,16 +822,20 @@ "type": "object", "description": "Dynamic configuration", "required": [ - "enable", + "enabled", "create", "annotations", "data" ], "properties": { - "enable": { + "enabled": { "type": "boolean", "description": "Enable dynamic configuration" }, + "enable": { + "type": "boolean", + "description": "Deprecated, use 'enabled' instead; still honored (dynamic configuration is enabled when either key is true) and will be removed in chart 2.0.0 (SPARK-59533)" + }, "create": { "type": "boolean", "description": "Create ConfigMap for dynamic configuration" diff --git a/build-tools/helm/spark-kubernetes-operator/values.yaml b/build-tools/helm/spark-kubernetes-operator/values.yaml index e33fb4dd..e9fdb472 100644 --- a/build-tools/helm/spark-kubernetes-operator/values.yaml +++ b/build-tools/helm/spark-kubernetes-operator/values.yaml @@ -94,7 +94,9 @@ operatorDeployment: # operator pod is denied. Note that this requires a CNI plugin with NetworkPolicy support, # and that egress traffic is not restricted. networkPolicy: - enable: false + # The legacy key `enable` is deprecated and will be removed in chart 2.0.0 (SPARK-59533). + # It is still honored: the NetworkPolicy is created when either `enabled` or `enable` is true. + enabled: false # List of NetworkPolicyPeer(s) allowed to reach the metrics port, e.g. the Prometheus # scraper. When empty, ingress to the metrics port is denied. metricsIngress: [ ] @@ -218,8 +220,10 @@ operatorConfiguration: metrics.properties: |+ # Metrics Properties Overrides dynamicConfig: - # Enable this for hot properties loading. - enable: false + # Enable this for hot properties loading. The legacy key `enable` is deprecated and will be + # removed in chart 2.0.0 (SPARK-59533). It is still honored: hot properties loading is + # enabled when either `enabled` or `enable` is true. + enabled: false # Source of the dynamic config overrides when enabled. Supported values: # configMap - (default) watch a ConfigMap via a Kubernetes informer. Requires the operator # to have RBAC to read ConfigMaps (created by this chart). diff --git a/docs/operations.md b/docs/operations.md index 4697d789..66dd7e1b 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -133,7 +133,8 @@ following table: | operatorConfiguration.spark-operator.properties | The default operator configuration. | | | operatorConfiguration.metrics.properties | The default operator metrics (sink) configuration. | | | operatorConfiguration.dynamicConfig.create | If set to true, a config map would be created & watched by operator as source of truth for hot properties loading. | false | -| operatorConfiguration.dynamicConfig.enable | If set to true, operator would honor the created config map as source of truth for hot properties loading. | false | +| operatorConfiguration.dynamicConfig.enabled | If set to true, operator would honor the created config map as source of truth for hot properties loading. | false | +| operatorConfiguration.dynamicConfig.enable | Deprecated, use `operatorConfiguration.dynamicConfig.enabled`. Still honored: `enable: true` wins over `enabled: false`. Removed in chart 2.0.0 (SPARK-59533). | | | operatorConfiguration.dynamicConfig.annotations | Annotations to be applied for the dynamicConfig resources. | `"helm.sh/resource-policy": keep` | | operatorConfiguration.dynamicConfig.data | Data field (key-value pairs) that acts as hot properties in the config map. | `spark.kubernetes.operator.reconciler.intervalSeconds: "60"` | @@ -154,7 +155,7 @@ for the operator pod: ```yaml operatorDeployment: networkPolicy: - enable: true + enabled: true metricsIngress: - namespaceSelector: matchLabels: @@ -174,6 +175,12 @@ Note that this requires a CNI plugin that enforces NetworkPolicy; on clusters wi a plugin the policy is silently ignored. Egress traffic of the operator (Kubernetes API server, DNS) is not restricted by this policy. +The legacy key `operatorDeployment.networkPolicy.enable` is deprecated in favor of `enabled` +and will be removed in chart `2.0.0` ([SPARK-59533](https://issues.apache.org/jira/browse/SPARK-59533)). +It is still honored: the NetworkPolicy is created when either key is `true`, so a stale +`enable: true` in a base values file wins over `enabled: false` and must be removed to turn +the feature off. The same rule applies to `operatorConfiguration.dynamicConfig.enable`. + ## Operator Health(Liveness) Probe with Sentinel Resource Learning diff --git a/tests/e2e/helm/dynamic-config-values-2.yaml b/tests/e2e/helm/dynamic-config-values-2.yaml index b6410cb1..38891b4e 100644 --- a/tests/e2e/helm/dynamic-config-values-2.yaml +++ b/tests/e2e/helm/dynamic-config-values-2.yaml @@ -27,7 +27,7 @@ workloadResources: operatorConfiguration: dynamicConfig: - enable: true + enabled: true create: true data: spark.kubernetes.operator.watchedNamespaces: "spark-3" diff --git a/tests/e2e/helm/dynamic-config-values-file.yaml b/tests/e2e/helm/dynamic-config-values-file.yaml index 568477a7..8a7296b2 100644 --- a/tests/e2e/helm/dynamic-config-values-file.yaml +++ b/tests/e2e/helm/dynamic-config-values-file.yaml @@ -33,7 +33,7 @@ operatorDeployment: operatorConfiguration: dynamicConfig: - enable: true + enabled: true source: file create: true data: diff --git a/tests/e2e/helm/dynamic-config-values.yaml b/tests/e2e/helm/dynamic-config-values.yaml index aac7cd24..d0d0569d 100644 --- a/tests/e2e/helm/dynamic-config-values.yaml +++ b/tests/e2e/helm/dynamic-config-values.yaml @@ -33,7 +33,7 @@ operatorDeployment: operatorConfiguration: dynamicConfig: - enable: true + enabled: true create: true data: spark.kubernetes.operator.watchedNamespaces: "default" diff --git a/tests/e2e/helm/helm-test-values/network-policy/values.yaml b/tests/e2e/helm/helm-test-values/network-policy/values.yaml index cef0a923..62ab1d85 100644 --- a/tests/e2e/helm/helm-test-values/network-policy/values.yaml +++ b/tests/e2e/helm/helm-test-values/network-policy/values.yaml @@ -21,7 +21,7 @@ operatorDeployment: networkPolicy: - enable: true + enabled: true metricsIngress: - namespaceSelector: matchLabels: