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

Add optional integer argument for --daemonize option #4759

Merged
merged 1 commit into from
Mar 29, 2016

Conversation

jrunning
Copy link
Contributor

The --daemonize option takes an optional integer argument for the number of seconds to wait before the first daemonized run.

Closes #3305

:long => "--daemonize",
:description => "Daemonize the process",
:proc => lambda { |p| true }
:short => "-d [WAIT]",
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this have backward compatibility implications and break existing scripts?

Copy link
Contributor

Choose a reason for hiding this comment

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

The brackets around WAIT mean it is an optional argument and the proc below makes it retain the backwards compatible behavior if it is not present

@lamont-granquist
Copy link
Contributor

I'm not sure this addresses #3305 though does it? The problem there is that if you have a non-zero splay and/or interval that you'll always sleep-before-running and this is the behavior that those users do not want to have at all -- they still want splay/interval options, but when present, they want the first run to be immediate and then for the chef-client to sleep afterwards. So a '--sleep-after-run' kind of flag to shift the interval/splay sleeps to the end of the loop.

@jrunning
Copy link
Contributor Author

@lamont-granquist Unless I'm misunderstanding, this addresses the issue. If they pass --daemonize 0 then the first run will happen immediately, followed by the first sleep.

@lamont-granquist
Copy link
Contributor

Ah okay, so i think the problem here is that you're missing all the exception decoration that we normally do around the chef-client run:

https://github.com/chef/chef/blob/jr/daemonize-immediate-run/lib/chef/application/client.rb#L457-L467

I think when you're sleeping you also need to have the sleep wrapped around with this logic or i suspect there's some edge conditions you might break:

https://github.com/chef/chef/blob/jr/daemonize-immediate-run/lib/chef/application/client.rb#L446-L453

@jrunning
Copy link
Contributor Author

@lamont-granquist Okay, that makes sense to me. Thanks!

@jrunning
Copy link
Contributor Author

@lamont-granquist Updated re: your comments; please let me know if you have any other feedback.

@tyler-ball
Copy link
Contributor

👍

1 similar comment
@lamont-granquist
Copy link
Contributor

👍

@jrunning jrunning merged commit 15a45cd into master Mar 29, 2016
@jrunning jrunning deleted the jr/daemonize-immediate-run branch March 29, 2016 20:37
@thommay thommay added Type: Enhancement Adds new functionality. and removed Enhancement labels Jan 25, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants