Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

teuthology/misc: Add timeout parameter to stop_daemons_of_type for better flexibility #2014

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NitzanMordhai
Copy link
Contributor

Updated stop_daemons_of_type to accept a timeout parameter, allowing dynamic control over the timeout value passed to the stop function of each daemon.

…tter flexibility

Updated stop_daemons_of_type to accept a timeout parameter,
allowing dynamic control over the timeout value passed to the
stop function of each daemon.

Signed-off-by: Nitzan Mordechai <[email protected]>
@kshtsk
Copy link
Contributor

kshtsk commented Jan 7, 2025

Is this tested already?

@NitzanMordhai
Copy link
Contributor Author

Is this tested already?

i ran few test using my PR issue. didn't riched to timeout, but the param was passed

@VallariAg
Copy link
Member

@NitzanMordhai the timeout didn't work even though the param was passed?
I'm not sure if timeouts are actually used in all kinds of daemon's stop() method:
It's used here:

def stop(self, timeout=300):

But I don't see it being used in these overrides of systemd and cephadm daemons:
def stop(self, timeout=300):

def stop(self, timeout=300):

@NitzanMordhai
Copy link
Contributor Author

@NitzanMordhai the timeout didn't work even though the param was passed?
I'm not sure if timeouts are actually used in all kinds of daemon's stop() method:
It's used here:

that's the purpose of that PR, to add the timeout so we can use it and not use the default

@VallariAg
Copy link
Member

@NitzanMordhai This PR relies on passing timeout to daemon.stop(timeout). But I'm not sure if this stop method actually uses the timeout param (depending on daemon type).

@NitzanMordhai
Copy link
Contributor Author

it will be pass to teuthology/orchestra/daemon/state.py
that will wait until the daemon stops (or timeout)

@VallariAg
Copy link
Member

Great, it should work well then!

@rzarzynski
Copy link
Contributor

@VallariAg, @NitzanMordhai: is this ready to merge? If it is, let's not hesitate and unblock the PR in ceph.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants