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>
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>
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>
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>
startOSDMigration guards against upgrading OSDs while an OSD migration is
still in progress by returning the set of OSDs pending migration, which the
caller then removes from the update queue.
When the last migrated OSD's deployment has not been recreated yet,
isLastOSDMigrationComplete returns (false, nil). startOSDMigration then ran
return nil, errors.Wrapf(err, ...) with err already nil, and
errors.Wrapf(nil, ...) returns nil, so it returned (nil, nil), the same
value it uses to signal that no migration was requested. The caller
therefore skipped removing the pending OSDs from the update queue and could
upgrade and restart them mid-migration, which the pending-migration guard
was meant to prevent.
Return the freshly computed migration config in this case so the caller
still prunes the pending OSDs, without starting a new migration and without
aborting the reconcile. Aborting here would wedge the cluster because the
code that recreates the interrupted OSD runs later in Start, so an early
error return would leave that OSD deleted and block every later reconcile.
Letting the reconcile continue recreates the interrupted OSD and lets the
migration resume on a subsequent reconcile.
Also construct a non-nil error for the sibling unhealthy-PGs guard, which
wrapped a nil err in the same way.
Add a regression test asserting that startOSDMigration returns the pending
migration set, and does not delete another OSD deployment, while a previous
migration is still in progress.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
assignMons schedules each mon in its own goroutine and shares a
failedMonSchedule bool to record whether any of them failed. The
resultLock mutex already guards the shared c.mapping.Schedule map
write, but the three failedMonSchedule = true assignments were done
without holding the lock. When two or more mons fail to schedule at
the same time (for example when waitForMonitorScheduling errors, the
node choice is nil, or getNodeInfoFromNode fails) their goroutines
write the flag concurrently, which is a write-write data race.
Guard the failedMonSchedule writes with the existing resultLock, the
same lock the goroutines already use for the map update. The post-Wait
read is left as is since the WaitGroup orders it after every write.
Add a regression test that fails scheduling for multiple mons at once
and confirms assignMons returns an error; run it with -race to catch
the race.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Port 1024 is the lowest non-privileged port on Linux (privileged ports
are 0-1023), so it should be honored as-is like any port >= 1024. The
dashboardInternalPort guard used `<=` instead of `<`, which pushed a
requested Port of 1024 into the "privileged" branch and reset the
container/targetPort back to the default (7000/8443). That left the
Service Port at 1024 but its targetPort at the default, a mismatch that
contradicts the constant's comment and the identical Port 1025 case.
Change the comparison to `<` and add a boundary case to
TestStartSecureDashboard asserting both Port and targetPort are 1024.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
The capacity-retention case in TestCephStatus asserted
formatTime(now-1m) against formatTime(now-1m), comparing an
expression to itself. That assertion can never fail and never
looks at the value under test, so the branch in
toCustomResourceStatus that keeps the previous capacity when
newStatus.PgMap.TotalBytes is 0 was left unverified.
Seed a known LastUpdated on the current status alongside the
existing TotalBytes, then assert the aggregated status carries
that timestamp through. This checks that the prior capacity,
including its LastUpdated, is retained when total bytes drop to 0.
The sibling cases already assert Capacity.LastUpdated the same way.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
currently, drbd hostname was ready using command
but now the change we use kubernetes built-in api
to read hostname from `spec.nodeName`.`
Signed-off-by: subhamkrai <srai@redhat.com>
shouldReconcileChangedNode only diffed the node spec, so changes to node
labels, including rook's own topology.rook.io/* labels, were never detected.
Relabeling a node therefore did not trigger a CephCluster reconcile until some
other condition eventually did.
Trigger a reconcile from the node update predicate when one of the OSD topology
labels rook cares about changes. The whitelist reuses the canonical label set
from topology.GetDefaultTopologyLabels() (the hostname, the Kubernetes
region/zone labels, and the topology.rook.io/* labels), so a reconcile is
scoped to topology changes instead of firing on any label or annotation change.
The UpdateFunc returns true directly for these changes because onK8sNode()
returns false for a node that is already an OSD host, which would otherwise
swallow the relabel.
Resolves#17652
Signed-off-by: Anas Khan <anas@anaskhan.me>
The //nolint directives at these sites omit the linter name, so each
suppresses every linter on its line rather than the one check it needs.
That hides any unrelated errcheck/gosec/govet finding later introduced
on the same line. Name the specific linter for each:
- staticcheck for the two operator sites: SA4004 (the intentional
single-iteration loop in the OSD PVC host lookup) and SA1019 (the
deliberate read of the deprecated S3.Enabled field in the RGW
API-enable builder).
- errcheck for the rbd-mirror deferred token-file cleanup and the
test-framework logging helpers (WriteString / writeHeader).
No behavior change.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
Several godoc comments led with a stale or incorrect identifier, left
over from renames, exported/unexported changes, copy-paste between
sibling declarations, or plain typos. As a result the documented name no
longer matched the function, method, type, or var it describes. Correct
each leading word to the name of the declaration it documents.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
When the operator starts, there is no need to handle the
node watcher immediately while the controller runtime
cache is being initialized. The cluster will anyway be
fully reconciled and create new OSDs if needed. The node
watcher is only intended for triggering a reconcile when
a node is added or updated at a later time after a sync
would anyway be triggered.
Signed-off-by: Travis Nielsen <tnielsen@redhat.com>
currently k8s add the default eviction time as 300 sec
setting it to 5 sec for the quick re-schedule
Signed-off-by: parth-gr <partharora1010@gmail.com>
When the OSD activate init container cannot match its configured
device, it falls back to a bare 'ceph-volume raw list', which
enumerates every block device on the node and opens each one to read
bluestore labels. Opening a krbd device whose backing cluster is
unhealthy can block in uninterruptible sleep; when that device's I/O
depends on the OSD being activated, activation deadlocks.
Build the fallback scan list explicitly with lsblk and exclude the
network-backed device types that can hang when their backing storage
is unavailable (rbd, nbd, drbd) along with zram (volatile RAM). Rook
never provisions OSDs on any of these, so the fallback can never
legitimately need to find an OSD on them. loop devices are kept,
since OSDs on loop devices are supported for CI and local testing.
Fail fast if the filtered list is empty rather than degenerating back
to a full scan.
Note this hardens only the rename-fallback path. The deadlock observed
in CI (kernel hung tasks under 'ceph-volume raw activate' opening a
stalled /dev/rbd0; actions runs 27325963851, 27333888237, and
27378983111, which reproduced it on this PR's own head) ultimately
originates inside ceph-volume: 'raw activate' runs an unconditional
LUKS2/TPM2 discovery sweep over all block devices even when given an
explicit --device. That requires a ceph-volume fix; this change makes
the Rook-controlled scan safe regardless of ceph version.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
The Clear method on updateQueue had no callers anywhere in the repo
(verified with grep, deadcode, and staticcheck). The queue is emptied via
Remove/Pop in practice. Remove the dead method.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
Add support for the osd.rook.io/device-class node label to assign
CRUSH device classes to all OSDs on a node.
The operator reads the label during OSD provisioning and
deployment reconciliation. If CR-level deviceClass conflicts
with the node label, the operator will skip reconciling the
node and logs an error.
Resolves: #17131
Signed-off-by: Yuxiang He <yuxiang@dropbox.com>
IsModuleInSpec has no callers anywhere in the repository (verified with
deadcode and grep). Remove the dead code.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
applyUpgradeOSDFunctionality() may run before all OSDs have registered
their new version, leaving require-osd-release unset and the cluster in
HEALTH_WARN.
Check for a single converged OSD version on each health-monitor tick and
set require-osd-release when it changes. The release name is cached to
avoid redundant ceph commands
Signed-off-by: Sunnatillo <sunnat.samadov@est.tech>
when using encryptedDevice:true with host based (non pvc) osds, resizing
the underlying disk and restarting the OSD didn't expand the OSD because
the LUKS encryption layer was not resized.
This PR adds resize capabilities inside the activate container. Full
device stack must be resized: PV → LV → LUKS
Steps:
1. Run pvresize and lvextend to grow the LVM layers.
2. Retrieve the LUKS key from the Ceph config-key store using the
lockbox crednetials already available in the activate container
3. Pass the key explicitly to cryptsetup resize via --key-file.
Signed-off-by: Santosh <sapillai@redhat.com>
User RaceFreeFakeWatcher isntead of FakeWatcher in TestAddRemoveNode
function. The test runs c.Start() in a goroutine whch stops the watcher
when done. If mockNodeOrchestrationCompeltion sends a watch ever after,
that, FakeWatcher panics with "send on a closed channel".
RaceFreeFakeWatcher safely ignores the sends on a stopped channel.
Signed-off-by: Santosh <sapillai@redhat.com>
Blaine Gardner
Artem Torubarov
Joshua Hoblitt
Anas Khan
Parth Arora
subhamkrai
Anas Khan
blackflytech
Joshua Hoblitt
pengqima
daixihegu
Travis Nielsen
kavirakesh14
Satoru Takeuchi
Yuxiang He
Sunnatillo
Artem Muterko
Santosh