Skip to content

mgr/cephadm: add ok-to-stop functions for ceph client services#38915

Merged
sebastian-philipp merged 1 commit intoceph:masterfrom
Daniel-Pivonka:clientsoktostop
Mar 2, 2021
Merged

mgr/cephadm: add ok-to-stop functions for ceph client services#38915
sebastian-philipp merged 1 commit intoceph:masterfrom
Daniel-Pivonka:clientsoktostop

Conversation

@Daniel-Pivonka
Copy link

@Daniel-Pivonka Daniel-Pivonka commented Jan 14, 2021

create ok-to-stop functions for client services.

depends on #38854
related to #38897

Signed-off-by: Daniel-Pivonka dpivonka@redhat.com
Signed-off-by: Adam King adking@redhat.com

@Daniel-Pivonka
Copy link
Author

Daniel-Pivonka commented Jan 14, 2021

created as a draft because we are still testing but wanted to get something up to share and get feedback from

mgfritch
mgfritch reviewed Jan 15, 2021
View reviewed changes
Copy link
Contributor

@mgfritch mgfritch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This appears to duplicate a fair amount of logic for if the num of dameons >=1. Can it be consolidated up into the existing ok_to_stop of the CephadmService class?

jmolmo
jmolmo requested changes Jan 15, 2021
View reviewed changes
Copy link
Member

@jmolmo jmolmo left a comment

Choose a reason for hiding this comment

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

I think this is what we need!.. Just to say that it could be nice to improve the detection of ha-rwg running properly with my suggestion.

@adk3798
Copy link
Contributor

adk3798 commented Jan 15, 2021

Removed ha-rgw ok-to-stop from this. This PR is meant to be more straightforward set of ok-to-stop functions for the remaining daemon types, however ha-rgw would require introduction of daemon types to ok-to-stop (currently the classes self.TYPE, which is a service type, and causes issues for ha-rgw). Since it's a more substantial change that ultimately affects all ok-to-stop functions by changing parameters we're going to split it into its own PR

@Daniel-Pivonka
Copy link
Author

Daniel-Pivonka commented Jan 18, 2021

This appears to duplicate a fair amount of logic for if the num of dameons >=1. Can it be consolidated up into the existing ok_to_stop of the CephadmService class?

i have something for this but it involves modifying the _enough_daemons_to_stop func so it will be easier to add once #38854 merged

@Daniel-Pivonka Daniel-Pivonka marked this pull request as ready for review January 18, 2021 21:23
@Daniel-Pivonka Daniel-Pivonka requested a review from a team as a code owner January 18, 2021 21:23
@Daniel-Pivonka Daniel-Pivonka force-pushed the clientsoktostop branch 3 times, most recently from dfd5eb0 to 857da69 Compare January 27, 2021 20:31
sebastian-philipp
sebastian-philipp approved these changes Jan 28, 2021
View reviewed changes
@Daniel-Pivonka Daniel-Pivonka force-pushed the clientsoktostop branch 2 times, most recently from 65b482f to 8bb50c1 Compare January 29, 2021 19:05
@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Feb 3, 2021

ping @jmolmo

@github-actions
Copy link

github-actions bot commented Feb 8, 2021

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

jmolmo
jmolmo approved these changes Feb 9, 2021
View reviewed changes
Copy link
Member

@jmolmo jmolmo left a comment

Choose a reason for hiding this comment

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

LGTM

sebastian-philipp
sebastian-philipp suggested changes Feb 13, 2021
View reviewed changes
This was referenced Feb 13, 2021
Merged
Merged
@Daniel-Pivonka Daniel-Pivonka force-pushed the clientsoktostop branch 2 times, most recently from d4e8dc9 to da5be7f Compare February 17, 2021 21:48
@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Feb 18, 2021
@github-actions
Copy link

github-actions bot commented Feb 21, 2021

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

@sebastian-philipp sebastian-philipp removed the wip-swagner-testing My Teuthology tests label Feb 22, 2021
mgr/cephadm: add ok-to-stop functions for ceph client services
674912b 
Signed-off-by: Daniel-Pivonka <dpivonka@redhat.com>
Signed-off-by: Adam King <adking@redhat.com>
@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Feb 26, 2021
@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Mar 2, 2021

https://pulpito.ceph.com/swagner-2021-03-02_10:27:34-rados:cephadm-wip-swagner-testing-2021-03-01-1657-distro-basic-smithi/

@sebastian-philipp sebastian-philipp merged commit 5dd8c08 into ceph:master Mar 2, 2021
@sebastian-philipp sebastian-philipp mentioned this pull request Mar 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgfritch mgfritch mgfritch left review comments

@jmolmo jmolmo jmolmo approved these changes

+1 more reviewer

@sebastian-philipp sebastian-philipp sebastian-philipp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cephadm orchestrator pybind wip-swagner-testing My Teuthology tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Daniel-Pivonka @adk3798 @sebastian-philipp @mgfritch @jmolmo