Skip to content

qa/config/crimson_qa_overrides: cephadm to use crimson binary#66458

Closed
Matan-B wants to merge 2 commits intoceph:mainfrom
Matan-B:wip-matanb-crimson-cephadm-container
Closed

qa/config/crimson_qa_overrides: cephadm to use crimson binary#66458
Matan-B wants to merge 2 commits intoceph:mainfrom
Matan-B:wip-matanb-crimson-cephadm-container

Conversation

@Matan-B
Copy link
Contributor

@Matan-B Matan-B commented Dec 1, 2025

Previously we were using a crimson dedicated images, ceph-osd is no longer overriden when Crimson is enabled. See: #65782

for packaged based deployments we are now using:

sudo update-alternatives --set ceph-osd /usr/bin/ceph-osd-crimson

containerized deployments should also be able to use crimson-osd when both osd are available in the image.
extra_container_args would allow to pass additional arguments directly to the container at runtime and applied (ceph orch apply)

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


Related pr : ceph/ceph-build#2497

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)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@Matan-B Matan-B added this to Crimson Dec 1, 2025
@github-actions github-actions bot added the tests label Dec 1, 2025
@Matan-B Matan-B force-pushed the wip-matanb-crimson-cephadm-container branch from eb806d0 to ad89bf5 Compare December 1, 2025 09:55
@Matan-B Matan-B moved this to Unassigned / Drafts / Discussion in Crimson Dec 1, 2025
@Matan-B Matan-B force-pushed the wip-matanb-crimson-cephadm-container branch from ad89bf5 to 9b87f33 Compare December 1, 2025 14:24
Previously we were using a crimson dedicated images,
`ceph-osd` is no longer overriden when Crimson is enabled.
See: ceph#65782

for packaged based deployments we are now using:
```
sudo update-alternatives --set ceph-osd /usr/bin/ceph-osd-crimson
```

containerized deployments should also be able to use crimson-osd when
both osd are available in the image.
extra_container_args would allow to pass additional arguments
directly to the container at runtime and applied (ceph orch apply)

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

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
Similarly to e194cca.
"ceph.py" now uses "crimson_compat" instead of relying on the flavor
of the image. "cephadm.py" could reuse this logic so that the
is_crimson method would also work for cephadm.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
@Matan-B Matan-B force-pushed the wip-matanb-crimson-cephadm-container branch from 9b87f33 to d6d7304 Compare December 1, 2025 17:37
@athanatos
Copy link
Contributor

athanatos commented Dec 2, 2025

This PR is reasonable as is and is a good way to make progress testing crimson with cephadm.

I think the next step (future PRs) would be:

  1. Add a osd_type: crimson|classic (classic default) flag to the osd spec definition for users to use instead of explicitly specifying extra_container_args. Internally, it's perfectly reasonable for cephadm to just add that flag, I just don't want users to need to interact with it. Users should simply need to specify that an osd should be crimson without needing to know what binaries to replace.
  2. Documentation -- crimson documentation needs updated instructions.

@Matan-B Matan-B marked this pull request as ready for review December 2, 2025 16:17
@Matan-B Matan-B moved this from Unassigned / Drafts / Discussion to Needs QA in Crimson Dec 2, 2025
@Matan-B
Copy link
Contributor Author

Matan-B commented Dec 2, 2025

I'll need to test this along with #66217 or enable a new cephadm test since we don't have any yet.

@Matan-B
Copy link
Contributor Author

Matan-B commented Dec 4, 2025

This PR is reasonable as is and is a good way to make progress testing crimson with cephadm.

I think the next step (future PRs) would be:

  1. Add a osd_type: crimson|classic (classic default) flag to the osd spec definition for users to use instead of explicitly specifying extra_container_args. Internally, it's perfectly reasonable for cephadm to just add that flag, I just don't want users to need to interact with it. Users should simply need to specify that an osd should be crimson without needing to know what binaries to replace.
  2. Documentation -- crimson documentation needs updated instructions.

Agreed, tracked here: https://tracker.ceph.com/issues/74081

@perezjosibm
Copy link
Contributor

@Matan-B
Copy link
Contributor Author

Matan-B commented Dec 8, 2025

Yes, thank you! Please note the PR currently only changes qa directory and does not require a dedicated build as we can pass qa "testing-branch" directly when scheduling a job.

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Matan-B
Copy link
Contributor Author

Matan-B commented Jan 12, 2026

Supporting crimson within cephadm natively is far better option - #66811.
I'll wait with this PR. Hopefully we won't need this one.

@Matan-B Matan-B closed this Feb 10, 2026
@Matan-B Matan-B removed this from Crimson Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants