cockroachdb / cockroachdb/helm-charts

Race condition bug in the CockroachDB Kubernetes Operator Helm chart (version 25.4.2-preview)

Open
#588 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
105
Forks
152
Avg merge
4d 23h
Merged PRs (30d)
5

Description

## 1. Problem Statement

### Observed Behavior
When deploying a 2-node CockroachDB cluster using the official `cockroachdb-parent` Helm chart with the CockroachDB Operator, the two pods **initialize as separate, independent clusters** instead of joining together.

### Evidence
```
Pod cockroachdb-fgrbw: Node 1 of Cluster A (clusterID: 15152a17-fcc9-4a51-8c04-af0ec0126551)
Pod cockroachdb-nbm5k: Node 1 of Cluster B (clusterID: 2c4d3ec6-47f5-4161-97ff-ef4b12f595f0)
```

### Impact
- **No replication**: Data written to one pod is NOT visible on the other
- **No fault tolerance**: If one pod dies, its data is lost
- **Intermittent failures**: Load balancer routes to different "clusters" causing auth failures
- **Data integrity risk**: Two independent databases masquerading as one cluster

---

## 2. Root Cause Analysis

### The Race Condition

The CockroachDB Operator creates resources in this order:
1. `CrdbCluster` CRD is created by Helm
2. Operator begins reconciliation, creates pods
3. Pods start with `--join cockroachdb-join.prod.svc.cluster.local:26258`
4. **PROBLEM**: The `cockroachdb-join` Service doesn't exist yet
5. DNS lookup fails: `lookup cockroachdb-join.prod.svc.cluster.local: no such host`
6. After ~7 seconds of retries, CockroachDB gives up and initializes a NEW cluster
7. Second pod does the same → TWO separate clusters
8. Operator finally creates `cockroachdb-join` Service (too late)

### Log Evidence from Affected Pod

```
# Pod startup attempting to join
+ exec /cockroach/cockroach start ... --join cockroachdb-join.prod.svc.cluster.local:26258

# DNS failures (service doesn't exist yet)
W251219 16:32:06.930975 server/init.go:375 outgoing join rpc to cockroachdb-join.prod.svc.cluster.local:26258 unsuccessful:
rpc error: code = Unavailable desc = "transport: error while dialing: dial tcp: lookup cockroachdb-join.prod.svc.cluster.local on 10.96.0.10:53: no such host"

W251219 16:32:07.944980 server/init.go:421 outgoing join rpc... unsuccessful: no such host

# After ~2 seconds of failures, pod gives up and creates NEW cluster
I251219 16:32:08.847335 server/server.go:1888 connecting to gossip network to verify cluster ID "7b187288-e725-4ce8-a1e2-a0ee115d494e"
status: initialized new cluster
clusterID: 7b187288-e725-4ce8-a1e2-a0ee115d494e
nodeID: 1
```

### Why This Happens

The CockroachDB Operator creates the `cockroachdb-join` headless Service **reactively** after processing the `CrdbCluster` CRD. However, by the time the Service is created, the pods have already started and failed to resolve DNS.

CockroachDB has a short retry window (~7 seconds by default) before it decides "I can't find any peers, I'll initialize myself as a new cluster."

---

## 3. The Fix

### Approach: Pre-create Services Before Pods Start

We add two Helm templates that create the headless Services **before** the `CrdbCluster` CRD is processed, using Helm hooks.

### File 1: `service.join.yaml`

```yaml
# Pre-create the join service before CrdbCluster to prevent race condition
# The operator will take ownership of this service but it needs to exist for DNS
apiVersion: v1
kind: Service
metadata:
name: {{ template "cockroachdb.fullname" . }}-join
namespace: {{ .Release.Namespace }}
labels:
crdb.cockroachlabs.com/cluster: {{ template "cockroachdb.fullname" . }}
{{- include "cluster.labels" . | nindent 4 }}
annotations:
argocd.argoproj.io/compare-options: IgnoreExtraneous
helm.sh/hook: pre-install,pre-upgrade
helm.sh/hook-weight: "-10"
spec:
type: ClusterIP
clusterIP: None
publishNotReadyAddresses: true
ports:
- name: sql
port: 26257
targetPort: sql
- name: grpc
port: 26258
targetPort: grpc
- name: http
port: 8080
targetPort: http
selector:
crdb.cockroachlabs.com/cluster: {{ template "cockroachdb.fullname" . }}
```

### File 2: `service.headless.yaml`

