aicam commented on code in PR #6617: URL: https://github.com/apache/texera/pull/6617#discussion_r3639869895
########## 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: Done — `computingUnitNodePool.capacityTypes` in `values-aws.yaml`, so `["on-demand", "spot"]` is a one-line change; an empty list drops the requirement entirely. I left the default at `["on-demand"]` and documented why inline: a spot reclamation kills the CU pods on that node for good, since they are bare pods with no controller to recreate them. Cheaper, but the failure mode is the same one this PR exists to prevent, so it seemed right to make it an explicit opt-in rather than the default. `architectures` got the same treatment (`kubernetes.io/arch`), defaulting to `["amd64"]` because the stock CU master image is amd64-only — empty or `["amd64","arm64"]` lets Karpenter launch Graviton nodes once the image is multi-arch. ########## 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: Done — `computingUnitNodePool.zones` in `values-aws.yaml`. I shipped it as `[]`, which drops the zone requirement entirely (any zone the cluster's subnets cover), with a commented example. Hardcoding the `us-west-1a/1c` from our cluster would have been wrong for the example overlay, which is `us-west-2` throughout — and a zone list that does not match the cluster leaves CU pods Pending with no obvious cause. ########## 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: Done — `computingUnitNodePool.instanceTypes` in `values-aws.yaml`, defaulting to `[]`, which omits the `node.kubernetes.io/instance-type` requirement so Karpenter picks whatever fits the pending CUs. The old m-family list survives as a commented example for anyone who wants to pin cost or packing. Same mechanism for all four requirement lists (`capacityTypes`, `architectures`, `zones`, `instanceTypes`): empty means unconstrained. Only `kubernetes.io/os: linux` is always emitted. -- 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]
