diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2018-03-05 10:37:47 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-05 10:37:47 -0800 |
commit | 41d69e2a5b2ff23488441b82f019b77b243dbdf7 (patch) | |
tree | d155da48c5d01329daaa762efd0ed4d7a383290b | |
parent | 96ae775bb89753871c30311a113c2efa269a2d71 (diff) | |
parent | 7a25e6b270d62f4a2a9a085ea25ed84169911f42 (diff) | |
download | chef-41d69e2a5b2ff23488441b82f019b77b243dbdf7.tar.gz |
Merge pull request #6914 from chef/lcg/no-fork-default-command-line
Don't use supervisor process for one-shot / command-line runs
-rw-r--r-- | chef-config/lib/chef-config/config.rb | 4 | ||||
-rw-r--r-- | lib/chef/application/client.rb | 10 | ||||
-rw-r--r-- | lib/chef/application/solo.rb | 10 | ||||
-rw-r--r-- | spec/integration/client/client_spec.rb | 100 | ||||
-rw-r--r-- | spec/unit/application/client_spec.rb | 50 |
5 files changed, 153 insertions, 21 deletions
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb index d5badcc58f..9d11ba9dc4 100644 --- a/chef-config/lib/chef-config/config.rb +++ b/chef-config/lib/chef-config/config.rb @@ -4,7 +4,7 @@ # Author:: AJ Christensen (<aj@chef.io>) # Author:: Mark Mzyk (<mmzyk@chef.io>) # Author:: Kyle Goodwin (<kgoodwin@primerevenue.com>) -# Copyright:: Copyright 2008-2017, Chef Software Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -440,7 +440,7 @@ module ChefConfig default :splay, nil default :why_run, false default :color, false - default :client_fork, true + default :client_fork, nil default :ez, false default :enable_reporting, true default :enable_reporting_url_fatals, false diff --git a/lib/chef/application/client.rb b/lib/chef/application/client.rb index 0834e5f037..706ba93ca0 100644 --- a/lib/chef/application/client.rb +++ b/lib/chef/application/client.rb @@ -2,7 +2,7 @@ # Author:: AJ Christensen (<aj@chef.io) # Author:: Christopher Brown (<cb@chef.io>) # Author:: Mark Mzyk (mmzyk@chef.io) -# Copyright:: Copyright 2008-2016, Chef Software, Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -228,8 +228,7 @@ class Chef::Application::Client < Chef::Application option :client_fork, :short => "-f", :long => "--[no-]fork", - :description => "Fork client", - :boolean => true + :description => "Fork client" option :recipe_url, :long => "--recipe-url=RECIPE_URL", @@ -362,6 +361,11 @@ class Chef::Application::Client < Chef::Application Chef::Config[:splay] = nil end + # supervisor processes are enabled by default for interval-running processes but not for one-shot runs + if Chef::Config[:client_fork].nil? + Chef::Config[:client_fork] = !!Chef::Config[:interval] + end + if !Chef::Config[:client_fork] && Chef::Config[:interval] && !Chef::Platform.windows? Chef::Application.fatal!(unforked_interval_error_message) end diff --git a/lib/chef/application/solo.rb b/lib/chef/application/solo.rb index 6b4aef42b4..f8502edc38 100644 --- a/lib/chef/application/solo.rb +++ b/lib/chef/application/solo.rb @@ -1,7 +1,7 @@ # # Author:: AJ Christensen (<aj@chef.io>) # Author:: Mark Mzyk (mmzyk@chef.io) -# Copyright:: Copyright 2008-2018, Chef Software, Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -175,8 +175,7 @@ class Chef::Application::Solo < Chef::Application option :client_fork, :short => "-f", :long => "--[no-]fork", - :description => "Fork client", - :boolean => true + :description => "Fork client" option :why_run, :short => "-W", @@ -261,6 +260,11 @@ class Chef::Application::Solo < Chef::Application Chef::Config[:interval] ||= 1800 end + # supervisor processes are enabled by default for interval-running processes but not for one-shot runs + if Chef::Config[:client_fork].nil? + Chef::Config[:client_fork] = !!Chef::Config[:interval] + end + Chef::Application.fatal!(unforked_interval_error_message) if !Chef::Config[:client_fork] && Chef::Config[:interval] if Chef::Config[:recipe_url] diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb index de12b8ba8e..77008524c8 100644 --- a/spec/integration/client/client_spec.rb +++ b/spec/integration/client/client_spec.rb @@ -46,7 +46,7 @@ describe "chef-client" do # we're running `chef-client` from the source tree and not the external one. # cf. CHEF-4914 let(:chef_client) { "ruby '#{chef_dir}/chef-client' --minimal-ohai" } - let(:chef_solo) { "ruby '#{chef_dir}/chef-solo' --minimal-ohai" } + let(:chef_solo) { "ruby '#{chef_dir}/chef-solo' --legacy-mode --minimal-ohai" } let(:critical_env_vars) { %w{_ORIGINAL_GEM_PATH GEM_PATH GEM_HOME GEM_ROOT BUNDLE_BIN_PATH BUNDLE_GEMFILE RUBYLIB RUBYOPT RUBY_ENGINE RUBY_ROOT RUBY_VERSION PATH}.map { |o| "#{o}=#{ENV[o]}" } .join(" ") } @@ -624,4 +624,102 @@ EOM expect(command.stdout).not_to include("INFO") end end + + when_the_repository "has a cookbook that knows if we're running forked" do + before do + file "cookbooks/x/recipes/default.rb", <<~EOM + puts Chef::Config[:client_fork] ? "WITHFORK" : "NOFORK" +EOM + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +EOM + end + + it "chef-client runs by default with no supervisor" do + command = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + + it "chef-solo runs by default with no supervisor" do + command = shell_out("#{chef_solo} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + + it "chef-client --no-fork does not fork" do + command = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default' --no-fork", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + + it "chef-solo --no-fork does not fork" do + command = shell_out("#{chef_solo} -c \"#{path_to('config/client.rb')}\" -o 'x::default' --no-fork", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + + it "chef-client with --fork uses a supervisor" do + command = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default' --fork", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("WITHFORK") + end + + it "chef-solo with --fork uses a supervisor" do + command = shell_out("#{chef_solo} -c \"#{path_to('config/client.rb')}\" -o 'x::default' --fork", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("WITHFORK") + end + end + + when_the_repository "has a cookbook that knows if we're running forked, and configures forking in config.rb" do + before do + file "cookbooks/x/recipes/default.rb", <<~EOM + puts Chef::Config[:client_fork] ? "WITHFORK" : "NOFORK" +EOM + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +client_fork true +EOM + end + + it "chef-client uses a supervisor" do + command = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("WITHFORK") + end + + it "chef-solo uses a supervisor" do + command = shell_out("#{chef_solo} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("WITHFORK") + end + end + + when_the_repository "has a cookbook that knows if we're running forked, and configures no-forking in config.rb" do + before do + file "cookbooks/x/recipes/default.rb", <<~EOM + puts Chef::Config[:client_fork] ? "WITHFORK" : "NOFORK" +EOM + file "config/client.rb", <<EOM +local_mode true +cookbook_path "#{path_to('cookbooks')}" +client_fork false +EOM + end + + it "chef-client uses a supervisor" do + command = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + + it "chef-solo uses a supervisor" do + command = shell_out("#{chef_solo} -c \"#{path_to('config/client.rb')}\" -o 'x::default'", :cwd => chef_dir) + command.error! + expect(command.stdout).to include("NOFORK") + end + end end diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb index 3f803f2033..0cd3cb3995 100644 --- a/spec/unit/application/client_spec.rb +++ b/spec/unit/application/client_spec.rb @@ -107,6 +107,7 @@ describe Chef::Application::Client, "reconfigure" do shared_examples "sets the configuration" do |cli_arguments, expected_config| describe cli_arguments do before do + cli_arguments ||= "" ARGV.replace(cli_arguments.split) app.reconfigure end @@ -144,6 +145,36 @@ describe Chef::Application::Client, "reconfigure" do end end + describe "--[no]-fork" do + before do + Chef::Config[:interval] = nil # FIXME: we're overriding the before block setting this + end + + context "by default" do + it_behaves_like "sets the configuration", "", client_fork: false + end + + context "with --fork" do + it_behaves_like "sets the configuration", "--fork", client_fork: true + end + + context "with --no-fork" do + it_behaves_like "sets the configuration", "--no-fork", client_fork: false + end + + context "with an interval" do + it_behaves_like "sets the configuration", "--interval 1800", client_fork: true + end + + context "with once" do + it_behaves_like "sets the configuration", "--once", client_fork: false + end + + context "with daemonize", :unix_only do + it_behaves_like "sets the configuration", "--daemonize", client_fork: true + end + end + describe "--config-option" do context "with a single value" do it_behaves_like "sets the configuration", "--config-option chef_server_url=http://example", @@ -186,21 +217,16 @@ describe Chef::Application::Client, "reconfigure" do Chef::Config[:splay] = nil end - context "when interval is given" do - before do - Chef::Config[:interval] = 600 - allow(ChefConfig).to receive(:windows?).and_return(false) - end - - it "should terminate with message" do - expect(Chef::Application).to receive(:fatal!).with( -"Unforked chef-client interval runs are disabled in Chef 12. + it "should terminal with message when interval is given" do + Chef::Config[:interval] = 600 + allow(ChefConfig).to receive(:windows?).and_return(false) + expect(Chef::Application).to receive(:fatal!).with( + "Unforked chef-client interval runs are disabled in Chef 12. Configuration settings: interval = 600 seconds Enable chef-client interval runs by setting `:client_fork = true` in your config file or adding `--fork` to your command line options." - ) - app.reconfigure - end + ) + app.reconfigure end context "when interval is given on windows" do |