Add a workaround for https://github.com/rook/rook/issues/18112.
This is not a proper fix but is a quick fix that will prevent the race
involving operator reload and `ClusterController`'s near-global
context.Context from allowing multiple simultaneous CephCluster
reconciles of the same cluster object.
There may be other ways in which a CephCluster reconcile could enter
this race, but this workaround should reliably resolve the entry after
admin cephx key rotation.
Possible entry after this workaround is in place could involve systems
where the rook-ceph-operator pod is running on a node with high CPU load
which might prevent an ongoing CephCluster reconcile from getting CPU
cycles needed to exit following context cancellation before the sleep
added in this workaround.
Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>
Two call sites build a shell command by interpolating values that were
never shell-quoted, so a value holding a metacharacter is executed
rather than passed along: RadosNamespaceHasObjects joins the finalized
rados argument vector with spaces, and CopyLocalFileToContainer
interpolates the destination path into a redirect.
Neither has misbehaved so far because rook composes both values itself,
not because the inputs are constrained to shell-safe characters.
CopyLocalFileToContainer receives a temp-file path. The rados probe
receives a "<metadataPoolName>:<zone><suffix>" string assembled by
adjustZoneDefaultPools, whose pool half comes from the default pool
placement's metadataPoolName or from spec.sharedPools.metadataPoolName,
neither of which carries pattern validation, or from the zone JSON of a
zone rook adopted rather than created.
Quote both with FormatCommand.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
A command logged by the exec package could not be copied out of the log
and re-run. Creating the RGW admin ops user, for example, logged an
unquoted "--display-name RGW Admin Ops User" and an unquoted "--caps
accounts=*;buckets=*;users=*;usage=read;metadata=read;zone=read", which
a shell reads as four arguments and six commands respectively. Render
these lines with FormatCommand instead.
cmd-reporter also named the command twice, because exec.Cmd already
carries argv[0] in Args and the log line prefixed Path to the whole of
Args.
The startup line drops the quotes that wrapped the entire argument list,
now that each argument carries its own; both in-tree samples of that
line are updated to match.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
Command arguments are passed to the kernel as a vector, so joining them
with plain spaces loses the boundaries between them. An argument holding
whitespace or a shell metacharacter reads back as several arguments, or
as several commands.
FormatCommand renders a command and its arguments as a single line a
POSIX shell reads back as the original vector. Quoting is minimal: an
argument is single-quoted only when it contains a character the shell
would not pass through literally, so the flag-heavy command lines rook
builds stay readable.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
The same 'split on / and prepend /dev when there is no separator' block
appeared five times in this file: GetDevicePartitions, GetDeviceProperties,
GetUdevInfo, GetDeviceFilesystems and GetDiskUUID. GetUdevInfo was the one
that had it wrong, which is what the previous commit fixed.
Move it into resolveDevicePath so there is a single definition to reason
about, and document why the check exists: a caller may pass a bare kernel
name, a device node, or the mount point of a PVC-backed block device, and
only the first needs the prefix.
No behaviour change. GetDevicePartitions still compares lsblk output against
the unmodified device name, so only the path passed to lsblk is resolved.
Signed-off-by: Somanchi Poorna Sobhita <somanchi004@gmail.com>
GetUdevInfo always formatted its argument as "/dev/%s", but it is called
both with a kernel name ("sdb") and with a full device path ("/dev/sdb").
In the second case udevadm is invoked on "/dev//dev/sdb" and fails, so
GetUdevInfo returns an error and DevLinks is never populated.
The neighbouring GetDeviceFilesystems, GetDiskUUID and GetDeviceProperties
all guard the prefix this way already; GetUdevInfo is the only one of the
four that does not.
This is reached from getOSDInfoFromCV via PopulateDeviceUdevInfo, where the
call site comment states the intent is to "populate udev DevLinks to match
CR device fullpath entries". The same blockPath is also passed to
PopulateDeviceInfo, which handles an absolute path correctly. Because
findDeviceClass only consults DEVLINKS when devLinks != "", that branch can
never match, and a per-device deviceClass set on a /dev/disk/by-id/... entry
in the CephCluster CR is silently ignored in favour of the disk-derived
class.
The failure is only logged as a warning, so it degrades silently rather than
failing the OSD.
Existing tests could not catch this: the udevadm mocks return a fixed value
for any input, and one matches with strings.Contains, which a doubled prefix
still satisfies. The new test asserts the exact argument.
Signed-off-by: Somanchi Poorna Sobhita <somanchi004@gmail.com>
If osd-replacement fails during provisioning of the new device by the
osd-prepare job (can be the case if the new device is faulty), then
ceph-volume will run its internal rollback logic, which purges the
reserved osd id and its CRUSH position. After the faulty device is
replaced with a working one, rook will reprovision it correctly and
reuse the metadata device slot, but as a new OSD, so data will likely
be rebalanced. Ceph hands out the lowest free id, so the new OSD often
takes the same number back, and then the create path recreates the
deployment by itself. Otherwise, and until a working device arrives,
the downscaled deployment of the destroyed osd is left behind. This
commit handles that case by deleting downscaled ready-for-swap osd
deployments without a corresponding OSD in the osd dump.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
The prepare job runs "ceph-volume raw prepare" per device and then trusts
a no-argument "ceph-volume raw list" to report the OSDs it just created.
That listing can come back without a freshly prepared device. On Ceph
v20.2.0 and v20.2.1, get_devices() enumerates through udev data under
/run/udev/data and returns nothing when that data is empty or not yet
populated (https://tracker.ceph.com/issues/77968). On every released
v20.2.x, a block device smaller than 4KiB in the scan, such as the 1KiB
extended-partition node of an MBR-partitioned OS disk, crashes the
batched ceph-bluestore-tool call behind the listing, which ceph-volume
reports as an empty result (https://tracker.ceph.com/issues/76354).
Either way the no-argument list comes back empty or incomplete even
though the OSD exists. Rook then silently reported fewer OSDs than it
prepared, the operator never created the missing OSD deployment, and the
cluster was left with OSDs in the osdmap but no daemons running.
Verify that every device prepared in raw mode is present in the listing.
For any that is missing, list it by name ("ceph-volume raw list
<device>"), which is immune to both failure modes and reports the OSD.
If even the per-device listing is empty, return an error so the prepare
job exits non-zero and Kubernetes restarts it with a freshly re-scanned
environment, rather than silently under-reporting the OSDs.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
osd-replacement was relying on exising ceph.rook.io/do-not-reconcile
label for fencing osd destroy process owned by osd health goroutine from
controller. However, this label can be used but other components like
rook krew maintenance plugin and cannot be owned by osd-replacement
process. Added a separate osd.rook.io/replace-in-progress annotation for
that purpose.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
if the account cr has a force delete annotation, forcefully
remove the account, even if it contatins data in it, using the
purge data flag
Signed-off-by: parth-gr <partharora1010@gmail.com>
Added failureDomains and osdsPerDomain fields to the
ErasureCodedSpec, which maps to the crush-num-failure-domains and
crush-osds-per-failure-domain Ceph EC profile parameters. This enables
creating EC pools that distribute chunks across fewer, larger hosts
without needing one host per data chunk.
Signed-off-by: Prabhala Tara Aasrita <taraprabhala@Prabhalas-MacBook-Pro.local>
The daemon only flagged a device change from in-use to empty, never the
reverse, so an in-use disk was never recorded as non-empty and a later
disk swap produced no ConfigMap update or reconcile. This is needed for
OSD replacement to work automatically: without it a destroyed OSD slot is
never reprovisioned after the disk is swapped. Compare the empty state in
both directions so the swap becomes a real transition that triggers the
reconcile.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
in case of multiple destroyed OSDs or multiple available
blank devices, Rook will try to match them by the same
device class if possible. If no matching DC, then DC of
destroyed OSD will be used
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
Destroyed OSDs awaiting replacement from osd-replacement flow should
be ignored by managed PDB to avoid the following situation:
replaced OSD is already destroyed, its deployment is downscaled zero
and it awaits the physical swap (can take days). This situation does
not trigger blocking PDB in happy case because OSD does not holds PGs.
However, if PGs go unclean during that window for an unrelated reason,
the destroyed OSD gets counted as a down OSD and can grab the single
"draining failure domain" slot - putting blocking PDBs on every other FD.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
A state machine for osd destroy phase of osd-replacement flow.
The state machine is implemented in OSD health goroutine, where each
step is handled on a separate tick. State machine is responsible
for draining, destroying OSD, and reserving its CRUSH position,
downscalling its deployment, and adding "readyForSwap" annotation.
Destroy phase does not zap the device on purpose. On disk signature
is kept to avoid automatic reprovisioning of destroyed OSD and to
detect physical swap.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
defines a new entry point "close-encrypted-devices" in osd job.
The job is used by osd-replacement flow to close dm-crypt mappings
on host for destroyed encrypted OSDs. The job is owned by OSD
health goroutine implementing destroy phase of osd-replacement.
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
contains osd prepare job edits to support osd replacement flow:
- removes destroyed OSD IDs from node status CM to stop cluster
controller from reprovisioning destroyed OSD
- adds provisioning logic for OSD replacement to match available
blank devices to destroyed OSD slots
Signed-off-by: Artem Torubarov <artem.torubarov@sap.com>
Add coverage for GetPort defaults and custom values, Service and probe
port wiring, Ganesha NFS_Port config, and create-or-update when
spec.server.port changes on an existing Service.
Signed-off-by: raaizik <raaizik@yahoo.com>
When NFS runs with host networking, port 2049 may already be in use.
Add CephNFS spec.server.port (default 2049), wire it through Ganesha
config, the operator Service, and probes, and regenerate CRDs.
Signed-off-by: raaizik <raaizik@yahoo.com>
the .nvmeof pool was incorrectly tagged with the rbd
application and initialized as an rbd pool. add .nvmeof
to the built-in pool switch so it gets the nvmeof-meta
application tag instead, and skip rbd pool init.
Signed-off-by: Oded Viner <oviner@redhat.com>
if the csi secret is updated or a client profile is updated
we need a re-reconile of rns
So added the watcher for it
Signed-off-by: parth-gr <partharora1010@gmail.com>
retrieveMultisiteZone is meant to gate the object-store reconcile on the
backing Ceph zone existing: it runs "radosgw-admin zone get" and, when
that fails, returns a non-nil error so the caller requeues. That gate is
dead code. The ENOENT check declares an inner err from exec.ExtractExitCode
that shadows the outer command error, and ExtractExitCode returns a nil
error for the ordinary exit failures radosgw-admin produces. Both the
ENOENT branch and the else branch then wrap that shadowed nil, and
errors.Wrapf(nil, ...) is nil, so the function returns
(waitForRequeueIfObjectStoreNotReady, nil). The caller only propagates the
requeue when the error is non-nil, so the requeue is dropped and reconcile
runs on.
The result is that a failed "zone get" no longer backs off. Reconcile
proceeds to stand up the object store anyway -- the RGW service, the
admin-ops endpoint, the deployment, and the pools radosgw scaffolds as it
comes up -- for a multisite store whose backing zone does not exist. This
is not the "normal multisite bootstrap" transient the original wording
suggested. getMultisiteResourceNames runs immediately before this and
already requeues until the CephObjectZone CR reports Ready, and the zone
controller marks it Ready only after it has created the Ceph zone, so a
healthy bootstrap never reaches this gate with a missing zone. That
CR-Ready gate, not this one, is what actually blocks bootstrap.
Where the dead gate does bite is the cases the CR status cannot cover:
- the Ceph zone deleted or renamed out of band while the CR still reads Ready
- a zone controller that reports Ready without leaving a usable zone behind
- any non-ENOENT "zone get" failure, e.g. a permission or connectivity
error, which the else branch swallows the same way
The check was correct when fc579f4520 introduced it in 2020 with
exec.ExitStatus, which returns (code, ok) and leaves err unshadowed.
bd58790c31 ("ceph: proxy ceph commands when multus is configured", 2021)
swapped it to exec.ExtractExitCode as an unrelated drive-by, inverting the
contract and killing the gate. The sibling realm, zonegroup, and zone
controllers were left untouched and still use exec.ExitStatus today.
Restore exec.ExitStatus so the outer error is no longer shadowed, both
branches wrap the real command error, and the caller requeues until the
zone exists -- matching the sibling controllers.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
Changes:
- Use a dedicated .nvmeof pool (via CephBlockPool CR named
builtin-nvmeof) for the NVMe-oF gateway internal state.
- Use a separate nvmeof pool for the StorageClass data.
- Remove the pool field from the CephNVMeOFGateway CRD.
The gateway now always uses the .nvmeof pool, hardcoded
in the operator.
- Add .nvmeof to the allowed CephBlockPool name overrides.
- Create a production example nvmeof.yaml (instances: 2,
replicas: 3) and a CI-only nvmeof-test.yaml (instances: 1,
replicas: 1).
- Update documentation and CI test script accordingly.
Signed-off-by: Oded Viner <oviner@redhat.com>
Blaine Gardner
Travis Nielsen
Joshua Hoblitt
Joshua Hoblitt
Somanchi Poorna Sobhita
Artem Torubarov
parth-gr
Prabhala Tara Aasrita
dependabot[bot]
raaizik
Oded Viner
Chiman Jain
Anas Khan