Skip to content

Conversation

dancmeyers
Copy link

@dancmeyers dancmeyers commented Sep 5, 2025

What does this PR do?

Ceph made various breaking changes to the status output in the octopus release in 2020, including (but not limited to) the entire mon_status command being removed.

The DataDog check as-was only checked if the release was exactly 'octopus', not any later release as well. Given that octopus is now 5 years old, and there doesn't seem to be anything in the responses that gives a semantic version or similar to numerically compare, it's easiest to just assume that we will get stats the New Way ™️, and also try any old way if the mon_status content exists in the raw map and the new way failed.

End-result functionality, on both pre-octopus and octopus, is identical to pre-PR. Hence existing tests still pass. Behaviour post-octopus is now correct.

Motivation

I configured the ceph integration for our cluster, and discovered on the dashboard that we were missing the ceph_fsid tag that is exposed across all the graphs as cluster, so I went digging for why that was and if there were any other details of the integration that might be similarly missing.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Ceph made various breaking changes to the status output in the `octopus`
release in 2020, including (but not limited to) the entire `mon_status`
command being removed.

The DataDog check as-was only checked if the release was exactly
'octopus', not any later release as well. Given that `octopus` is now 5
years old, and there doesn't seem to be anything in the responses that
gives a semantic version of similar to numerically compare, it's easiest
to just _assume_ that we will get stats the New Way ™️, and also try
any old way if the `mon_status` content exists in the `raw` map and the
new way failed.
@dancmeyers dancmeyers force-pushed the danm/fix-ceph-version-lookup branch from cd7aef4 to 0a8adbe Compare September 5, 2025 17:41
Copy link

codecov bot commented Sep 5, 2025

Codecov Report

❌ Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.00%. Comparing base (d444872) to head (0a8adbe).

Additional details and impacted files
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant