mon, osd, *: expose upmap-primary in OSDMap::get_features() by rzarzynski · Pull Request #57776 · ceph/ceph

rzarzynski · 2024-05-29T19:35:18Z

This is a minimal fix to ensure only peers understanding pg-upmap-primary are able to connect, and thus to exclude the possibility of running into the pg_upmap_primaries.empty() assertion in encoders.

Fixes for other problems will follow up.

The intention is to ship this patch in the very next minor release of reef.

Fixes: https://tracker.ceph.com/issues/61948

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

idryomov

As automated test coverage is clearly lacking, it would be nice if the list of test cases that were executed manually on this PR was posted for the record.

src/osd/OSDMap.cc

rzarzynski · 2024-05-30T09:34:52Z

Added a new commit that introduces CEPH_FEATUREMASK_SERVER_REEF to ceph_release_features().
Only changed the commit description of the original commit to convey the manual testing steps. No other changes.

JoshSalomon · 2024-05-30T10:38:07Z

lgtm

idryomov

Only changed the commit description of the original commit to convey the manual testing steps.

I expected to see it in a comment, but since you added it to the commit messages to be preserved in git, some nits (feel free to ignore!):

*** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH *** and 2024-05-30T08:56:41.464+0000 7feb3d44a700 -1 WARNING: all dangerous and experimental features are enabled. lines are unnecessary clutter
Error EINVAL: osd.1 is already primary for pg 1.0 output doesn't have a corresponding command (probably a leftover from removing a command with a typo?)
in the second commit message, it would be nice to include ceph daemon mon.a sessions output with the quincy client or mention that it's a continuation of the test from the previous commit with ceph -w from quincy still running

src/common/ceph_strings.cc

ljflores · 2024-06-04T22:44:14Z

