The kafka-zk-to-kraft-migration suite did not exercise the pre-upgrade hook,
it reimplemented a simplified flow beside it: a controller pool named
"controller" instead of the hashed c-<hash>, no strimzi.io/next-node-ids pin,
and a single broker so the controller-vs-broker node-id collision could never
occur. That is exactly the defect the hook exists to prevent, so the suite
stayed green on a revision whose in-place migration lost every message —
reproduced by hand 2/2 on a live cluster. A suite that cannot fail on this
code is worse than no suite: it reads as migration coverage and invites trust
it has not earned, while costing ~380s on every Kafka PR and asserting only
that a PVC object exists, never reading a record back. The migration hook's
bats suite and the live upgrade runs are the real coverage; the Strimzi bump
would delete this suite regardless.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The fresh-create Kafka suite relied on the shared 5m cleanup default, but a
KRaft cluster tears down broker and controller node pools, the entity
operator, and their PVCs — more work than a single ZooKeeper node. Under a
contended CI run (concurrent Postgres/OpenSearch/Redis suites) that teardown
overran 5m and failed the suite after every assertion had already passed, so
cleanup latency, not the test, was deciding the verdict. Give this suite its
own 8m cleanup timeout; the migration suite runs in its own namespace and is
unaffected.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Fresh KRaft clusters no longer need to be alone in a namespace. Their
node pools get short, release-hashed names — "b-<hash>" for brokers,
"c-<hash>" for controllers — instead of the fixed "kafka"/"controller".
Each release hashes differently, so the names are unique within the
namespace (KafkaNodePool object names are namespace-unique) and any
number of Kafka instances coexist. The hash keeps the pool name a fixed
length: Strimzi derives every pod/PVC name as <cluster>-<pool>-<id> and
the cluster name is already the release name, so embedding the release
name in the pool name too would double it into every pod hostname and
blow the 63-character DNS label limit for long release names. A
render-time guard fails with a clear message when even the hashed
controller node name would still exceed 63 characters.
The fixed "kafka" name is still required to adopt an existing ZooKeeper
cluster's <cluster>-kafka-N brokers and data in place during migration,
so kafka.brokerPoolName resolves to "kafka" for a migrating/migrated
cluster and "b-<hash>" for a fresh one. A second ZooKeeper->KRaft
migration into a namespace that already holds a "kafka" pool owned by
another cluster fails closed, both at render time and in the pre-upgrade
hook (whose owner probe uses --ignore-not-found + an explicit exit-code
check, so a transient kubectl error aborts rather than silently waiving
the collision guard). This replaces the previous blanket "one Kafka per
namespace" guard, which over-restricted the fresh case.
The pool names are resolved from a live Helm lookup; that is safe because
Helm propagates any non-NotFound read error and fails the whole render
(pkg/engine/lookup_func.go), so a transient apiserver/RBAC miss retries
rather than silently flipping a live cluster's pool name — an empty
lookup means genuine absence or a never-applied client-side render.
The migration hook and workload monitors follow the same naming. Tests:
helm-unittest fixtures (via kubernetesProvider) cover every resolution
branch and the fail-closed guards; hack/kafka-migration-hook.bats drives
the migration Job's real shell script against a stub kubectl across the
fresh / read-error / already-KRaft / owner-collision / happy-path /
stuck / retry cases. Verified on a live cluster: in-place ZooKeeper->KRaft
migration preserved all data and adopted the original broker PVCs, and
two fresh clusters coexist with distinct pools, pods and PVCs.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Addresses the second review round on the ZK->KRaft migration.
Migration hook:
- Fully qualify the Strimzi resources as kafkas.kafka.strimzi.io /
kafkanodepools.kafka.strimzi.io. The bare plural "kafkas" also exists in this
chart's apps.cozystack.io group, so a bare name resolves by per-cluster
discovery priority and can hit the tenant kind — which the hook Role does not
grant, failing the pre-upgrade hook deterministically.
- Classify NotFound structurally via `kubectl get --ignore-not-found` instead of
substring-matching stderr, so an unrelated "... not found" error can no longer
be mistaken for a fresh install and permanently skip a live ZooKeeper cluster.
This also drops the jq dependency.
- Treat a KRaft-native / already-migrated CR (annotated strimzi.io/kraft=enabled)
as done, never driving it backwards into kraft=migration.
- Keep only `before-hook-creation` on the Job's hook-delete-policy so a failed
attempt's pod logs survive for a post-mortem until the next run.
- Route the kubectl image through cozy-lib.image (cluster registry mirror) and
pin it by digest (clastix/kubectl:v1.32@sha256:...), matching the other
cozystack hooks, instead of an unpinned docker.io tag.
Controller quorum:
- Fix the controller node-pool replica count at creation instead of deriving it
from kafka.replicas on every reconcile. Strimzi 0.45 cannot scale a controller
pool, so a day-2 edit of kafka.replicas (e.g. 1 -> 3) must not flip the quorum;
the helper now keeps the existing "controller" pool's replica count when it is
present.
Docs:
- Document that migration to KRaft is one-way: neither a version-field downgrade
nor a pre-KRaft chart rollback is supported once migrated (Strimzi has no
KRaft->ZooKeeper rollback after completion).
E2E:
- Run the ZK->KRaft chainsaw migration in its own namespace instead of the
shared tenant-test. It raw-applies Strimzi resources plus a namespace-wide
LimitRange and leaves deleteClaim:false PVCs, which polluted the sibling kafka
suite. Raise the seed readiness timeout for CI headroom. Validated against a
live cluster: migration adopts the broker PVC in place and reaches KRaft under
a 128Mi LimitRange, in isolation.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The ZK->KRaft pre-upgrade hook created the adopting broker ("kafka") and
"controller" KafkaNodePools without a resources block. Those hook-created pools
govern the Kafka pods for the entire migration window — Helm applies the
resourced steady-state kafkanodepools.yaml only after the pre-upgrade hook
finishes. In a tenant namespace the LimitRange defaults every container to
128Mi, so the migration's controller (and rolled broker) JVMs OOM, never reach
KRaftPostMigration, and the hook times out — wedging the migration and thrashing
it on every reconcile.
Give both hook pools the same resources the steady-state pools get
(cozy-lib.resources.defaultingSanitize from kafka.resourcesPreset/resources).
This also avoids a redundant second broker roll (resource-less -> resourced)
after the hook completes.
Tests: a helm-unittest assertion that both migration pools render a resources
block; the Chainsaw ZK->KRaft suite now seeds under a 128Mi LimitRange (and
gives the seed cluster explicit resources), reproducing the tenant-quota
condition, and asserts the migration still reaches KRaft with the broker PVC
adopted. Validated green against a live cluster under the LimitRange.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Follow-up to the KRaft-migration review. The broker KafkaNodePool must be named
exactly "kafka" so Strimzi adopts an existing ZooKeeper cluster's
<cluster>-kafka-N brokers and data in place during migration, but KafkaNodePool
object names are unique within a namespace — so two Kafka clusters cannot both
own a "kafka" pool there. This is an upstream constraint and Strimzi's own
guidance is one Kafka cluster per namespace
(strimzi/strimzi-kafka-operator discussions/11120).
Adopt that model instead of the earlier fresh-vs-migrated conditional naming:
- New kafka.assertSingleInstance helper fails the render (with a clear message
and the upstream reference) if the namespace already holds another Kafka
cluster. Reconciles/upgrades of the same release are allowed; lookup returns
nothing during `helm template`, so unit tests never trip it.
- Node pools are now named "kafka" (broker) and "controller" unconditionally,
which also adopts existing brokers on migration and yields clean
<cluster>-kafka-N / <cluster>-controller-N names. The fresh-vs-migrated
brokerPoolName helper is removed.
- WorkloadMonitor pool-name selectors and the migration hook updated to match;
unit tests updated for the fixed pool names.
- README documents the one-Kafka-per-namespace rule and links the Strimzi
discussion.
The ZK->KRaft Chainsaw migration suite (seed ZooKeeper cluster, migrate, assert
the original broker PVC survives and no wrongly-named PVC is created, cluster
reaches KRaft) was run green against a live cluster.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Addresses the branch review on the ZK->KRaft migration path.
CRITICAL — broker node pool adoption. Strimzi derives broker/PVC names as
<cluster>-<pool>-<id>; a pre-node-pool ZooKeeper cluster has brokers
<cluster>-kafka-N, so the adopting broker pool must be named exactly "kafka"
or the operator provisions fresh empty brokers and orphans the tenant's data.
A new kafka.brokerPoolName helper resolves the broker pool to "kafka" for an
existing (migrating) cluster and keeps the release-scoped "<release>-broker"
for fresh installs, so multiple Kafka instances still coexist in one namespace.
The migration hook creates the pool as "kafka" and fails closed if a "kafka"
pool already belongs to another cluster (node pool names are namespace-unique,
so in-place migration of co-namespaced ZK clusters is unsupported). Wired
through kafkanodepools.yaml, workloadmonitor.yaml and the hook.
MAJOR — migration robustness.
- Raise the HelmRelease timeout: the migration Job can run tens of minutes but
the release timeout defaults to 10m, so helm-controller aborted and thrashed
the pre-upgrade hook. Add release.cozystack.io/helm-install-timeout=45m on the
kafka ApplicationDefinition, above the Job's activeDeadlineSeconds (2400s).
- Idempotent migration: do not re-issue kraft=migration once the cluster is past
the ZooKeeper phase (an invalid backward transition Strimzi rejects on retry).
- Fail closed on the decision gates: distinguish a genuine NotFound (skip) from
any other kubectl error (abort), and never treat an empty kafkaMetadataState on
an existing CR as "migrated".
MINOR — version downgrade documented as unsupported; integer (not lexical)
compare in the version gate; deleteClaim/PVC-retention and the one-per-namespace
migration limitation documented in the README; docs/storage-immutability.md
updated for KRaft.
Tests — helm-unittest coverage for the broker pool name, the hook guards and the
idempotent/fail-closed logic; a bats check for the helm-install-timeout budget
ordering; and a Chainsaw ZK->KRaft migration suite that seeds a ZooKeeper
cluster, drives the migration, and asserts the original broker PVC survives (not
recreated) and the cluster reaches KRaft. The migration sequence and the
Chainsaw assertions were validated against a live cluster.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Replace ZooKeeper-based Kafka with KRaft (Kafka Raft) metadata mode.
Strimzi 0.45 is the last line that still supports ZooKeeper, so this
prepares the transition before ZooKeeper support is removed upstream.
Chart changes:
- The Kafka CR always renders strimzi.io/kraft=enabled with node pools
and no spec.zookeeper block. Storage, replicas and resources move to
separate broker + controller KafkaNodePools (a homogeneous separate-pool
layout works for both fresh installs and migrations; Strimzi 0.45 does
not support migration into a combined pool).
- Kafka version is now user-configurable via the standard files/versions.yaml
pattern (v3.9 default, v3.8 available); appVersion bumped to 3.9.0.
- WorkloadMonitors track the broker and controller pools (no zookeeper
monitor); the dashboard RBAC grants the controller monitor and drops the
zookeeper one.
Migration of existing ZooKeeper clusters is handled by a Helm pre-upgrade
Job (templates/migration-hook.yaml) gated by a <release>-kafka-deployed-version
ConfigMap, following the established seaweedfs/etcd hook pattern. The Job
creates the broker + controller pools with Helm ownership labels, drives the
Strimzi ZK->KRaft state machine (node-pools=enabled, kraft=migration ->
kraft=enabled once KRaftPostMigration is reached) and is skipped once the
ConfigMap records version >= "1".
Testing:
- Helm unit tests cover the Kafka CR, node pools, migration hook, metrics,
topics, version map, dashboard RBAC and workload monitors.
- The e2e suite is ported to the current Chainsaw framework
(hack/e2e-chainsaw/kafka): it asserts the KRaft topology (both node pools,
kafkaMetadataState=KRaft), the absence of ZooKeeper, and bootstrap
endpoints.
Rebased onto current main: TLS listener support, entity-operator resource
guards, x-cozystack-options schema annotations and the WorkloadMonitor
replica-path fix are all preserved and reconciled with KRaft.
Signed-off-by: Matthieu <matthieu@hidora.com>
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The post-delete absence assertions could pass vacuously if the two
release topics were never created. Asserting both topics first makes
the E2E prove that the pre-delete cleanup actually removed them.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Replaces the per-app BATS E2E suite (hack/e2e-apps/*.bats) with declarative
Kyverno Chainsaw tests under hack/e2e-chainsaw/, extending this branch's pilot
(postgres + bucket) to all 18 app suites.
- Declarative ports assert on status/conditions and concrete fields; the
repeated "timeout ... until kubectl get" + "kubectl wait" boilerplate
collapses into a single polling assert with structured failure diffs and
automatic events/describe/podLogs capture on failure.
- Inherently imperative suites keep their logic in script steps: openbao
init/unseal, kuberture split-horizon probes, vminstance, the gateway
admission/impersonation cases, and kubernetes-latest/previous, which wrap
the relocated hack/e2e-chainsaw/_lib/run-kubernetes.sh verbatim.
- Gateway tests derive the tenant apex from the namespace host label at
runtime, so they are host-independent.
CI: the e2e job now runs "chainsaw test hack/e2e-chainsaw/" via a new
test-chainsaw target and uploads the JUnit chainsaw-report.xml; the chainsaw
binary is added to the e2e-sandbox image. install-cozystack and test-openapi
stay BATS. The per-app 3-retry loop is dropped, since assertion polling
replaces the fixed-timeout flakiness it papered over.
Verified against a dev cluster (DB/app, storage and VM suites pass). Service
port assertions use the ports[*].port projection form because Chainsaw
v0.2.15 mis-evaluates JMESPath number-literal port filters.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Alexey Artamonov