In createOrUpdateCephUser, when the desired user carries no explicit
keys the reconciler falls back to the live RGW user's keys. If that live
user also has zero keys the code intends to fail, but it built the error
with errors.Wrapf(err, ...) at a point where err is already nil (the
prior SetUserQuota error was handled and returned just above).
errors.Wrapf(nil, ...) returns nil, so the failure was swallowed and
createOrUpdateCephUser returned success on a user the operator itself
flagged as broken.
The reconcile then continued to generateCephUserSecret, which indexes
userConfig.Keys[0] to populate the Kubernetes secret and panicked on the
empty key slice. The reconciler's deferred RecoverAndLogException caught
and logged that panic, so the reconcile was abandoned before it reached
the Ready status update; and because the recovered Reconcile returns a
zero Result with a nil error, the request was not requeued either. The
user was left neither marked Ready nor retried.
Construct the error with errors.Errorf so the intended failure is
surfaced instead of being swallowed. Add a regression test that returns
a keyless live user and asserts a non-nil "no keys set" error.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
this commit add support for updating rgw caps with
`user-info-without-key` and `accounts` to cephobjectstoreuser crd
Adding the check for min ceph version from which these caps
are available.
Co-Authoured-by: Jiffin Tony Thottan <thottanjiffin@gmail.com>
Signed-off-by: subhamkrai <srai@redhat.com>
Add annotations to RBD node secrets in
import-external-cluster.sh so that externally
created secrets with
custom names can be discovered at runtime.
Update CreateUpdateClientProfileRadosNamespace to
look up secrets by annotation
instead of using hardcoded names.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
decodePeerToken logged the whole decoded PeerToken with "%+v" at debug
level. PeerToken embeds Key, the peer cluster's cephx auth secret, so
running the operator with ROOK_LOG_LEVEL=DEBUG wrote that credential to
the operator logs in plaintext (CWE-532).
Log only the non-sensitive fields (fsid, client id, mon host, namespace),
which keep the message useful for diagnostics, and drop Key. Add a
regression test that captures the debug output and asserts the key is
never present.
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>
currently there was a bug in the code where it didnt removed
the account from the ceph cluster during intial intialize
Signed-off-by: parth-gr <partharora1010@gmail.com>
testPodSpecPlacement accepts a pref parameter but never asserts the
preferred anti-affinity count, so the PreferredDuringScheduling branch
of SetNodeAntiAffinityForPod went unverified. The assertion was dropped
in b91f4211c9 when the separate preferred flag was removed from
SetNodeAntiAffinityForPod, leaving pref unused.
Restore the missing assertion so the helper validates both the required
and preferred term counts. Drop the third empty-placement case, which
became an exact duplicate of the second once the two-boolean API was
collapsed and now adds no coverage.
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>
The compareJSON test helper unmarshaled the expected JSON into both
the expected and actual variables, so assert.Equal always compared the
expected value against itself. The actualJSON argument was never read,
which made the assertion a tautology that could never fail and silently
disabled validation of updateCsiClusterConfig's output.
Parse actualJSON for the actual value. Enabling the check surfaced a
stale expectation in the multus sub-test: updateNetNamespaceFilePath
clears rbd.netNamespaceFilePath (holder pods are no longer used with the
ceph-csi-operator), so the expected literal is updated to drop that
field and match the real output.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Add annotations to CephFS and RBD provisioner secrets in
import-external-cluster.sh so that externally created secrets with
custom names can be discovered at runtime.
Update CreateUpdateClientProfileRadosNamespace and
generateProfileSubVolumeGroupSpec to look up secrets by annotation
instead of using hardcoded names.
closes: https://github.com/rook/rook/issues/16956
Signed-off-by: parth-gr <partharora1010@gmail.com>
CephFS reconciliation loop on Filesystem creation was swallowing errors.
The err variable which contained the error was replaced with the resulted error from the
kubernetes update status call on the CRD.
This commit fixes that and aggregate both errors in case there is both a kubernetes status update failure and a creation error.
Signed-off-by: Maxime Bertin <mbertin@luccasoftware.com>
Add the node publish secret to the client
profile that is required for the PVC's created
from SC without any node publish secret.
Signed-off-by: Madhu Rajanna <madhupr007@gmail.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>
Fix duplicate words, incorrect articles (a/an), it's/its, and other small
grammar mistakes in Go comments and user-facing messages across pkg/, cmd/,
and tests/.
Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
The AWS SDK v1 to v2 migration in #17468 left the S3Agent wrapper methods
calling the SDK with context.TODO(), so S3 operations could not be cancelled
when the controller context is cancelled. Add a leading context.Context
parameter to CreateBucket, PutObjectInBucket, GetObjectInBucket,
DeleteObjectInBucket, PutBucketPolicy, and GetBucketPolicy, forward it to the
underlying client, and pass clusterInfo.Context from the bucket provisioner.
Resolves#17526
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
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>
Anas Khan
Joshua Hoblitt
Travis Nielsen
subham rai
Blaine Gardner
Michael Adam
subhamkrai
Madhu Rajanna
Parth Arora
Artem Torubarov
Maxime Bertin
Anas Khan
Joshua Hoblitt
blackflytech
pengqima
daixihegu
kavirakesh14
Satoru Takeuchi
Santosh Pillai