```yaml
# Pre-create the headless service before CrdbCluster
apiVersion: v1
kind: Service
metadata:
name: {{ template "cockroachdb.fullname" . }}
namespace: {{ .Release.Namespace }}
labels:
crdb.cockroachlabs.com/cluster: {{ template "cockroachdb.fullname" . }}
{{- include "cluster.labels" . | nindent 4 }}
annotations:
argocd.argoproj.io/compare-options: IgnoreExtraneous
helm.sh/hook: pre-install,pre-upgrade
helm.sh/hook-weight: "-10"
service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
spec:
type: ClusterIP
clusterIP: None
publishNotReadyAddresses: true
ports:
- name: sql
port: 26257
targetPort: sql
- name: grpc
port: 26258
targetPort: grpc
- name: http
port: 8080
targetPort: http
selector:
crdb.cockroachlabs.com/cluster: {{ template "cockroachdb.fullname" . }}
```

---

## 4. Technical Justification

### Why Helm Hooks?

| Annotation | Purpose |
|------------|---------|
| `helm.sh/hook: pre-install,pre-upgrade` | Ensures Service exists BEFORE main chart resources (CrdbCluster) are created |
| `helm.sh/hook-weight: "-10"` | Negative weight = runs early in hook sequence |

### Why These Specific Service Configurations?

| Field | Value | Reason |
|-------|-------|--------|
| `clusterIP: None` | Headless | Required for StatefulSet pod DNS discovery |
| `publishNotReadyAddresses: true` | Critical | Allows DNS to return pod IPs even before pods pass readiness checks. This is essential because pods need to discover each other during startup. |
| `selector: crdb.cockroachlabs.com/cluster` | Matches Operator | Uses the same label the Operator uses, so the Service will select CockroachDB pods |

### Why ArgoCD Annotation?

```yaml
argocd.argoproj.io/compare-options: IgnoreExtraneous
```

The Operator will eventually "adopt" these Services and add its own annotations/labels. Without `IgnoreExtraneous`, ArgoCD would constantly show drift and try to revert the Operator's changes.

### Why This Doesn't Break Anything

1. **Selector matches exactly**: The selector `crdb.cockroachlabs.com/cluster: cockroachdb` is what the Operator uses
2. **Ports match exactly**: 26257 (SQL), 26258 (gRPC), 8080 (HTTP) are standard CockroachDB ports
3. **Operator can still manage**: The Operator will update the Service if needed; we just ensure it exists first
4. **Idempotent**: If Service already exists, Helm hooks handle this gracefully

---

## 5. Why Modify the Official Chart?

### Arguments FOR Modification

1. **This is a bug in the chart**: The chart should work out-of-the-box for multi-node deployments
2. **No upstream fix available**: The official 25.4.2-preview chart doesn't address this
3. **Minimal invasive change**: We only ADD two template files; we don't modify existing ones
4. **Standard Kubernetes patterns**: Pre-creating Services before StatefulSets is a well-known pattern
5. **Documented CockroachDB issue**: The "join service resolves to own IP" problem is acknowledged in operator discussions

### Arguments AGAINST (and Rebuttals)

| Concern | Rebuttal |
|---------|----------|
| "Don't modify vendor charts" | This is a critical bug fix, not a customization. The chart is broken without it. |
| "Upstream may break compatibility" | Our templates use stable APIs (v1 Service) and Helm hooks that have existed for years. |
| "Operator should handle this" | The Operator creates Services reactively, not proactively. This is a design limitation. |
| "Use a different approach" | Init containers waiting for DNS still fail if Service doesn't exist at all. Pre-creating Services is the only reliable fix. |

---

## 6. Proof That The Fix Works

### Before Fix (Failed Deployment)

```bash
$ kubectl exec cockroachdb-fgrbw -- cockroach node status
id address
1 cockroachdb-fgrbw.cockroachdb.prod.svc.cluster.local:26258 # Only sees itself

$ kubectl exec cockroachdb-nbm5k -- cockroach node status
id address
1 cockroachdb-nbm5k.cockroachdb.prod.svc.cluster.local:26258 # Only sees itself (DIFFERENT CLUSTER!)
```

### After Fix (Successful Deployment)

```bash
$ kubectl exec cockroachdb-rgdlv -- cockroach node status
id address locality
1 cockroachdb-rgdlv.cockroachdb.prod.svc.cluster.local:26258 zone=fsn1
2 cockroachdb-sfcnz.cockroachdb.prod.svc.cluster.local:26258 zone=hel1

$ kubectl exec cockroachdb-sfcnz -- cockroach node status
id address locality
1 cockroachdb-rgdlv.cockroachdb.prod.svc.cluster.local:26258 zone=fsn1
2 cockroachdb-sfcnz.cockroachdb.prod.svc.cluster.local:26258 zone=hel1
```

