Ronen Friedman [Fri, 28 Mar 2025 16:16:52 +0000 (11:16 -0500)]
osd/scrub: decoupling the FSM from the scrubber
The scrubber FSM has always been accessing the PgScrubber
object via an abstract interface (ScrubMachineListener).
But the reverse was not true: the PgScrubber owned a unique_ptr
directly to an instance of Scrubber::ScrubMachine.
This PR introduces a new interface, ScrubFsmIf, that is
implemented by the ScrubMachine. This simplifies
unit-testing the PgScrubber, as we can now inject a mock
implementation of the FSM interface.
Ronen Friedman [Tue, 18 Mar 2025 09:03:39 +0000 (04:03 -0500)]
common/Formatter: introduce helpers for formatting container types.
Specifically:
- with_array_sections() creates a Formmater::ArraySection, then
formats every element in the container using the provided
callable.
Special handling for std::map: the callable is called with
both the key and the value.
- with_obj_array_section() dumps containers of objects:
First - an ArraySection is created; then - for each object
in the container, the object is formatted within an ObjectSection,
using the provided callable.
If the container is an std::map, the callable is called with both
the key and the object itself.
Matt Benjamin [Fri, 21 Feb 2025 19:43:53 +0000 (14:43 -0500)]
rgw:cksum: implement crc64nvme and combined 32- and 64-bit CRCs
* internally compensate for at-rest byteswapped crc64 representation (e.g., before combine step)
* generalize Cksum crc api for 32bit and 64bit crcs, other cleanup
* prototype abstract cksum::Combiner workflow
* add support for forward and backward handling of composite vs full object checksums
by marking the composites and the update flag day
* clean up checksum formatting and checksum type reporting
* add unit tests for Combiner interface
* validate and track requested checksum type (i.e., composite or full), plus
unit test fixture for combinations of full matrix of cksum types
* doh. GET/HEAD checksums are in headers
* add crcany license to COPYING
* return ChecksumType as header in GET/HEAD
Found by Casey in review
* avoid fmt of char* null when no checksum header supplied
A cksum_hdr_t(nullptr, nullptr) results in this case, and is intended,
but its components obviously can't be presented to std::format unless
cast to a safe pointer type.
* fail checksum mismatch with BadDigest
When uploading an S3 object with an invalid checksum, the return code
should be BadDigest to mirror more closely the AWS S3 implementation.
See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html
Fixes: https://tracker.ceph.com/issues/70614
Reported by Alex Wojno <awojno@bloomberg.net>
* fix comparison and display of composite checksums
A string comparision bounds error and a bitmask comparison
error are fixed.
* fix build on centos9
On gcc(?13?) we can't declare rgw::cksum::FLAG_NONE
and also rgw::cksum::Cksum::FlAG_NONE.
SAD!!
* include <variant> invariantly
* fix checksum type return from complete-multipart
This one is in the XML response
* aieee, don't leak Combiners
Use unique_ptr to polymorphic type...correctly.
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Kefu Chai [Sun, 30 Mar 2025 03:59:12 +0000 (11:59 +0800)]
cephfs-top: Removes unused `global` statements
Recent flake8 runs were failing with:
```
py3: flake8==7.2.0,mccabe==0.7.0,pip==25.0.1,pycodestyle==2.13.0,pyflakes==3.3.0,setuptools==75.8.0,wheel==0.45.1
py3: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/cephfs/top> flake8 --ignore=W503 --max-line-length=100 cephfs-top
cephfs-top:344:9: F824 `global fs_list` is unused: name is never assigned in scope
cephfs-top:466:13: F824 `global current_states` is unused: name is never assigned in scope
cephfs-top:872:9: F824 `global metrics_dict` is unused: name is never assigned in scope
cephfs-top:872:9: F824 `global current_states` is unused: name is never assigned in scope
cephfs-top:911:9: F824 `global fs_list` is unused: name is never assigned in scope
cephfs-top:981:9: F824 `global current_states` is unused: name is never assigned in scope
cephfs-top:1126:13: F824 `global current_states` is unused: name is never assigned in scope
py3: exit 1 (0.77 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/cephfs/top> flake8 --ignore=W503 --max-line-length=100 cephfs-top pid=2309605
py3: FAIL code 1 (8.15=setup[7.38]+cmd[0.77] seconds)
evaluation failed :( (8.24 seconds)
```
Since these variables are only being referenced and not assigned within
their scopes, the `global` declarations are unnecessary and can be
safely removed. This change:
- Removes all flagged `global` statements
- Fixes the failing flake8 checks in the CI pipeline
- Maintains the original code behavior as variable references still work without the `global` keyword
The `global` keyword is only needed when assigning to global variables
within a function scope, not when simply referencing them.
Kefu Chai [Sun, 30 Mar 2025 03:48:28 +0000 (11:48 +0800)]
qa: Remove unnecessary global statements in tests
Removes unused `global` statements from Python test files to fix flake8
F824 errors.
Recent flake8 runs were failing with:
```
./tasks/radosgw_admin.py:330:5: F824 `global log` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:99:5: F824 `global incompat_paths` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:164:5: F824 `global backward_compat` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:165:5: F824 `global fast_shouldnt_skip` is unused: name is never assigned in scope
```
Since these variables are only being referenced and not assigned within
their scopes, the `global` declarations are unnecessary and can be
safely removed. This change:
- Removes all flagged `global` statements
- Fixes the failing flake8 checks in the CI pipeline
- Maintains the original code behavior as variable references still work
without the `global` keyword
The `global` keyword is only needed when assigning to global variables
within a function scope, not when simply referencing them.
Kefu Chai [Sun, 30 Mar 2025 02:31:21 +0000 (10:31 +0800)]
googletest: pick up change to silence CMake warnings
bump up googletest submodule to the latest stable release, in order to
silence the warning from CMake:
```
CMake Deprecation Warning at src/googletest/CMakeLists.txt:4 (cmake_minimum_required):
Compatibility with CMake < 3.10 will be removed from a future version of
CMake.
Update the VERSION argument <min> value. Or, use the <min>...<max> syntax
to tell CMake that the project requires at least <min> but has been updated
to work with policies introduced by <max> or earlier.
CMake Deprecation Warning at src/googletest/googlemock/CMakeLists.txt:45 (cmake_minimum_required):
Compatibility with CMake < 3.10 will be removed from a future version of
CMake.
Update the VERSION argument <min> value. Or, use the <min>...<max> syntax
to tell CMake that the project requires at least <min> but has been updated
to work with policies introduced by <max> or earlier.
CMake Deprecation Warning at src/googletest/googletest/CMakeLists.txt:56 (cmake_minimum_required):
Compatibility with CMake < 3.10 will be removed from a future version of
CMake.
Update the VERSION argument <min> value. Or, use the <min>...<max> syntax
to tell CMake that the project requires at least <min> but has been updated
to work with policies introduced by <max> or earlier.
```
Kefu Chai [Sun, 30 Mar 2025 03:14:01 +0000 (11:14 +0800)]
neorados/test: use [[maybe_unused]] instead of GTEST_ATTRIBUTE_UNUSED_
This change:
- Aligns with GoogleTest's own evolution, which now uses
`[[maybe_unused]]` internally (via
`GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED`) as of
https://github.com/google/googletest/commit/5a37b517
- Leverages C++20 features already available in Ceph
- Removes dependency on GoogleTest internal macros
- Enables updating the GoogleTest submodule to silence CMake warnings
Using the standard C++ attribute provides better compatibility and
future-proofing than relying on GoogleTest's internal implementation
details.
This change also allows us to bump up googletest submodule to silence a
CMake warning.
Kefu Chai [Sun, 30 Mar 2025 02:56:05 +0000 (10:56 +0800)]
neorados/test: Simplify copy prevention by using GtestNonCopyable
Updates the test framework to use GoogleTest's simplified copy
prevention mechanism by inheriting from
`::testing::internal::GtestNonCopyable` instead of custom macros.
This change:
- Replaces deprecated GTEST_DISALLOW_* macros that were removed in
GoogleTest (https://github.com/google/googletest/commit/bf66935e07825318ae519675d73d0f3e313b3ec6)
- Aligns with GoogleTest's current approach for preventing test suite
copying, See
https://github.com/google/googletest/commit/93f08be653c36ddc6943e9513fc14c7292b4d007
- Enables updating the GoogleTest submodule to silence CMake warnings
- Improves compatibility with recent distro-packaged GoogleTest versions
- add the minimal required GTest version when finding this package,
this change should have no impact to our packaging. as both deb
and rpm packagings are using bundled googletest submodule. and
googletest 1.14 has been included by recent stable releases of
popular distros, like fedora 41 and ubuntu 24.04.
While `GtestNonCopyable` is technically an internal helper, we already
use other helpers from the `::testing::internal` namespace, so this
change maintains consistency with our existing approach.
Kefu Chai [Sun, 30 Mar 2025 02:10:28 +0000 (10:10 +0800)]
cmake: do not pass PRE_BUILD to add_custom_command(OUTPUT)
Removes the invalid PRE_BUILD argument from add_custom_command(OUTPUT)
calls that was triggering CMake warnings. The PRE_BUILD option is only
valid for target-based custom commands and specifically with Visual
Studio generators.
Additionally sets the CMP0175 policy to NEW to ensure CMake fails on
invalid arguments to add_custom_command(), helping catch similar issues
earlier in the development process.
This change:
- Removes unnecessary PRE_BUILD argument from OUTPUT-based custom commands
- Sets CMP0175 policy to enforce validation of add_custom_command() arguments
- Resolves CMake warning in src/common/options/CMakeLists.txt
```
CMake Warning (dev) at src/common/options/CMakeLists.txt:34 (add_custom_command):
The following keywords are not supported when using
add_custom_command(OUTPUT): PRE_BUILD.
Policy CMP0175 is not set: add_custom_command() rejects invalid arguments.
Run "cmake --help-policy CMP0175" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Call Stack (most recent call first):
src/common/options/CMakeLists.txt:85 (add_options)
This warning is for project developers. Use -Wno-dev to suppress it.
```
Kefu Chai [Sun, 30 Mar 2025 01:39:53 +0000 (09:39 +0800)]
cmake: Allow building with fmt <= 11.1.4
Testing confirms successful builds with fmt 11.1.4, which is currently
packaged in Fedora 42. This change relaxes the fmt version constraints
to improve compatibility with Fedora 42 and other distributions shipping
this version of fmt-devel.
Kefu Chai [Wed, 26 Mar 2025 03:43:43 +0000 (11:43 +0800)]
src: migrate to fmt::formattable for format support
Replace custom has_formatter concept with fmtlib's fmt::formattable
- Deprecate usage of fmt::has_formatter
- Resolve compiler warnings with recent fmtlib versions
The custom concept of has_formatter is no longer necessary with
recent updates to the fmtlib library, as fmt::formattable was introduced
in fmtlib v11.0.0. By switching to fmt::formattable, we simplify our
code and stop using the deprecated `fmt::has_formatter` template.
Kefu Chai [Sat, 29 Mar 2025 13:10:05 +0000 (21:10 +0800)]
librbd: replace deprecated atomic_store with std::atomic<shared_ptr>
Update shared pointer atomic operations to use C++20's std::atomic<std::shared_ptr<T>>
instead of the deprecated atomic_store functions. This change addresses deprecation
warnings from GCC-15's libstdc++ where atomic shared pointer operations outside the
std::atomic class are being phased out:
```
/home/kefu/dev/ceph/src/librbd/ImageCtx.cc:1010:5: warning: 'atomic_store<neorados::IOContext>' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Wdeprecated-declarations]
1010 | atomic_store(&data_io_context, ctx);
| ^
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/shared_ptr_atomic.h:181:5: note: 'atomic_store<neorados::IOContext>' has been explicitly marked deprecated here
181 | _GLIBCXX20_DEPRECATED_SUGGEST("std::atomic<std::shared_ptr<T>>")
| ^
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/x86_64-redhat-linux/bits/c++config.h:2055:45: note: expanded from macro '_GLIBCXX20_DEPRECATED_SUGGEST'
2055 | # define _GLIBCXX20_DEPRECATED_SUGGEST(ALT) _GLIBCXX_DEPRECATED_SUGGEST(ALT)
| ^
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/x86_64-redhat-linux/bits/c++config.h:2023:19: note: expanded from macro '_GLIBCXX_DEPRECATED_SUGGEST'
2023 | __attribute__ ((__deprecated__ ("use '" ALT "' instead")))
| ^
```
The implementation now uses the standard-compliant approach that's recommended in
the compiler warnings, while maintaining backward compatibility with older compilers
by conditionally selecting the appropriate implementation.
Kefu Chai [Tue, 25 Mar 2025 04:03:30 +0000 (12:03 +0800)]
common: disable OpenSSL engine support if it is disabled
OpenSSL 3.0 documentation recommends moving from the ENGINE API to the
Providers API. Recent distributions may compile OpenSSL without engine
support by default, necessitating more flexible configuration handling.
So, in this change:
- Add a CMake option `WITH_OPENSSL_ENGINE` to explicitly control engine support
- Respect `openssl_engine_opts` when engine support is enabled
- Provide clear error messaging when engine options are set but support is disabled
See also:
- OpenSSL 3.0 documentation:
https://wiki.openssl.org/index.php/OpenSSL_3.0#Engines_and_.22METHOD.22_APIs
Zac Dover [Thu, 27 Mar 2025 12:24:13 +0000 (22:24 +1000)]
src/common: add guidance for mon_warn_pg_not_scrubbed
Add an explanation of how to set the value of
"mon_warn_pg_not_scrubbed_ratio" to the confval definition of that
variable. Although this variable contains the string "mon", it is set on
the Manager. I have added a note to direct users to set this value on
the Manager.
This issue was pointed out by Petr Tlapa on Slack in late March of 2025.
This issue is part of a small project that also encompasses
https://github.com/ceph/ceph/pull/62459, which is associated with commit aeef59a50ee31072648ba0c7436b6522137614cd
Co-authored-by: Anthony D'Atri <anthony.datri@gmail.com> Signed-off-by: Zac Dover <zac.dover@proton.me>
Previously, we relied on the __GNUC__ macro to check for std::atomic<std::shared_ptr>
support, which was inaccurate. This approach failed with Clang builds using libstdc++,
even when the feature was implemented. The warning looks like:
```
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/shared_ptr_atomic.h:131:5: note: 'atomic_load_explicit<std::vector<entity_addrvec_t>>' has been explicitly marked deprecated here
131 | _GLIBCXX20_DEPRECATED_SUGGEST("std::atomic<std::shared_ptr<T>>")
| ^
```
This change uses a standard-compliant feature test macro (__cpp_lib_atomic_shared_ptr)
to correctly detect support for std::atomic<std::shared_ptr>. This resolves compilation
issues and improves portability across different compilers and standard library
implementations.
Matt Benjamin [Tue, 18 Feb 2025 19:11:08 +0000 (14:11 -0500)]
rgw_cksum: add 64bit and 32bit crc constructions from crcany
These constructions provide conforming implemenations of Amazon
S3 CRC32 (ISO hdlc), CRC32C (iscsi), and CRC64/NVME checksums,
in particular, linear combining of adjacent checksums, by
Mark Adler.
Source: https://github.com/madler/crcany
License: approximate BSD with "optional" advertising clause
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
John Mulligan [Wed, 26 Mar 2025 17:37:04 +0000 (13:37 -0400)]
cephadm: stop sidecar systemd units when restarting main units
Previously, sidecar units might (will) not be stopped when the primary
unit is stopped. This breaks the normal cephadm workflow to redeploy:
stop and then start the services. This patch reuses the logic from
the service stop to stop the individual sidecars.
Note: the sidecars are not individually started as that should already
be handled automatically when starting the primary service!
Co-authored-by: Adam King <adking@redhat.com> Signed-off-by: Adam King <adking@redhat.com> Signed-off-by: John Mulligan <jmulligan@redhat.com>
Ilya Dryomov [Thu, 20 Mar 2025 17:28:44 +0000 (18:28 +0100)]
rbd: mirror_uuids -> mirror_uuid in remote mirror peer listing
... in "rbd mirror image status" and "rbd mirror pool status --verbose"
formatted outputs. In unformatted outputs this value is shown in place
of a name in case a site name isn't available.
Ilya Dryomov [Thu, 20 Mar 2025 16:10:47 +0000 (17:10 +0100)]
rbd: extend use of "none" placeholder to IMAGES section
Currently if there are no mirror-enabled images, IMAGES section
in "rbd mirror pool status --verbose" output isn't terminated:
$ rbd mirror pool status data --verbose
health: OK
daemon health: OK
image health: OK
images: 0 total
DAEMONS
service 4388:
...
health: OK
IMAGES$
DAEMONS section has a "none" placeholder for when there are no
rbd-mirror daemons running. Fix some issues with the separator logic
and employ the placeholder in IMAGES section:
$ rbd mirror pool status data --verbose
health: OK
daemon health: OK
image health: OK
images: 0 total
DAEMONS
service 4388:
...
health: OK
IMAGES
none
$
$ rbd mirror pool status data --verbose
health: UNKNOWN
daemon health: UNKNOWN
image health: OK
images: 0 total
rgw: fix the version in DOCODE_START() of RGWZoneGroupPlacementTier
This commit fixes an undetected merge conflict between PRs #61745
and #60159. The dencoding problem has been introduced very recently,
it is straightforward and causes failures of the make check bot
everywhere, therefore -- if no objections -- I want to merge this
patch without the Teuthology testing.
Redouane Kachach [Fri, 21 Mar 2025 12:13:56 +0000 (13:13 +0100)]
mgr/cephadm: making mgmt-gateway an oauth2-proxy dependency
This change enables better automation, especially for complex setups
like high-availability configurations. Previously, users had to
manually deploy the mgmt-gateway before the oauth-proxy; if this
sequence wasn't followed, cephadm would raise an error.