@rzarzynski can you stage a Reef backport too? We cherry-picked it on the reef-release, but we should also merge this to Reef (unless I'm mistaken)

idryomov · 2024-06-05T06:29:14Z

@rzarzynski can you stage a Reef backport too? We cherry-picked it on the reef-release, but we should also merge this to Reef (unless I'm mistaken)

@ljflores Did you see #57794?

github-actions · 2024-07-10T13:53:24Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

This is a minimal fix to ensure only peers understanding `pg-upmap-primary` are able to connect, and thus to exclude the possibility of running into the `pg_upmap_primaries.empty()` assertion in encoders. Fixes for other problems will follow up. The intention is to ship this patch in the very next minor release of reef. Manual testing -------------- \### start using upmap-primar is presence of `quincy` client NOTE: incompatible clients aren't disconnected but this is known and expected as we lack the machinery. ``` [rzarzynski@o06 build]$ bin/ceph osd get-require-min-compat-client reef [rzarzynski@o06 build]$ bin/ceph daemon mon.a sessions | jq -jr '.[] | .name, "\t", .con_features, "\t", .con_features_hex, "\n"' | grep client client.? 4540701547738038271 3f03cffffffdffff client.? 4540138320759226367 3f01cfbf7ffdffff [rzarzynski@o06 build]$ bin/ceph osd pool create test_pool 1 1 pool 'test_pool' created [rzarzynski@o06 build]$ bin/ceph osd pg-upmap-primary 1.0 2 change primary for pg 1.0 to osd.2 [rzarzynski@o06 build]$ bin/ceph daemon mon.a sessions | jq -jr '.[] | .name, "\t", .con_features, "\t", .con_features_hex, "\n"' | grep client client.? 4540701547738038271 3f03cffffffdffff client.? 4540138320759226367 3f01cfbf7ffdffff ``` \### `main` client is still able to connect ``` [rzarzynski@o06 build]$ bin/ceph -w cluster: id: d570a7c-84ca-4fd0-aafb-80138762c6af health: HEALTH_WARN 11 mgr modules have failed dependencies 1 pool(s) do not have an application enabled services: mon: 1 daemons, quorum a (age 64m) mgr: x(active, since 64m) osd: 3 osds: 3 up (since 64m), 3 in (since 64m) data: pools: 1 pools, 1 pgs objects: 0 objects, 0 B usage: 3.0 GiB used, 300 GiB / 303 GiB avail pgs: 1 active+clean ``` \### `quincy` client is refused ``` [rzarzynski@o06 build-quincy]$ bin/ceph -s -c /home/rzarzynski/ceph2/build/ceph.conf 2024-05-30T08:59:42.411+0000 7f0911a9b700 -1 --2- 127.0.0.1:0/2812481872 >> [v2:127.0.0.1:40536/0,v1:127.0.0.1:40537/0] conn(0x7f090c111500 0x7f090c1118f0 secure :-1 s=SESSION_CONNECTING pgs=0 cs=0 l=0 rev1=1 crypto rx=0x7f08fc0048c0 tx=0x7f08fc009e30 comp rx=0 tx=0).handle_ident_missing_features client does not support all server features: 80000000 2024-05-30T08:59:42.612+0000 7f0911a9b700 0 --2- 127.0.0.1:0/2812481872 >> [v2:127.0.0.1:40536/0,v1:127.0.0.1:40537/0] conn(0x7f090c111500 0x7f090c1118f0 unknown :-1 s=AUTH_CONNECTING pgs=0 cs=0 l=0 rev1=1 crypto rx=0 tx=0 comp rx=0 tx=0).send_auth_request get_initial_auth_request returned -2 ``` \### stop using upmap-primary ``` [rzarzynski@o06 build]$ bin/ceph osd rm-pg-upmap-primary 1.0 clear 1.0 pg_upmap_primary mapping ``` \### `quincy` client may connect again ``` [rzarzynski@o06 build-quincy]$ bin/ceph -s -c /home/rzarzynski/ceph2/build/ceph.conf cluster: id: d570a7c-84ca-4fd0-aafb-80138762c6af health: HEALTH_WARN 11 mgr modules have failed dependencies 1 pool(s) do not have an application enabled services: mon: 1 daemons, quorum a (age 77m) mgr: x(active, since 77m) osd: 3 osds: 3 up (since 76m), 3 in (since 76m) data: pools: 1 pools, 1 pgs objects: 0 objects, 0 B usage: 3.0 GiB used, 300 GiB / 303 GiB avail pgs: 1 active+clean ``` Fixes: https://tracker.ceph.com/issues/61948 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

ljflores · 2024-07-17T17:52:11Z

jenkins test make check

ljflores · 2024-07-17T17:52:17Z

jenkins test api

ljflores · 2024-07-23T16:30:56Z

jenkins test make check

ljflores · 2024-07-23T16:31:05Z

Rados approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrackercephcomissues66706

yuriw · 2024-07-23T17:55:36Z

@rzarzynski @ljflores pls merge when all checks passed
ref: https://tracker.ceph.com/issues/66706

ljflores · 2024-07-23T18:27:51Z

jenkins test make check

mon, osd, *: expose upmap-primary in OSDMap::get_features()

upmap-read mode of the balancer isn't supported by the kernel client. There was a recent change on the RADOS side which actually enforced this, see ceph/ceph#57776. Prior to 18.2.4 and 19.1.1 all sorts of incompatibility checks were missing in this area. So we need to revert upmap-read mode for now. Signed-off-by: sp98 <sapillai@redhat.com>

upmap-read mode of the balancer isn't supported by the kernel client. There was a recent change on the RADOS side which actually enforced this in ceph/ceph#57776.Prior to 18.2.4 and 19.1.1 all sorts of incompatibility checks were missing in this area. So we need to revert upmap-read mode for now. Signed-off-by: sp98 <sapillai@redhat.com>

rzarzynski added the bug-fix label May 29, 2024

rzarzynski requested a review from JoshSalomon May 29, 2024 19:35

rzarzynski requested a review from a team as a code owner May 29, 2024 19:35

github-actions bot added the core label May 29, 2024

rzarzynski requested review from idryomov and ljflores May 29, 2024 19:35

idryomov reviewed May 29, 2024

View reviewed changes

src/osd/OSDMap.cc Show resolved Hide resolved

athanatos approved these changes May 30, 2024

View reviewed changes

rzarzynski force-pushed the wip-bug-61948 branch from de34460 to 0648bd5 Compare May 30, 2024 09:27

github-actions bot added the common label May 30, 2024

JoshSalomon approved these changes May 30, 2024

View reviewed changes

idryomov approved these changes May 30, 2024

View reviewed changes

src/common/ceph_strings.cc Show resolved Hide resolved

rzarzynski force-pushed the wip-bug-61948 branch from 0648bd5 to 4d74ff6 Compare May 30, 2024 11:51

rzarzynski added the needs-qa label May 30, 2024

rzarzynski mentioned this pull request May 30, 2024

reef: mon, osd, *: expose upmap-primary in OSDMap::get_features() #57794

Merged

14 tasks

ljflores approved these changes May 30, 2024

View reviewed changes

rzarzynski mentioned this pull request May 31, 2024

squid: mon, osd, *: expose upmap-primary in OSDMap::get_features() #57805

Merged

14 tasks

idryomov mentioned this pull request May 31, 2024

mon: validate everybody understands MSR on set-require-min-compat-client #57808

Merged

14 tasks

neha-ojha added the wip-yuri12-testing label Jun 26, 2024

github-actions bot added the needs-rebase label Jul 10, 2024

ljflores force-pushed the wip-bug-61948 branch from 4d74ff6 to 331f1af Compare July 16, 2024 18:14

github-actions bot removed the needs-rebase label Jul 16, 2024

ljflores added the needs-squid-backport PR needs a squid backport label Jul 23, 2024

yuriw added TESTED ready-to-merge and removed wip-yuri12-testing labels Jul 23, 2024

ljflores merged commit baaf7c8 into ceph:main Jul 24, 2024

NitzanMordhai pushed a commit to NitzanMordhai/ceph that referenced this pull request Aug 1, 2024

Merge pull request ceph#57776 from rzarzynski/wip-bug-61948

3d2888e

mon, osd, *: expose upmap-primary in OSDMap::get_features()

idryomov mentioned this pull request Aug 13, 2024

use "upmap-read" as default balancer mode red-hat-storage/ocs-operator#2708

Merged

sp98 mentioned this pull request Aug 13, 2024

remove upmap-read balancer mode red-hat-storage/ocs-operator#2747

Merged

Conversation

rzarzynski commented May 29, 2024 • edited by ljflores Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

idryomov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rzarzynski commented May 30, 2024

Uh oh!

JoshSalomon commented May 30, 2024

Uh oh!

idryomov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ljflores commented Jun 4, 2024

Uh oh!

idryomov commented Jun 5, 2024

Uh oh!

github-actions bot commented Jul 10, 2024

Uh oh!

ljflores commented Jul 17, 2024

Uh oh!

ljflores commented Jul 17, 2024

Uh oh!

ljflores commented Jul 23, 2024

Uh oh!

ljflores commented Jul 23, 2024

Uh oh!

yuriw commented Jul 23, 2024

Uh oh!

ljflores commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

rzarzynski commented May 29, 2024 •

edited by ljflores

Loading