ceph / ceph/ceph-csi

journal: the RADOS namespace on journal.Config has a race condition

Open
#6,551 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.