mgr/cephadm: mgr or mds scale-down should prefer non-active daemons by adk3798 · Pull Request #36485 · ceph/ceph

adk3798 · 2020-08-05T21:54:10Z

When placing daemons during a mgr/mds scale-down, give preference
to the host with the active daemon so the active daemon is not
picked for removal

When removing daemons during a mgr/mds scale-down, prefer to remove
standby daemons so the active daemon is not killed

Fixes: https://tracker.ceph.com/issues/44252
Signed-off-by: Adam King adking@redhat.com

sebastian-philipp

I'm missing any tests here. Mind if you create a test in test_scheduling? I really want to avoid this to break at any time in the future.

src/pybind/mgr/cephadm/module.py

adk3798 · 2020-08-07T01:27:54Z

Swapped from passing the get_active_daemon function to the scheduler to having a new field in the DaemonDescription object to mark if a daemon is the active one (only applies to mgr and mds). Right now, I'm setting it in the _check_daemons function in the cephadm module which runs near the end of the serve loop. It was discussed to set the flag in the get_daemons_with_volatile_status function in the inventory file but I don't think this function is called often enough to expect the flag to be set when a scale-down happens.

I still need to make tests.

mchangir · 2020-08-07T15:25:03Z

@adk3798
The commit message confused me to no end.
If we are scaling down and we give preference to active daemons, then I'd think that daemons that are active are the ones actually killed. But its the other way round.
I think the phrasing should be:
During scale down of mgr/mds daemons, do not kill active daemons and instead prefer or prioritize standby (non-active) daemons for killing.

I hope this makes sense.

adk3798 · 2020-08-07T15:43:27Z

@adk3798
The commit message confused me to no end.
If we are scaling down and we give preference to active daemons, then I'd think that daemons that are active are the ones actually killed. But its the other way round.
I think the phrasing should be:
During scale down of mgr/mds daemons, do not kill active daemons and instead prefer or prioritize standby (non-active) daemons for killing.

I hope this makes sense.

The intention with that part of the message was to mirror the line from the ceph tracker "2. make the scheduler prefer active daemons when placing them." but I can see how that can be confusing, especially when the first line of the commit says it will prefer non-active daemons, so I'll change it to speak only about preferring non-active daemons when removing them and leave out language about placing.

adk3798 · 2020-08-07T19:05:46Z

Added some python tests for placing on hosts when one of the daemons has the is_active flag set

src/pybind/mgr/cephadm/tests/test_scheduling.py

src/pybind/mgr/orchestrator/_interface.py

src/pybind/mgr/cephadm/services/cephadmservice.py

sebastian-philipp · 2020-08-10T23:25:32Z

jenkins test make check

sebastian-philipp · 2020-08-17T07:53:01Z

src/pybind/mgr/cephadm/module.py

            if dd.daemon_type in ['grafana', 'iscsi', 'prometheus', 'alertmanager', 'nfs']:
                daemons_post[dd.daemon_type].append(dd)
+
+            if dd.daemon_type in ['mgr', 'mds']:


why only for mgr and mds? We have get_active_daemon also for grafana

@sebastian-philipp The reason I was originally very careful and only allowing mds or mgr daemons here was because I had left get_active_daemon undefined for many of the daemon types. I've refactored it a bit so if it's called for a service that hasn't defined the function it just gets back an empty Daemon Desc and then checks if the daemon_id on the Daemon Desc it gets back matches the daemon that is being checked. Thoughts?

Also, the reasoning here is the same reason I was limiting it to mgr and mds elsewhere. I've marked those conversations as resolved but can go back to them if this system is insufficient.

lgtm. I'll try to get though QA asap

src/pybind/mgr/cephadm/schedule.py

src/pybind/mgr/cephadm/services/cephadmservice.py

src/pybind/mgr/cephadm/tests/test_spec.py

src/pybind/mgr/orchestrator/_interface.py

adk3798 · 2020-08-17T18:43:10Z

jenkins test make check

When removing daemons during a mgr/mds scale-down, prefer to remove standby daemons so the active daemon is not killed Fixes: https://tracker.ceph.com/issues/44252 Signed-off-by: Adam King <adking@redhat.com>

