journal: the RADOS namespace on journal.Config has a race condition
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 617
- Avg merge
- 5d 10h
- Merged PRs (30d)
- 43
Description
# Describe the bug
The namespace of each request gets written into a shared journal object. This is no problem if everything goes into a single namespace, but that may not be the case.
If two CreateVolume calls (other operations too) are issued to ceph-csi and they access two different namespaces, the last namespace will be used by both created volumes.
If you look at the code this behavior becomes obvious. cj.namespace lives across all operations (shared global state) but it gets set by a local context every time Connect() is called.
https://github.com/ceph/ceph-csi/blob/fade382fd1b2b23e3cff87fea0a958d85ec39873/internal/journal/voljournal.go#L262-L276
It's a journal bug affecting rbd, cephfs, nfs and nvmeof.
## Example of the race condition
```mermaid
sequenceDiagram
autonumber
participant a as CreateVolume for clusterID-1
participant b as ExpandVolume for clusterID-2
participant cfg as store.VolJournal
one shared journal.Config
participant r as RADOS
a->>cfg: Connect with namespace ns-1
Note over cfg: cj.namespace = ns-1
b->>cfg: Connect with namespace ns-2
Note over cfg: cj.namespace = ns-2
a->>cfg: read cj.namespace
cfg-->>a: ns-2
a->>r: setOMapKeys in ns-2
Note over r: the clusterID-1 journal entry lands
in the clusterID-2 namespace
```
# Reproducer
Reproduced on ceph-csi **v3.16.2** (Rook / `ceph-csi-operator` deployment), Ceph 20.2.2, Kubernetes v1.36.1. `Connect` is unchanged at v3.16.3, v3.17.1 and devel (a06264b5e), so this is not already fixed.
Setup, run, and what came out of it
Two `clusterID` entries against **one** CephFS. `cluster-a` leaves `cephFS.radosNamespace` unset, so it defaults to `csi`. `cluster-b` sets it to `csi-tenant1` *and* names a `subvolumeGroup` that does not exist — so its `CreateVolume` reserves its journal entry, fails at subvolume creation, and retries forever, calling `Connect(…, "csi-tenant1")` on a loop.
```yaml
---
apiVersion: v1
kind: ConfigMap
metadata:
name: ceph-csi-config
data:
config.json: |-
[
{
"clusterID": "cluster-a",
"monitors": ["10.0.0.1:6789"],
"cephFS": {}
},
{
"clusterID": "cluster-b",
"monitors": ["10.0.0.1:6789"],
"cephFS": { "radosNamespace": "csi-tenant1", "subvolumeGroup": "does-not-exist" }
}
]
---
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
name: sc-a
provisioner: cephfs.csi.ceph.com
reclaimPolicy: Delete
parameters:
clusterID: cluster-a
fsName: myfs
pool: myfs-replicated
csi.storage.k8s.io/provisioner-secret-name: csi-cephfs-secret
csi.storage.k8s.io/provisioner-secret-namespace: default
csi.storage.k8s.io/controller-expand-secret-name: csi-cephfs-secret
csi.storage.k8s.io/controller-expand-secret-namespace: default
csi.storage.k8s.io/node-stage-secret-name: csi-cephfs-secret
csi.storage.k8s.io/node-stage-secret-namespace: default
---
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
name: sc-b
provisioner: cephfs.csi.ceph.com
reclaimPolicy: Delete
parameters:
clusterID: cluster-b
fsName: myfs
pool: myfs-replicated
csi.storage.k8s.io/provisioner-secret-name: csi-cephfs-secret
csi.storage.k8s.io/provisioner-secret-namespace: default
csi.storage.k8s.io/controller-expand-secret-name: csi-cephfs-secret
csi.storage.k8s.io/controller-expand-secret-namespace: default
csi.storage.k8s.io/node-stage-secret-name: csi-cephfs-secret
csi.storage.k8s.io/node-stage-secret-namespace: default
```
Six PVCs per StorageClass, applied in one go:
```bash
for i in $(seq 1 6); do for sc in sc-a sc-b; do cat <` objects present in **both** namespaces — a single `ReserveName` torn across two, since it re-reads `cj.namespace` between RADOS round-trips.
The subvolumes themselves all landed correctly in the `csi` subvolume group, which comes from the per-request volume options rather than the journal `Config`. Only the journal is misplaced, so the filesystem and its journal disagree.
I did not separately run the RBD path, but it reaches the same two globals with `rbd.radosNamespace`, which unlike the CephFS field has no default.
# Suggested Fix
The expected behavior is that every namespace passed to `Connect` governs every RADOS access through the returned `Connection`, regardless of what other
requests are doing. Since it is per-operation state, it should not be reachable through state shared between operations.
To do that I would make namespace `Connection`-scoped.
# Relation to #6531
This has to be fixed before #6531's NFS item, else fixing that will break NFS. Every NFS `Connect()` call passes the same constant namespace today. Resolving `cephFS.radosNamespace` per clusterID (which fixes #6531's NFS issue), would trigger the namespace `Connect()` race condition.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in internal/journal/voljournal.go around lines 262-276, then trace Connect and the journal Config and Connection types. Make the namespace belong to each returned Connection rather than shared operation state, and verify that concurrent operations keep every RADOS access in the namespace passed to their own Connect call across the affected journal paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100