**Both nodes see each other. Single cluster ID: `83ef2889-085b-426d-865e-013bbe923741`**

### Replication Test

```bash
# Write to node 1 (fsn1)
$ kubectl exec cockroachdb-rgdlv -- cockroach sql -e "INSERT INTO replication_test (value) VALUES ('test-data');"
INSERT 1

# Read from node 2 (hel1) - DIFFERENT physical server
$ kubectl exec cockroachdb-sfcnz -- cockroach sql -e "SELECT * FROM replication_test;"
id value created_at
1134074409334800385 test-data 2025-12-19 16:40:43.859526+00
```

**Data written to fsn1 (Germany) immediately visible on hel1 (Finland). Replication works.**

---

## 7. Files Modified

| File | Action | Risk |
|------|--------|------|
| `charts/cockroachdb/templates/service.join.yaml` | CREATED | Low - additive only |
| `charts/cockroachdb/templates/service.headless.yaml` | CREATED | Low - additive only |
| `values.yaml` | MODIFIED | Low - added optional init container |

---

## 8. Alternative Approaches Considered

### Option A: Init Container That Waits for DNS
**Rejected**: If the Service doesn't exist, DNS will never resolve. The init container would wait forever or timeout.

### Option B: Increase CockroachDB Join Timeout
**Rejected**: No documented way to configure this in the Operator. Would require modifying CockroachDB startup flags, which is fragile.

### Option C: Deploy Services Separately Before Chart
**Rejected**: Breaks the "single Helm install" workflow. Requires manual ordering of kubectl commands.

### Option D: Pre-create Services in Helm Chart (CHOSEN)
**Accepted**: Uses standard Helm hooks, minimal code, proven to work, no external dependencies.

---

## 9. Compatibility Notes

- **Helm version**: Tested with Helm 3.x (hooks have been stable since Helm 2)
- **Kubernetes version**: v1 Service API is stable since Kubernetes 1.0
- **ArgoCD**: Compatible with annotation for ignoring drift
- **CockroachDB Operator**: Works with 25.4.2-preview (operator will adopt pre-created Services)

---

## 10. Conclusion

The modifications to the CockroachDB Helm chart are:
1. **Necessary**: The chart has a race condition bug that prevents proper cluster formation
2. **Minimal**: Only 2 new template files added, using standard Kubernetes/Helm patterns
3. **Safe**: Uses stable APIs, doesn't modify existing templates, operator-compatible
4. **Proven**: Successfully deployed and tested with data replication verified

This fix should be considered for upstream contribution to the official CockroachDB Operator Helm chart.

---

## 11. Response to Documentation Review

This section addresses concerns raised during independent technical review.

### Concern 1: Service Names and Labels Are Not Documented

**Status: VALIDATED BY EMPIRICAL EVIDENCE (VERSION-SPECIFIC)**

The reviewer correctly noted that `cockroachdb-join` and `crdb.cockroachlabs.com/cluster` are not in official documentation. However, live cluster inspection confirms these are the actual values for version 25.4.2-preview:

```bash
$ kubectl get svc -n prod -l 'crdb.cockroachlabs.com/cluster=cockroachdb'
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S)
cockroachdb ClusterIP None 26257/TCP,26258/TCP,8080/TCP
cockroachdb-join ClusterIP None 26257/TCP,26258/TCP,8080/TCP

$ kubectl get pods -n prod -o jsonpath='{.items[0].spec.containers[0].command}' | grep join
--join cockroachdb-join.prod.svc.cluster.local:26258
```

**Conclusion**: These names/labels are taken from the current **25.4.2-preview** chart. They are technically correct but represent implementation details, not a documented contract. If the operator changes them in future versions, these hook templates must be updated accordingly.

### Concern 2: Dual Ownership Between Helm and Operator

**Status: REASONABLE BUT UNDOCUMENTED**

The Operator has adopted the pre-created Services and added `ownerReferences`:

```yaml
ownerReferences:
- apiVersion: crdb.cockroachlabs.com/v1alpha1
controller: true
kind: CrdbCluster
name: cockroachdb
```

