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.

Sign up for GitHub

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

Jump to bottom

Make the onbootsec parameter to attribute #707

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Make the onbootsec parameter to attribute #707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61bfe68
Make the onbootsec parameter to attribute
vineethp08 Aug 30, 2020
253c7be
Make the onbootsec parameter to attribute
vineethp088 Sep 11, 2020
af2ea6d
Update systemd_timer.rb
vineethp088 Sep 11, 2020
5fc0b14
Signed-off-by: vineethp08 <[email protected]>
vineethp088 Sep 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions attributes/default.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
default['chef_client']['log_file'] = 'client.log'
default['chef_client']['interval'] = '1800'
default['chef_client']['splay'] = '300'
default['chef_client']['delay_after_boot'] = '60'
default['chef_client']['conf_dir'] = '/etc/chef'
default['chef_client']['bin'] = '/opt/chef/bin/chef-client'

Expand Down
2 changes: 1 addition & 1 deletion recipes/systemd_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class ::Chef::Recipe
'Unit' => { 'Description' => 'chef-client periodic run' },
'Install' => { 'WantedBy' => 'timers.target' },
'Timer' => {
'OnBootSec' => '1min',
'OnBootSec' => "#{node['chef_client']['delay_after_boot']}sec",
'OnUnitInactiveSec' => "#{node['chef_client']['interval']}sec",
'RandomizedDelaySec' => "#{node['chef_client']['splay']}sec",
}
Expand Down
2 changes: 1 addition & 1 deletion resources/systemd_timer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

property :user, String, default: 'root'

property :delay_after_boot, String, default: '1min'
Copy link
Contributor

Choose a reason for hiding this comment

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

This resource is actually a backport from Chef Infra Client 16 so we need to keep the default value the same as it is in client at 1min.

Copy link
Author

Choose a reason for hiding this comment

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

We wanted to make the OnBootsec parameter as attribute, so that it can be controlled. We are seeing a problem with the chef client timer its getting elapased ( when runinng the chef client as a timer). We do not see that problem all the time/servers but we could reproduce the elapsed issue after reducing/increasing the onbootsec parameter. So if it can be controlled by node attribute it will help us fixing this issue. We can keep the default as ( 1 min/60sec) , May be I didnt get the point here about the chef16 dependecny.

Copy link

Choose a reason for hiding this comment

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

This cookbook has a systemd resource copied from Chef Infra 16, so that users of the cookbook can use older versions of Chef Infra. In order to ease transitioning to use the resource in Chef Infra 16, this needs to be defined the same as the one there: https://github.com/chef/chef/blob/master/lib/chef/resource/chef_client_systemd_timer.rb#L65-L67

The other changes in this PR are fine, but changing the default from '1min' to '60sec' needs to be reverted.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, Reverted let me know if this is fine. We just need that attribute to be controlled.

Copy link
Author

Choose a reason for hiding this comment

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

Please let me know if this can be merged or need any additional modification ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vineethp088 Please check #707 (comment)

property :delay_after_boot, String, default: '60sec'
property :interval, String, default: '30min'
property :splay, [Integer, String], default: 300,
coerce: proc { |x| Integer(x) },
Expand Down
2 changes: 1 addition & 1 deletion test/integration/timer_systemd/timer_systemd_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

control 'has expected unit content' do
describe file('/etc/systemd/system/chef-client.timer') do
its('content') { should match 'OnBootSec = 1min' }
its('content') { should match 'OnBootSec = 60sec' }
its('content') { should match 'OnUnitInactiveSec = 1800sec' }
its('content') { should match 'RandomizedDelaySec = 300sec' }
end
Expand Down