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

Update to the chef_client_scheduled_task resource frequency_modify default functionality #9920

Merged
merged 7 commits into from Jun 5, 2020
Merged

Update to the chef_client_scheduled_task resource frequency_modify default functionality #9920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8f91ff0
Update to the chef_client_scheduled_task resource so that it defaults…
chef-davin May 27, 2020
60cc229
Merge branch 'master' of github.com:chef/chef
chef-davin May 29, 2020
8d914c9
minor fixes for linting and comparator mistake
chef-davin May 29, 2020
921c165
Merge branch 'master' of github.com:chef/chef
chef-davin Jun 3, 2020
c572969
put the logic into the default parameter of the property definition, …
chef-davin Jun 3, 2020
f43db02
unit test updates for chef_client_scheduled task
chef-davin Jun 3, 2020
cc35e6b
Modify my fixes for the windows unit tests so that I don't break all …
chef-davin Jun 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/chef/resource/chef_client_scheduled_task.rb
View file
Open in desktop
Expand Up @@ -48,6 +48,16 @@ class ChefClientScheduledTask < Chef::Resource
daemon_options ["--override-runlist mycorp_base::default"]
end
```

**Run #{Chef::Dist::PRODUCT} daily at 01:00 am, specifying a named run-list**:

```ruby
chef_client_scheduled_task "Run chef-client named run-list daily" do
frequency 'daily'
start_time '01:00'
daemon_options ['-n audit_only']
end
```
DOC

resource_name :chef_client_scheduled_task
Expand All @@ -72,7 +82,8 @@ class ChefClientScheduledTask < Chef::Resource
coerce: proc { |x| Integer(x) },
callbacks: { "should be a positive number" => proc { |v| v > 0 } },
description: "Numeric value to go with the scheduled task frequency",
default: 30
default: lazy { frequency == "minute" ? 30 : 1 },
default_description: "30 if frequency is 'minute', 1 otherwise"

property :accept_chef_license, [true, false],
description: "Accept the Chef Online Master License and Services Agreement. See <https://www.chef.io/online-master-agreement/>",
Expand Down Expand Up @@ -129,6 +140,7 @@ class ChefClientScheduledTask < Chef::Resource

# According to https://docs.microsoft.com/en-us/windows/desktop/taskschd/schtasks,
# the :once, :onstart, :onlogon, and :onidle schedules don't accept schedule modifiers

windows_task new_resource.task_name do
run_level :highest
command full_command
Expand Down
20 changes: 15 additions & 5 deletions spec/unit/resource/chef_client_scheduled_task_spec.rb
View file
Open in desktop
Expand Up @@ -43,6 +43,16 @@
expect(resource.frequency_modifier).to eql(10)
end

it "expects default frequency modifier to be 30 when frequency is set to 'minute'" do
resource.frequency "minute"
expect(resource.frequency_modifier).to eql(30)
end

it "expects default frequency modifier to be 1 when frequency is set to 'daily'" do
resource.frequency "daily"
expect(resource.frequency_modifier).to eql(1)
end

it "validates the start_time property input" do
expect { resource.start_time("8:00 am") }.to raise_error(Chef::Exceptions::ValidationFailed)
expect { resource.start_time("8:00") }.to raise_error(Chef::Exceptions::ValidationFailed)
Expand Down Expand Up @@ -70,12 +80,12 @@

describe "#client_cmd" do
it "creates a valid command if using all default properties" do
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb")
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb") | eql("C:/opscode/chef/bin/chef-client -L C:\\chef/log/client.log -c C:\\chef/client.rb")
end

it "uses daemon_options if set" do
resource.daemon_options ["--foo 1", "--bar 2"]
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --foo 1 --bar 2")
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --foo 1 --bar 2") | eql("C:/opscode/chef/bin/chef-client -L C:\\chef/log/client.log -c C:\\chef/client.rb --foo 1 --bar 2")
Copy link
Contributor

Choose a reason for hiding this comment

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

Long term we need a better solution for cross platform specs, but this will do for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tas50 could we do some regex with it instead?
(I don't know if this is correct ruby regex syntax, I'm probably conflating with *nix syntax)

expect(provider.client_cmd).to match(/opscode\/chef\/bin\/chef-client -L .*chef\/log\/client.log -c .*chef\/client.rb --foo 1 --bar 2/)

It's not exactly pretty, but it's still feels better than the or statement (at least to me).

Copy link
Contributor

Choose a reason for hiding this comment

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

Historically we've had a lot of really subtle issues building these commands which makes me nervious to use a regex. I think we may just have to per platform set expects here so it's more obvious what we're trying to achieve.

end

it "uses custom config dir if set" do
Expand All @@ -86,17 +96,17 @@
it "uses custom log files / paths if set" do
resource.log_file_name "my-client.log"
resource.log_directory "C:/foo/bar"
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L C:/foo/bar/my-client.log -c /etc/chef/client.rb")
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L C:/foo/bar/my-client.log -c /etc/chef/client.rb") | eql("C:/opscode/chef/bin/chef-client -L C:/foo/bar/my-client.log -c C:\\chef/client.rb")
end

it "uses custom chef-client binary if set" do
resource.chef_binary_path "C:/foo/bar/chef-client"
expect(provider.client_cmd).to eql("C:/foo/bar/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb")
expect(provider.client_cmd).to eql("C:/foo/bar/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb") | eql("C:/foo/bar/chef-client -L C:\\chef/log/client.log -c C:\\chef/client.rb")
end

it "sets the license acceptance flag if set" do
resource.accept_chef_license true
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --chef-license accept")
expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --chef-license accept") | eql("C:/opscode/chef/bin/chef-client -L C:\\chef/log/client.log -c C:\\chef/client.rb --chef-license accept")
end
end
end