I'm facing a problem that `ceph orch daemon redeploy <our own mgr>` really fails badly: 1. client sends `redeploy` to the mon 2. mon sends `redeploy` to the mgr 3. we synchonously call _create_daemon() with our own mgr. this never completes 4. the mon re-sends the command to the mgr as soon as it starts 5. goto 2. and we#re in an evil endless loop I'm now trying to 1. ok-to-stop to always return False in this case and 2. call ok-to-stop from `orch daemon redeploy` ,but this makes some problems, as we then no longer failove the mgr thus never undeploy the active MGR. Turns out this is really closely related to AdamKm's ceph#36485 (preferring to remove standby daemons instead of active ones), but that dosn't solve `orch daemon redeploy`. So for now, I think making ok-to-stop always false for our own mgr is the wrong idea for that I should rely on ceph#36485 instead. but how do I solve `daemon redeploy`? I think we _have_ to failover before redeploying our own mgr I mean, self._create_daemon is the last call we ever execute. becuase then we're gone and another MGR takes over Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

sebastian-philipp · 2020-08-20T09:14:38Z

https://pulpito.ceph.com/swagner-2020-08-19_07:38:40-rados:cephadm-wip-swagner-testing-2020-08-18-1624-distro-basic-smithi/

adk3798 added needs-review cephadm labels Aug 5, 2020

adk3798 requested a review from a team as a code owner August 5, 2020 21:54

sebastian-philipp suggested changes Aug 6, 2020

View reviewed changes

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

adk3798 force-pushed the cephadm-44252 branch from 788cc6e to ddc0fe8 Compare August 7, 2020 01:20

sebastian-philipp requested a review from mchangir August 7, 2020 14:36

adk3798 force-pushed the cephadm-44252 branch 2 times, most recently from 86e4ee7 to 7c0fad6 Compare August 7, 2020 18:50

sebastian-philipp suggested changes Aug 10, 2020

View reviewed changes

src/pybind/mgr/cephadm/tests/test_scheduling.py Show resolved Hide resolved

src/pybind/mgr/orchestrator/_interface.py Show resolved Hide resolved

src/pybind/mgr/cephadm/services/cephadmservice.py Outdated Show resolved Hide resolved

adk3798 force-pushed the cephadm-44252 branch from 7c0fad6 to cfe21c1 Compare August 10, 2020 19:02

adk3798 force-pushed the cephadm-44252 branch 2 times, most recently from 9bf3424 to 949bf30 Compare August 11, 2020 19:50

sebastian-philipp suggested changes Aug 17, 2020

View reviewed changes

adk3798 force-pushed the cephadm-44252 branch from 949bf30 to 27f9788 Compare August 17, 2020 15:21

sebastian-philipp approved these changes Aug 17, 2020

View reviewed changes

sebastian-philipp added needs-qa and removed needs-review labels Aug 17, 2020

mgr/cephadm: mgr or mds scale-down should prefer non-active daemons

0fbf12c

When removing daemons during a mgr/mds scale-down, prefer to remove standby daemons so the active daemon is not killed Fixes: https://tracker.ceph.com/issues/44252 Signed-off-by: Adam King <adking@redhat.com>

adk3798 force-pushed the cephadm-44252 branch from 27f9788 to 0fbf12c Compare August 18, 2020 14:05

sebastian-philipp added the wip-swagner-testing My Teuthology tests label Aug 18, 2020

sebastian-philipp mentioned this pull request Aug 19, 2020

[DNM] cephadm: Attempt to make ok-to-stop fail for the current MGR. #36720

Closed

3 tasks

sebastian-philipp merged commit 35b0261 into ceph:master Aug 20, 2020

sebastian-philipp mentioned this pull request Aug 21, 2020

octopus: cephadm batch backport August (2) #36748

Merged

snyk-bot mentioned this pull request May 8, 2023

[Snyk] Upgrade bootstrap from 4.6.1 to 4.6.2 liavt/ceph#124

Open

Conversation

adk3798 commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastian-philipp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adk3798 commented Aug 7, 2020

Uh oh!

mchangir commented Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adk3798 commented Aug 7, 2020

Uh oh!

adk3798 commented Aug 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sebastian-philipp commented Aug 10, 2020

Uh oh!

sebastian-philipp Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

adk3798 Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

sebastian-philipp Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adk3798 commented Aug 17, 2020

Uh oh!

sebastian-philipp commented Aug 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adk3798 commented Aug 5, 2020 •

edited

Loading

mchangir commented Aug 7, 2020 •

edited

Loading