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>
The go workspace file go.work.sum) were committed to the tree with PR #17925.
The official guide advises against committing them:
https://go.dev/ref/mod#go-work-file
An exception in which committing the go.work file makes sense
is the case when the modules in a repository are developed exclusively with each other
but not together with external modules.
The rook base module and pkg/apis certainly satisfy this criterion.
While this justifies committing go.work, changes to go.work.sum creates
uynreasonable churn in commits.
Therefore, this change removes go.work.sum
again to reduce churn
Signed-off-by: Michael Adam <obnox@samba.org>
Fix a non-ASCII dash in the branch creation example, describe the master
updates needed after a release branch is created (mergify rules, supported
Kubernetes versions, upgrade guide), add the pending release notes reset to
the post-release steps, document the env variables gen_release_notes.sh
requires and the meaning of FROM_BRANCH and TO_TAG, remove the unused rc
tag example, update the example versions and example PRs to the v1.20
releases and link them, and list all of the artifacts a release build
publishes.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.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 rook-ceph-cluster chart can deploy CephObjectStores but offered no
way to create S3 users for them, so CephObjectStoreUser resources had to
be applied out-of-band. Add a cephObjectStoreUsers values array that
renders one CephObjectStoreUser per entry, following the same name plus
spec passthrough pattern as the multisite resources, with helm-unittest
coverage.
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>
The charts have no unit tests. CI runs ct lint plus a helm template and
kustomize build, which prove the charts render but never assert what
they render, so conditional template logic goes unverified.
Add helm-unittest, pinned and installed as a standalone binary, a
test.helm make target, and a Helm Unittests workflow that runs it. The
first suite covers the toolbox deployment: the image composed from the
toolbox and cephImage settings, the precedence of both over the older
toolbox.image setting, the hostNetwork branch driven by the cluster
network provider, and the revisionHistoryLimit guard.
The new job is a required check in .mergify.yml, alongside the existing
chart linting.
Suites live under tests/ in a chart and are excluded from the packaged
chart via .helmignore. The runner is invoked with --strict so that a
misspelled key fails the suite instead of silently rendering defaults.
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>
The release-1.20 branch is running the tests on K8s 1.36.1. Update
the mergify auto-merge rules to match on this version instead
of the previous version 1.35 that was being run.
Signed-off-by: Travis Nielsen <tnielsen@redhat.com>
Nothing verified that the commit being tagged for a release actually
carries the intended version. Tagging a release branch before the version
update PR is merged publishes images and manifests that reference the
previous release. Add build/release/validate-tag.sh to check that the tree
is clean and that deploy/examples/images.txt and the helm chart values
match the tag, run it from the release build workflow before any artifact
is published, and document it in the tagging steps. Alpha tags are exempt
as they only mark the creation of a release branch.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
Blaine Gardner
Travis Nielsen
Santosh Pillai
Michael Adam
Joshua Hoblitt
Joshua Hoblitt
Somanchi Poorna Sobhita
Artem Torubarov
subham rai
dependabot[bot]
parth-gr
Malay Kumar Parida