Project

General

Profile

Feature #51984

[RFE] Provide warning when the 'require-osd-release' flag does not match current release.

Added by Vikhyat Umrao 4 months ago. Updated about 17 hours ago.

Status:
Fix Under Review
Priority:
High
Category:
-
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
pacific, octopus, nautilus
Reviewed:
Affected Versions:
Component(RADOS):
Pull request ID:

Description

History

#1 Updated by Sebastian Wagner 4 months ago

Thinking. cephadm sets this automatically after the upgrade finishes in https://github.com/ceph/ceph/blob/c50d8ebdefcf8fe7b7b96ddf30124cc41ae77654/src/pybind/mgr/cephadm/upgrade.py#L761-L771 . Can we further hide this flag altogether and set this always to the minimum OSD version within the cluster? Is there a need to later add outdated OSDs to a cluster? Seems like an edge case to me. I'd favor if we hide things instead of creating new warnings.

#2 Updated by Vikhyat Umrao 4 months ago

Sebastian Wagner wrote:

Thinking. cephadm sets this automatically after the upgrade finishes in https://github.com/ceph/ceph/blob/c50d8ebdefcf8fe7b7b96ddf30124cc41ae77654/src/pybind/mgr/cephadm/upgrade.py#L761-L771 . Can we further hide this flag altogether and set this always to the minimum OSD version within the cluster? Is there a need to later add outdated OSDs to a cluster? Seems like an edge case to me. I'd favor if we hide things instead of creating new warnings.

I agree we could do it at MGR when all the OSD's up/in the MGR can set the flag, instead of leaving it to external tools it could be ceph-ansible, cephadm or manual upgrades scripts etc. If we could do this no need for a warning.

#3 Updated by Vikhyat Umrao 3 months ago

  • Priority changed from Normal to High

#4 Updated by Neha Ojha 16 days ago

  • Assignee set to Sridhar Seshasayee

#5 Updated by Sridhar Seshasayee 9 days ago

I am providing the history of PRs and commits that resulted in
the loss/removal of the checks for 'require-osd-release' flag
starting from the mimic release.

1. The commit below from PR: https://github.com/ceph/ceph/pull/15643
added the new OSDMonitor::_check_health(). This did not
implement the 'require-osd-release' checks but left a
placeholder to implement it at a later point in time.

   mon/OSDMonitor: implement new health checks

    Signed-off-by: Sage Weil <sage@redhat.com>
    PR: https://github.com/ceph/ceph/pull/15643
    commit 24a1636302f522e7355ef39584868ac22c75eae8
    committed on Jul 12, 2017

2. The following commit from the same PR in (1) above moved
OSDMonitor::_check_health() to OSDMap::check_health()
as is without adding the 'require-osd-release' checks.

    mon: move osd health checks into OSDMap method
    ...with one check moving into HealthMonitor where it belongs.

     Signed-off-by: Sage Weil <sage@redhat.com>
     PR: https://github.com/ceph/ceph/pull/15643
     commit: 6a9924270b9fa263955e904366527405db1890aa     
     committed on Jul 12, 2017

3. The below commit from PR: https://github.com/ceph/ceph/pull/17322
removed the legacy OSDMonitor::get_health() method. This
method had the legacy 'require-osd-release' checks and
was removed as part of the following commit.

    mon: remove legacy get_health infrastructure

     Signed-off-by: Sage Weil <sage@redhat.com>
     PR: https://github.com/ceph/ceph/pull/17322
     commit: 729a08923fb4403b88baafa0ae75643cfefe6510
     committed on Sep 6, 2017

Therefore, the original checks would now need to be
re-introduced within OSDMap::check_health().

#6 Updated by Sridhar Seshasayee 7 days ago

  • Status changed from New to Fix Under Review
  • Pull request ID set to 44090

#7 Updated by Neha Ojha about 17 hours ago

  • Backport set to pacific, octopus, nautilus

Also available in: Atom PDF