mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-23 03:27:50 +09:00
fix(kafka): skip the OpenAPI patch lookup in the migration hook
The pre-install/pre-upgrade hook applies the node pools and the drain KafkaRebalance with kubectl, and its container has no resources of its own, so in a tenant with resource quotas it runs under the LimitRange default of 128Mi. When it applies an object that already exists, kubectl downloads the cluster's whole OpenAPI v2 document only to decide that a custom resource takes a JSON merge patch: with the 374 CRDs of an e2e cluster that peaks at about 300Mi, so every upgrade of a Kafka in such a tenant OOM-kills the hook and fails with "pre-upgrade hooks failed". --openapi-patch=false skips that lookup and builds the same three-way merge patch from the last-applied annotation, peaking at about 35Mi on the same cluster. A server-side apply would be as small but changes what gets removed: the "kafka" pool is created with "kubectl create", whose manager keeps co-owning every field it set, so a later server-side apply leaves those fields in place when the chart drops them. A larger memory limit would charge every upgrade against the tenant quota instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
3 files changed
+29
-7
No files matched your search
@@ -60,7 +60,7 @@ case "$args" in
|
||||
[ -n "${STUB_CREATE_FAIL:-}" ] && { echo "AlreadyExists" >&2; exit 1; }
|
||||
exit 0
|
||||
;;
|
||||
*"apply -f -"*)
|
||||
*"apply "*"-f -"*)
|
||||
cat >> "${KUBECTL_LOG:-/dev/null}"
|
||||
exit 0
|
||||
;;
|
||||
@@ -206,7 +206,7 @@ run_hook() {
|
||||
|
||||
[ "$rc" != 0 ]
|
||||
grep -qF 'already belongs to cluster' "$tmp/out"
|
||||
if grep -qE 'apply -f -|create -f -|kraft=|node-pools=' "$tmp/kubectl.log"; then
|
||||
if grep -qE 'apply .*-f -|create -f -|kraft=|node-pools=' "$tmp/kubectl.log"; then
|
||||
echo "FAIL: a foreign-owned kafka pool did not abort before mutating"
|
||||
cat "$tmp/kubectl.log"
|
||||
return 1
|
||||
@@ -223,7 +223,7 @@ run_hook() {
|
||||
|
||||
[ "$rc" != 0 ]
|
||||
grep -qF 'owner label failed' "$tmp/out"
|
||||
if grep -qE 'apply -f -|create -f -|kraft=|node-pools=' "$tmp/kubectl.log"; then
|
||||
if grep -qE 'apply .*-f -|create -f -|kraft=|node-pools=' "$tmp/kubectl.log"; then
|
||||
echo "FAIL: proceeded to mutate the cluster after a failed owner read"
|
||||
cat "$tmp/kubectl.log"
|
||||
return 1
|
||||
|
||||
@@ -122,6 +122,15 @@ spec:
|
||||
np=kafkanodepools.kafka.strimzi.io
|
||||
kr=kafkarebalances.kafka.strimzi.io
|
||||
|
||||
# Every apply here passes --openapi-patch=false. Without it, applying
|
||||
# an object that already exists makes kubectl download the cluster's
|
||||
# whole OpenAPI v2 document only to learn that a custom resource
|
||||
# takes a JSON merge patch: about 300Mi with the platform's CRDs,
|
||||
# while this container runs under a tenant LimitRange default of
|
||||
# 128Mi. With it kubectl builds the same merge patch from the
|
||||
# last-applied annotation. Not --server-side: fields that
|
||||
# "kubectl create" set on the "kafka" pool stay co-owned by that
|
||||
# manager, so a server-side apply would never remove them.
|
||||
# ---- Node-pool manifests (broker pool reused for the create-CAS) ----
|
||||
broker_manifest=$(cat <<MANIFEST
|
||||
apiVersion: kafka.strimzi.io/v1beta2
|
||||
@@ -256,10 +265,10 @@ spec:
|
||||
fi
|
||||
fi
|
||||
echo "Ensuring broker KafkaNodePool \"$brokerPool\" exists..."
|
||||
echo "$broker_manifest" | kubectl apply -f -
|
||||
echo "$broker_manifest" | kubectl apply --openapi-patch=false -f -
|
||||
|
||||
echo "Ensuring controller KafkaNodePool exists..."
|
||||
echo "$controller_manifest" | kubectl apply -f -
|
||||
echo "$controller_manifest" | kubectl apply --openapi-patch=false -f -
|
||||
|
||||
# ---- Decide what to do from the Kafka CR ----
|
||||
# Fail closed. --ignore-not-found makes kubectl exit 0 with empty
|
||||
@@ -384,7 +393,7 @@ spec:
|
||||
# F1: stand up this cluster's own release-hashed broker pool with
|
||||
# node IDs pinned high, clear of the existing broker/controller IDs.
|
||||
echo "Creating target broker pool \"$freshBrokerPool\"..."
|
||||
cat <<POOL | kubectl apply -f -
|
||||
cat <<POOL | kubectl apply --openapi-patch=false -f -
|
||||
apiVersion: kafka.strimzi.io/v1beta2
|
||||
kind: KafkaNodePool
|
||||
metadata:
|
||||
@@ -441,7 +450,7 @@ spec:
|
||||
exit 1
|
||||
fi
|
||||
echo "Draining brokers $kafkaIds via Cruise Control rebalance..."
|
||||
cat <<REB | kubectl apply -f -
|
||||
cat <<REB | kubectl apply --openapi-patch=false -f -
|
||||
apiVersion: kafka.strimzi.io/v1beta2
|
||||
kind: KafkaRebalance
|
||||
metadata:
|
||||
|
||||
@@ -447,3 +447,16 @@ tests:
|
||||
path: spec.template.spec.containers[0].args[0]
|
||||
pattern: "roles:\\s+- controller[\\s\\S]*?resources:\\s+limits:"
|
||||
documentIndex: 3
|
||||
|
||||
- it: applies without the OpenAPI patch lookup, so an upgrade fits the tenant LimitRange default
|
||||
release:
|
||||
name: test-kafka
|
||||
namespace: tenant-test
|
||||
documentIndex: 3
|
||||
asserts:
|
||||
- notMatchRegex:
|
||||
path: spec.template.spec.containers[0].args[0]
|
||||
pattern: "kubectl[^|\\n]* apply (-f|--filename|--server-side)"
|
||||
- matchRegex:
|
||||
path: spec.template.spec.containers[0].args[0]
|
||||
pattern: "echo \"\\$broker_manifest\" \\| kubectl apply --openapi-patch=false -f -\\n[\\s\\S]*echo \"\\$controller_manifest\" \\| kubectl apply --openapi-patch=false -f -\\n[\\s\\S]*<<POOL \\| kubectl apply --openapi-patch=false -f -\\n[\\s\\S]*<<REB \\| kubectl apply --openapi-patch=false -f -\\n"
|
||||
Reference in new issue
Block a user