Skip to content

feat: add Apache Ignite 3 kit (ignite3)#684

Open
rustyrazorblade wants to merge 2 commits into
mainfrom
668-add-ignite
Open

feat: add Apache Ignite 3 kit (ignite3)#684
rustyrazorblade wants to merge 2 commits into
mainfrom
668-add-ignite

Conversation

@rustyrazorblade

Copy link
Copy Markdown
Owner

Closes #668

Summary

  • Adds a new ignite3 kit deploying Apache Ignite 3 on db nodes via raw K8s manifests (StatefulSet + headless Service + NodePort Service + one-time cluster-init Job)
  • Configurable storage profiles: aimem (volatile), aipersist (default, in-memory + disk persistence), rocksdb (disk-based LSM)
  • --version arg (default 3.1.0), --replicas arg (default: db node count)
  • SQL capability via thin client JDBC driver on port 10800 (ignite3 sql)
  • Metrics: Ignite's native OTLP exporter pushes to the OTel Collector after cluster init
  • Traces: OTel Java agent (/usr/local/otel/opentelemetry-javaagent.jar) mounted from host AMI, sends to node-local OTel Collector via status.hostIP:4317
  • Logs: covered by existing OTel filelog receiver (/var/log/**/*.log picks up K3s pod logs automatically)
  • No new Kotlin code — kit uses existing manifest, shell, wait, and delete step infrastructure

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review: feat: add Apache Ignite 3 kit (ignite3)

Overall this is a well-structured addition — the design doc is thorough, the observability integration (OTLP push, OTel Java agent) is elegant, and the decision to use raw manifests over a non-existent Helm chart is correct. A few issues need fixing before this is production-ready.


Bugs

uninstall leaves orphaned resources

The uninstall sequence only deletes the headless Service and PVCs. If a user runs kit uninstall ignite3 without first running ignite3 stop, the StatefulSet, NodePort Service, ConfigMap, and cluster-init Job are all left running. This puts the cluster in a broken state (headless service gone, pods still running with broken discovery).

The uninstall sequence needs to cover all resources, or explicitly require / invoke stop first. Looking at how the stop sequence is defined, the simplest fix is to prepend the stop steps to uninstall.


Init Job idempotency

cluster-init-job.yaml.template sets backoffLimit: 0 and restartPolicy: Never. If ignite3 start is run a second time without stop, the Job named cluster-init already exists and kubectl apply will attempt to patch it. Kubernetes does not allow updating most Job spec fields, so this will either fail or silently no-op while leaving the cluster in an already-initialized state.

Options: check for the existing Job before applying, or delete the Job as the first step of start if it exists (kubectl delete job cluster-init --ignore-not-found).


Documentation version mismatch

docs/user-guide/ignite3.md line 58:

| `--version` | Apache Ignite 3 Docker image version | `3.0.0` |

But kit.yaml has default: "3.1.0" and libs.versions.toml pins ignite3 = "3.1.0". The doc should say 3.1.0.


Missing Validation

The spec explicitly requires:

Scenario: Invalid storage value is rejected — WHEN the user supplies an unrecognized value for --storage, THEN an error is emitted before any resources are applied.

There is no validation in kit.yaml or the start shell scripts. The invalid value would be silently interpolated into ignite-config.conf and likely cause a runtime failure deep in the Ignite startup sequence rather than a clean early error. Add a guard at the top of the start shell step:

case "${STORAGE_PROFILE}" in
  aimem|aipersist|rocksdb) ;;
  *) echo "ERROR: unknown --storage value '${STORAGE_PROFILE}'. Valid: aimem, aipersist, rocksdb" >&2; exit 1 ;;
esac

Resource Design

PVCs always created, even for aimem

statefulset.yaml.template unconditionally includes a volumeClaimTemplates block that provisions a 10Gi PVC per pod. For the aimem profile (pure in-memory, no persistence), these PVCs are wasted storage and create unnecessary cleanup overhead.

Consider either conditionally omitting the PVC template when STORAGE_PROFILE=aimem (requires two StatefulSet templates), or mounting an emptyDir for aimem and only using the PVC template for aipersist/rocksdb.


OTLP configuration uses CONTROL_HOST_PRIVATE to reach NodePort

curl -s -f -X PATCH "http://${CONTROL_HOST_PRIVATE}:30300/management/v1/configuration/cluster" \

NodePort services are exposed on db node IPs, not the control node. Whether CONTROL_HOST_PRIVATE:30300 is reachable depends on whether the control node is in the K3s cluster as a server node (in which case NodePorts are accessible there too). This may work in practice but should be documented or made explicit. If it's intentional (control node runs K3s server), a comment would help future readers.


Minor

  • The stop sequence deletes the ignite-config ConfigMap but uninstall does not attempt to delete it (beyond what stop would have already removed). Once the uninstall sequence is fixed to cover all resources, this will be addressed automatically.
  • services.yaml.template (headless service) only exposes port 3344 (discovery). Port 10300 (REST) is not listed. This is technically fine for DNS-based routing since headless service A records return pod IPs directly, but adding port 10300 to the spec would make the intent explicit and generate correct SRV records.
  • The dependency added is org.apache.ignite:ignite-jdbc while the design doc references org.apache.ignite:ignite-client. The JDBC artifact is the right choice for the driver class org.apache.ignite.jdbc.IgniteJdbcDriver — but worth confirming this artifact includes all transitive dependencies needed for the thin client (auth, SSL, etc.) since ignite-client was the original intent.

Summary

Severity Issue
🔴 Bug uninstall leaves StatefulSet, NodePort Service, ConfigMap, and Job running
🔴 Bug Re-running start without stop fails on Job update
🟡 Spec gap --storage validation not implemented per spec
🟡 Design PVCs always created regardless of storage profile
🟡 Clarity OTLP REST curl targets CONTROL_HOST_PRIVATE; verify this is correct or add a comment
🟢 Doc Version default mismatch (3.0.0 in docs vs 3.1.0 in kit.yaml)

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Total Project Coverage 70.94%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add ignite

1 participant