aicam commented on code in PR #6617:
URL: https://github.com/apache/texera/pull/6617#discussion_r3639728178
##########
bin/k8s/templates/base/pod-disruption-budgets/app-service-pdbs.yaml:
##########
Review Comment:
Move this file and folder under `aws` since they are mainly for that purpose
##########
bin/k8s/utils/cu-nodepool.yaml:
##########
Review Comment:
Move this one under `aws`, its purpose is only for aws deployment
##########
common/config/src/main/resources/kubernetes.conf:
##########
@@ -54,4 +54,21 @@ kubernetes {
# GPU resource key used in Kubernetes (vendor-specific)
computing-unit-gpu-resource-key = "nvidia.com/gpu"
computing-unit-gpu-resource-key =
${?KUBERNETES_COMPUTING_UNIT_GPU_RESOURCE_KEY}
+
+ # Optional: pin CU pods to a dedicated, tainted Karpenter NodePool so their
+ # disruption policy (WhenEmpty) is isolated from shared nodes and persists.
+ # See bin/k8s/utils/cu-nodepool.yaml. When the label key+value are set, the
+ # cu-manager adds a matching nodeSelector to each CU pod; when the toleration
+ # key is set, it adds a NoSchedule/Exists toleration for that taint.
+ #
+ # Leave all three EMPTY (default) to schedule CU pods on the cluster's
default
+ # pool — required for local/dev clusters that have no dedicated CU NodePool.
+ compute-unit-node-selector-label = ""
+ compute-unit-node-selector-label =
${?KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_LABEL}
Review Comment:
Should this value match bin/k8s/utils/cu-nodepool.yaml metadata.name? if so,
add it to default env values in values-aws.yaml and a comment to make sure user
know match them, the default value should be default name of node pool
##########
bin/k8s/utils/cu-nodepool.yaml:
##########
@@ -0,0 +1,124 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Dedicated Karpenter NodePool for on-demand CU pods (AWS / EKS only).
+#
+# This is a standalone cluster manifest (like the other bin/k8s/utils files),
+# NOT part of the Helm release — apply it with kubectl on an AWS EKS cluster
+# that runs Karpenter. It has no effect on, and is not rendered by, an on-prem
+# install.
+#
+# WHY THIS EXISTS
+# ---------------
+# CU pods (namespace texera-workflow-computing-unit-pool) are *bare* pods — the
+# cu-manager creates them with no Deployment/StatefulSet behind them (see
+# KubernetesClient.createPod). So if Karpenter evicts one, nothing recreates
it:
+# the CU is permanently gone (often leaving a "zombie" DB row that still shows
+# RUNNING). The EKS Auto Mode default `general-purpose` pool runs
+# consolidationPolicy: WhenEmptyOrUnderutilized, so ~30s after a node looks
+# underutilized Karpenter evicts its pods to repack — which can mass-kill live
+# CUs during a burst of concurrent users.
+#
+# Patching the `general-purpose` pool to WhenEmpty does NOT hold: it is
+# EKS-managed (metadata.labels app.kubernetes.io/managed-by=eks) and Auto Mode
+# reconciles the edit back to WhenEmptyOrUnderutilized within ~30 min.
+#
+# THE FIX
+# -------
+# Give CUs their own NON-EKS-managed NodePool so its disruption policy actually
+# persists, and set consolidationPolicy: WhenEmpty. Karpenter then never evicts
+# a running CU to repack an underutilized node, while EMPTY CU nodes still
scale
+# down normally (no orphaned EC2 instances).
+#
+# We deliberately do NOT put a karpenter.sh/do-not-disrupt annotation on CU
pods
+# (see KubernetesClient.createPod): that annotation blocks empty-node scaledown
+# and expiry too, which is exactly what pinned nodes / leaked EC2 instances in
a
+# past incident. WhenEmpty gives the protection without that side effect.
+#
+# CU pods land here via a nodeSelector + toleration set by the cu-manager
+# (KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_LABEL / _VALUE / _TOLERATION_KEY,
wired
+# from values-aws.yaml). The prepuller DaemonSet already tolerates all taints
+# via operator: Exists.
+#
+# HOW TO APPLY (standalone, like the other bin/k8s/utils manifests)
+# kubectl apply -f bin/k8s/utils/cu-nodepool.yaml
+# Verify:
+# kubectl get nodepool cu-pool -o
jsonpath='{.spec.disruption.consolidationPolicy}' # -> WhenEmpty
+#
+# NOTE: nodeClassRef points at the EKS Auto Mode `default` NodeClass. This pool
+# is OURS (not managed-by=eks), so its settings persist. Adjust the zones and
+# instance-type list below to match your cluster/region.
+apiVersion: karpenter.sh/v1
+kind: NodePool
+metadata:
+ name: cu-pool
+spec:
+ disruption:
+ # Only reclaim a CU node once it is completely empty — never evict a
running
+ # CU to repack. This is the whole point of the pool.
+ consolidationPolicy: WhenEmpty
+ # How long a node must sit empty before removal. Long enough to avoid
thrash
+ # while CUs churn (a freed node is likely to receive the next CU); short
+ # enough to not leak idle nodes.
+ consolidateAfter: 2m
+ budgets:
+ - nodes: "10%"
+ template:
+ metadata:
+ labels:
+ # CU pods select this via nodeSelector
(KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_*).
+ texera.io/node-role: computing-unit
+ spec:
+ # Bounded node rotation. With WhenEmpty + no do-not-disrupt, a node that
+ # always hosts >=1 CU never goes empty; expiry guarantees it still
rotates
+ # (for patching) at most every 14 days, forcibly draining CUs then. Set
+ # long so it rarely interrupts an active session.
+ expireAfter: 336h
+ nodeClassRef:
+ group: eks.amazonaws.com
+ kind: NodeClass
+ name: default
+ taints:
+ # Keeps everything except CU pods (and the prepuller, which tolerates
it
+ # via operator: Exists) off these nodes. Toleration uses operator:
+ # Exists, so the taint needs no value.
+ - key: texera.io/computing-unit
+ effect: NoSchedule
+ requirements:
+ - key: karpenter.sh/capacity-type
Review Comment:
Move dependencies on CPU type to values-aws.yaml, user should be able to
allow both CPU types
##########
bin/k8s/utils/cu-nodepool.yaml:
##########
@@ -0,0 +1,124 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Dedicated Karpenter NodePool for on-demand CU pods (AWS / EKS only).
+#
+# This is a standalone cluster manifest (like the other bin/k8s/utils files),
+# NOT part of the Helm release — apply it with kubectl on an AWS EKS cluster
+# that runs Karpenter. It has no effect on, and is not rendered by, an on-prem
+# install.
+#
+# WHY THIS EXISTS
+# ---------------
+# CU pods (namespace texera-workflow-computing-unit-pool) are *bare* pods — the
+# cu-manager creates them with no Deployment/StatefulSet behind them (see
+# KubernetesClient.createPod). So if Karpenter evicts one, nothing recreates
it:
+# the CU is permanently gone (often leaving a "zombie" DB row that still shows
+# RUNNING). The EKS Auto Mode default `general-purpose` pool runs
+# consolidationPolicy: WhenEmptyOrUnderutilized, so ~30s after a node looks
+# underutilized Karpenter evicts its pods to repack — which can mass-kill live
+# CUs during a burst of concurrent users.
+#
+# Patching the `general-purpose` pool to WhenEmpty does NOT hold: it is
+# EKS-managed (metadata.labels app.kubernetes.io/managed-by=eks) and Auto Mode
+# reconciles the edit back to WhenEmptyOrUnderutilized within ~30 min.
+#
+# THE FIX
+# -------
+# Give CUs their own NON-EKS-managed NodePool so its disruption policy actually
+# persists, and set consolidationPolicy: WhenEmpty. Karpenter then never evicts
+# a running CU to repack an underutilized node, while EMPTY CU nodes still
scale
+# down normally (no orphaned EC2 instances).
+#
+# We deliberately do NOT put a karpenter.sh/do-not-disrupt annotation on CU
pods
+# (see KubernetesClient.createPod): that annotation blocks empty-node scaledown
+# and expiry too, which is exactly what pinned nodes / leaked EC2 instances in
a
+# past incident. WhenEmpty gives the protection without that side effect.
+#
+# CU pods land here via a nodeSelector + toleration set by the cu-manager
+# (KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_LABEL / _VALUE / _TOLERATION_KEY,
wired
+# from values-aws.yaml). The prepuller DaemonSet already tolerates all taints
+# via operator: Exists.
+#
+# HOW TO APPLY (standalone, like the other bin/k8s/utils manifests)
+# kubectl apply -f bin/k8s/utils/cu-nodepool.yaml
+# Verify:
+# kubectl get nodepool cu-pool -o
jsonpath='{.spec.disruption.consolidationPolicy}' # -> WhenEmpty
+#
+# NOTE: nodeClassRef points at the EKS Auto Mode `default` NodeClass. This pool
+# is OURS (not managed-by=eks), so its settings persist. Adjust the zones and
+# instance-type list below to match your cluster/region.
+apiVersion: karpenter.sh/v1
+kind: NodePool
+metadata:
+ name: cu-pool
+spec:
+ disruption:
+ # Only reclaim a CU node once it is completely empty — never evict a
running
+ # CU to repack. This is the whole point of the pool.
+ consolidationPolicy: WhenEmpty
+ # How long a node must sit empty before removal. Long enough to avoid
thrash
+ # while CUs churn (a freed node is likely to receive the next CU); short
+ # enough to not leak idle nodes.
+ consolidateAfter: 2m
+ budgets:
+ - nodes: "10%"
+ template:
+ metadata:
+ labels:
+ # CU pods select this via nodeSelector
(KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_*).
+ texera.io/node-role: computing-unit
+ spec:
+ # Bounded node rotation. With WhenEmpty + no do-not-disrupt, a node that
+ # always hosts >=1 CU never goes empty; expiry guarantees it still
rotates
+ # (for patching) at most every 14 days, forcibly draining CUs then. Set
+ # long so it rarely interrupts an active session.
+ expireAfter: 336h
+ nodeClassRef:
+ group: eks.amazonaws.com
+ kind: NodeClass
+ name: default
+ taints:
+ # Keeps everything except CU pods (and the prepuller, which tolerates
it
+ # via operator: Exists) off these nodes. Toleration uses operator:
+ # Exists, so the taint needs no value.
+ - key: texera.io/computing-unit
+ effect: NoSchedule
+ requirements:
+ - key: karpenter.sh/capacity-type
+ operator: In
+ values: ["on-demand"]
+ - key: kubernetes.io/arch
+ operator: In
+ values: ["amd64"] # CU master image is amd64-only
+ - key: kubernetes.io/os
+ operator: In
+ values: ["linux"]
+ - key: topology.kubernetes.io/zone
+ operator: In
+ values: ["us-west-1a", "us-west-1c"]
Review Comment:
Add zones to values-aws.yaml
##########
bin/k8s/utils/general-purpose-nodepool-disruption.yaml:
##########
@@ -0,0 +1,72 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Disruption-policy override for the EKS Auto Mode `general-purpose` NodePool
+# (AWS / EKS only). Standalone cluster manifest, NOT part of the Helm release.
+#
+# WHY THIS EXISTS
+# ---------------
+# Any CU pod that does NOT carry the cu-pool nodeSelector/toleration (e.g.
before
+# cu-nodepool.yaml is applied, or if the placement env vars are unset) lands on
Review Comment:
We assume users follow instructions correctly, apply helm charts and set
envs, if this can solve the issue this file is addressing, remove it, we don't
want to keep an extra helm chart file as a fixing tool to apply conditionally,
if the file must exist, move it under `aws`
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/KubernetesClient.scala:
##########
@@ -133,6 +133,37 @@ object KubernetesClient {
specBuilder.withRuntimeClassName("nvidia")
}
+ // Pin CU pods to the dedicated Karpenter CU NodePool when configured.
+ //
+ // The CU NodePool (bin/k8s/utils/cu-nodepool.yaml) is non-EKS-managed and
Review Comment:
remove this comment
##########
bin/k8s/utils/cu-nodepool.yaml:
##########
@@ -0,0 +1,124 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Dedicated Karpenter NodePool for on-demand CU pods (AWS / EKS only).
+#
+# This is a standalone cluster manifest (like the other bin/k8s/utils files),
+# NOT part of the Helm release — apply it with kubectl on an AWS EKS cluster
+# that runs Karpenter. It has no effect on, and is not rendered by, an on-prem
+# install.
+#
+# WHY THIS EXISTS
+# ---------------
+# CU pods (namespace texera-workflow-computing-unit-pool) are *bare* pods — the
+# cu-manager creates them with no Deployment/StatefulSet behind them (see
+# KubernetesClient.createPod). So if Karpenter evicts one, nothing recreates
it:
+# the CU is permanently gone (often leaving a "zombie" DB row that still shows
+# RUNNING). The EKS Auto Mode default `general-purpose` pool runs
+# consolidationPolicy: WhenEmptyOrUnderutilized, so ~30s after a node looks
+# underutilized Karpenter evicts its pods to repack — which can mass-kill live
+# CUs during a burst of concurrent users.
+#
+# Patching the `general-purpose` pool to WhenEmpty does NOT hold: it is
+# EKS-managed (metadata.labels app.kubernetes.io/managed-by=eks) and Auto Mode
+# reconciles the edit back to WhenEmptyOrUnderutilized within ~30 min.
+#
+# THE FIX
+# -------
+# Give CUs their own NON-EKS-managed NodePool so its disruption policy actually
+# persists, and set consolidationPolicy: WhenEmpty. Karpenter then never evicts
+# a running CU to repack an underutilized node, while EMPTY CU nodes still
scale
+# down normally (no orphaned EC2 instances).
+#
+# We deliberately do NOT put a karpenter.sh/do-not-disrupt annotation on CU
pods
+# (see KubernetesClient.createPod): that annotation blocks empty-node scaledown
+# and expiry too, which is exactly what pinned nodes / leaked EC2 instances in
a
+# past incident. WhenEmpty gives the protection without that side effect.
+#
+# CU pods land here via a nodeSelector + toleration set by the cu-manager
+# (KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_LABEL / _VALUE / _TOLERATION_KEY,
wired
+# from values-aws.yaml). The prepuller DaemonSet already tolerates all taints
+# via operator: Exists.
+#
+# HOW TO APPLY (standalone, like the other bin/k8s/utils manifests)
+# kubectl apply -f bin/k8s/utils/cu-nodepool.yaml
+# Verify:
+# kubectl get nodepool cu-pool -o
jsonpath='{.spec.disruption.consolidationPolicy}' # -> WhenEmpty
+#
+# NOTE: nodeClassRef points at the EKS Auto Mode `default` NodeClass. This pool
+# is OURS (not managed-by=eks), so its settings persist. Adjust the zones and
+# instance-type list below to match your cluster/region.
+apiVersion: karpenter.sh/v1
+kind: NodePool
+metadata:
+ name: cu-pool
+spec:
+ disruption:
+ # Only reclaim a CU node once it is completely empty — never evict a
running
+ # CU to repack. This is the whole point of the pool.
+ consolidationPolicy: WhenEmpty
+ # How long a node must sit empty before removal. Long enough to avoid
thrash
+ # while CUs churn (a freed node is likely to receive the next CU); short
+ # enough to not leak idle nodes.
+ consolidateAfter: 2m
+ budgets:
+ - nodes: "10%"
+ template:
+ metadata:
+ labels:
+ # CU pods select this via nodeSelector
(KUBERNETES_COMPUTE_UNIT_NODE_SELECTOR_*).
+ texera.io/node-role: computing-unit
+ spec:
+ # Bounded node rotation. With WhenEmpty + no do-not-disrupt, a node that
+ # always hosts >=1 CU never goes empty; expiry guarantees it still
rotates
+ # (for patching) at most every 14 days, forcibly draining CUs then. Set
+ # long so it rarely interrupts an active session.
+ expireAfter: 336h
+ nodeClassRef:
+ group: eks.amazonaws.com
+ kind: NodeClass
+ name: default
+ taints:
+ # Keeps everything except CU pods (and the prepuller, which tolerates
it
+ # via operator: Exists) off these nodes. Toleration uses operator:
+ # Exists, so the taint needs no value.
+ - key: texera.io/computing-unit
+ effect: NoSchedule
+ requirements:
+ - key: karpenter.sh/capacity-type
+ operator: In
+ values: ["on-demand"]
+ - key: kubernetes.io/arch
+ operator: In
+ values: ["amd64"] # CU master image is amd64-only
+ - key: kubernetes.io/os
+ operator: In
+ values: ["linux"]
+ - key: topology.kubernetes.io/zone
+ operator: In
+ values: ["us-west-1a", "us-west-1c"]
+ - key: node.kubernetes.io/instance-type
+ operator: In
+ # m-family general compute; Karpenter picks the cheapest that fits
the
+ # pending CUs. 2xlarge–4xlarge packs several 4vCPU/16Gi CUs per node.
+ values:
Review Comment:
Allow empty (which means any instance type), or specific types, but should
be moved to `values-aws.yaml`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