This leverages the same "adopt existing Service" pattern that Cockroach Labs uses in their [migration documentation](https://www.cockroachlabs.com/docs/stable/migrate-cockroachdb-kubernetes-operator#step-5-update-the-crdbcluster-manifest), where existing Services are annotated and labeled for Helm adoption. However, this specific dual-ownership scenario (Helm hooks + Operator reconciliation) is not explicitly documented for the preview operator chart.

The `argocd.argoproj.io/compare-options: IgnoreExtraneous` annotation prevents GitOps drift detection from conflicting with Operator management.

### Concern 3: Preview Status Means Internals Could Change

**Status: ACCEPTED WITH MITIGATION**

Valid concern. This is a Preview operator with no guarantees about internal behavior stability.

| Potential Change | Impact | Mitigation |
|-----------------|--------|------------|
| Service name changes | Duplicate Services created | Easy to detect, update templates |
| Selector changes | Services don't match pods | Monitor pod selection, unlikely given CRD-based design |
| Operator fixes the bug | Pre-created Services become no-ops | Harmless redundancy |

Given current behavior, the most likely downside is redundant/no-op Services. Because this is a Preview operator, we treat this as a **version-specific workaround** and will re-validate after every operator upgrade. The fix uses stable Kubernetes v1 APIs.

### Concern 4: Not Officially Endorsed

**Status: ACKNOWLEDGED**

This is a workaround for an undocumented bug in a preview chart. It is:
- Consistent with CockroachDB's documented networking patterns
- Compatible with Cockroach Labs' "adopt existing Services" pattern (per migration docs)
- Pragmatically necessary for production deployments

**Recommendation**: Submit upstream bug report with logs showing race condition, propose that Operator reconcile Services before creating pods.

---

## 12. Required Operational Safeguards

This workaround is approved for production use with the following mandatory safeguards:

### 12.1 Version Tracking

| Component | Current Version | Last Validated | Re-validate |
|-----------|-----------------|----------------|-------------|
| CockroachDB Operator Helm Chart | 25.4.2-preview | 2025-12-19 | Service names, labels, reconciliation order |
| CockroachDB Image | v25.4.2 | 2025-12-19 | `cockroach node status` CLI behavior |

**Action Required**: Re-validate this workaround after every Operator/Helm chart upgrade.

### 12.2 Cluster Health Monitoring

Detect "split cluster" failure by verifying all pods see all other pods:

```bash
# Detection script - uses documented 'cockroach node status' command
# Each pod should see EXPECTED_NODES nodes; if any sees fewer, cluster may be split
EXPECTED_NODES=2
NAMESPACE=prod

# Get all CockroachDB pod names
PODS=$(kubectl get pods -n $NAMESPACE -l crdb.cockroachlabs.com/cluster=cockroachdb \
-o jsonpath='{range .items[*]}{.metadata.name}{"\n"}{end}')

FAILED=0
for POD in $PODS; do
NODE_COUNT=$(kubectl exec -n $NAMESPACE $POD -c cockroachdb -- \
cockroach node status --certs-dir=/cockroach/cockroach-certs 2>/dev/null | \
tail -n +2 | wc -l)

if [ "$NODE_COUNT" -lt "$EXPECTED_NODES" ]; then
echo "CRITICAL: Pod $POD only sees $NODE_COUNT nodes (expected $EXPECTED_NODES)"
FAILED=1
fi
done

if [ "$FAILED" -eq 1 ]; then
echo "Cluster integrity check FAILED - possible split cluster"
exit 1
fi
echo "OK: All pods see $EXPECTED_NODES nodes"
```

**Optional**: For absolute verification, perform a write-then-read test across pods:

```bash
# Write to pod 1, read from pod 2 - if data appears, same cluster
kubectl exec -n prod cockroachdb-0 -c cockroachdb -- cockroach sql \
--certs-dir=/cockroach/cockroach-certs \
-e "CREATE TABLE IF NOT EXISTS health_check (id INT PRIMARY KEY, ts TIMESTAMP DEFAULT now());
UPSERT INTO health_check (id) VALUES (1);"

kubectl exec -n prod cockroachdb-1 -c cockroachdb -- cockroach sql \
--certs-dir=/cockroach/cockroach-certs \
-e "SELECT * FROM health_check WHERE id = 1;"
```

### 12.3 Upstream Issue Tracking

**TODO**: File bug report at https://github.com/cockroachdb/cockroach-operator/issues with:
- Title: "Race condition: pods initialize separate clusters when join service doesn't exist at startup"
- Include: Log evidence showing DNS failures and `initialized new cluster` messages
- Proposed fix: Operator should create Services before pods, or use init container to wait

---

## 13. Final Verdict

**Classification**: Version-specific production workaround

| Aspect | Status |
|--------|--------|
| Technically correct | ✓ Matches actual Operator implementation |
| Aligned with CockroachDB K8s patterns | ✓ Headless Services, correct ports |
| Uses documented adoption pattern | ✓ Same as migration docs |
| Officially supported | ✗ Relies on undocumented internals |
| Permanent solution | ✗ Must re-validate on upgrades |

### Undocumented Dependencies (Minimum Viable)

| Dependency | Used In | Risk | Alternative Exists? |
|------------|---------|------|---------------------|
| `cockroachdb-join` service name | Helm hook templates | May change in future Operator versions | ❌ No (verified by docs AI) |
| `crdb.cockroachlabs.com/cluster` label | Helm hook selectors | May change in future Operator versions | ❌ No (verified by docs AI) |

**Docs AI Verification**: Exhaustively searched for documented alternatives. The CrdbCluster CRD does not expose fields to configure service names, service selectors, join addresses, or pre-reconcile hooks. These two dependencies are **unavoidable** given the current operator design.

**Note**: Monitoring uses `cockroach node status` (documented CLI) — zero undocumented functions in monitoring.

**Approved for production** with the understanding that this is a pragmatic workaround for a Preview operator bug, not a documented configuration. All undocumented dependencies must be re-validated on version upgrades.

---

## 14. Final Approval Request

### Problem
CockroachDB Operator Helm chart (25.4.2-preview) has a race condition where pods start before the `cockroachdb-join` Service exists, causing nodes to initialize as separate clusters instead of joining.

### Chosen Solution
Pre-create headless Services using Helm `pre-install,pre-upgrade` hooks so DNS resolves before pods start.

### Files Added to Helm Chart
```
charts/cockroachdb/templates/service.join.yaml # Pre-creates cockroachdb-join Service
charts/cockroachdb/templates/service.headless.yaml # Pre-creates cockroachdb Service
```

### Documented Patterns Used
| Pattern | Source |
|---------|--------|
| Headless Services with `clusterIP: None` | [Local K8s deployment docs](https://www.cockroachlabs.com/docs/stable/deploy-cockroachdb-with-kubernetes-insecure#step-2-start-cockroachdb) |
| Ports 26257, 26258, 8080 | [Network ports docs](https://www.cockroachlabs.com/docs/stable/configure-cockroachdb-operator#network-ports) |
| `publishNotReadyAddresses: true` | Standard K8s pattern for StatefulSet discovery |
| Adopt existing Services | [Migration docs step 5](https://www.cockroachlabs.com/docs/stable/migrate-cockroachdb-kubernetes-operator#step-5-update-the-crdbcluster-manifest) |

### Undocumented Dependencies Accepted
| Dependency | Justification | Docs AI Verified |
|------------|---------------|------------------|
| `cockroachdb-join` service name | No CRD field to configure service names | ✓ No alternative exists |
| `crdb.cockroachlabs.com/cluster` label | No CRD field to configure Service selectors | ✓ No alternative exists |

**Docs AI Confirmation**: Searched for alternatives including CRD fields, `startFlags`, alternative discovery mechanisms, pre-reconcile hooks, and using `cockroachdb-public` for join. **None are documented.** These two dependencies represent the **minimum unavoidable reliance** on implementation details.

**Monitoring uses documented CLI** (`cockroach node status`) — no undocumented SQL functions required.

### Safeguards Committed
1. ✓ Version tracking with explicit re-validation requirements
2. ✓ Monitoring scripts for split-cluster detection
3. ✓ Node count verification
4. ✓ Upstream bug report to be filed

### Classification
**Version-specific production workaround** — not a permanent solution, not officially supported, must be re-validated on every Operator/CockroachDB upgrade.

### Request
Approve this as the optimal available solution for a Preview operator race condition, given:
- No documented alternative exists
- Fix uses documented K8s/CockroachDB patterns where possible
- All undocumented dependencies are explicitly tracked
- Safeguards are in place to detect regression

---

## Appendix: Quick Reference Commands

```bash
# Verify cluster formation
kubectl exec -n prod -c cockroachdb -- cockroach node status --certs-dir=/cockroach/cockroach-certs

# Check for DNS resolution issues in logs
kubectl logs -n prod -c cockroachdb | grep -i "join\|no such host\|initialized"

# Verify services exist before pods
kubectl get svc -n prod | grep cockroach

# Test replication
kubectl exec -n prod -- cockroach sql --url='...' -e "INSERT INTO test (v) VALUES ('x');"
kubectl exec -n prod -- cockroach sql --url='...' -e "SELECT * FROM test;"
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.