(maint) Allow not applying settings catalog on startup

On initialization of a JRuby instance, Puppet will compile and
apply a catalog containing resources for the configured directories
specified in the appropriate settings sections. The goal of this is
to ensure any directories Puppet requires exist. In practice this
should be managed by our packages. Attempting to apply a catalog every
time a JRuby instance is unnecessary, slow, and causes race conditions
when attempting to instantiate JRuby instances in parallel.

Puppet has a setting "settings_catalog" that controls this behavior.
This PR addresses issues that arise when we set "settings_catalog" to
`false`:
 1. In code the process of compiling a settings catalog brings most of
    Puppet's ruby code into scope. When not compiling a settings catalog
    we must actually declare our dependencies.
 2. The $yamldir/facts directory is not created by packages
    despite being used by default.

Author: justinstoller

dev-resources/puppetlabs/services/master/master_service_test/ca_files_test/puppet.conf

@@ -89,6 +89,11 @@ jruby-puppet: {
     # For testing running requests through a single JRuby instance. DO NOT ENABLE unless
     # explicitly testing this functionality.
     # multithreaded: true
+
+    # (optional) When (re)filling a pool one instance will be initialized first, then
+    # the remaining instances will be initialized at the specified level of concurrency.
+    # Set to one for the previous serialized behavior. Default is three.
+    # instance-creation-concurrency: 1
 }
 
 # Settings related to HTTP client requests made by Puppet Server.

dev/puppetserver.conf

@@ -100,7 +100,8 @@
                        :puppet-platform-version 8
                        :java-args ~(str "-Xms2g -Xmx2g "
                                      "-Djruby.logger.class=com.puppetlabs.jruby_utils.jruby.Slf4jLogger")
-                       :create-dirs ["/opt/puppetlabs/server/data/puppetserver/jars"]
+                       :create-dirs ["/opt/puppetlabs/server/data/puppetserver/jars"
+                                     "/opt/puppetlabs/server/data/puppetserver/yaml"]
                        :repo-target "puppet8"
                        :nonfinal-repo-target "puppet8-nightly"
                        :bootstrap-source :services-d

project.clj

@@ -5,8 +5,12 @@
 
 describe Puppet::Server::Logging do
   context 'when setting the log level' do
-    before :each do
-      Puppet::Server::PuppetConfig.initialize_puppet(puppet_config: {})
+    it 'flush logging queue' do
+      # The logger will queue any old log messages, creating a logging destination
+      # will force the pending queue to be flushed to this logger. We don't care
+      # about these messages so we discard the logger, but we do not want them to
+      # interfere with the next tests.
+      _, _ = Puppet::Server::Logging.capture_logs('debug') do; end
     end
 
     it 'correctly filters messages' do

spec/puppet-server-lib/puppet/jvm/logging_spec.rb

@@ -1,3 +1,5 @@
+require 'puppet/util/log'
+
 module Puppet
   module Server
     # Log to an array, just for testing.

src/ruby/puppetserver-lib/puppet/server/log_collector.rb

@@ -1,3 +1,5 @@
+require 'puppet'
+require 'puppet/util'
 require 'puppet/server'
 require 'java'
 

src/ruby/puppetserver-lib/puppet/server/logger.rb

@@ -1,3 +1,5 @@
+require 'puppet'
+require 'puppet/util/log'
 require 'puppet/server/log_collector'
 
 module Puppet

src/ruby/puppetserver-lib/puppet/server/logging.rb

@@ -1,6 +1,13 @@
 require 'puppet/server'
 require 'puppet/server/logger'
 require 'puppet/server/http_client'
+require 'puppet/indirector/indirection'
+require 'puppet/file_serving/content'
+require 'puppet/file_serving/metadata'
+require 'puppet/file_bucket/file'
+require 'puppet/node'
+require 'puppet/application_support'
+require 'puppet/ssl/oids'
 
 class Puppet::Server::PuppetConfig
 
@@ -88,6 +95,8 @@ def self.initialize_puppet(puppet_config:)
       Puppet.push_context(dummy_ssl_context)
     end
 
+    # We have no added support for setting "settings_catalog" to false in the puppet.conf.
+    # We should default to not applying the settings catalog and remove this line in Puppet 9.
     Puppet.settings.use :main, :server, :ssl, :metrics
 
     if Puppet::Indirector::Indirection.method_defined?(:set_global_setting)

src/ruby/puppetserver-lib/puppet/server/puppet_config.rb

@@ -316,45 +316,6 @@
                           (+ (:duration-millis requested-instance)
                              (:time requested-instance))))))))))))))
 
-(deftest ^:integration ca-files-test
-  (testing "CA settings from puppet are honored and the CA
-            files are created when the service starts up"
-    (let [ca-files-test-runtime-dir (str master-service-test-runtime-dir
-                                         "/ca-files-test")
-          ca-files-test-puppet-conf (fs/file test-resources-path
-                                             "ca_files_test/puppet.conf")]
-      (fs/delete-dir ca-files-test-runtime-dir)
-      (testutils/with-puppet-conf-files
-       {"puppet.conf" ca-files-test-puppet-conf}
-       ca-files-test-runtime-dir
-       (logutils/with-test-logging
-        (bootstrap-testutils/with-puppetserver-running
-         app
-         {:jruby-puppet {:gem-path gem-path
-                         :server-conf-dir ca-files-test-runtime-dir
-                         :max-active-instances 1}
-          :webserver {:port 8081}}
-         (let [jruby-service (tk-app/get-service app :JRubyPuppetService)]
-           (jruby-service/with-jruby-puppet
-            jruby-puppet jruby-service :ca-files-test
-            (letfn [(test-path!
-                      [setting expected-path]
-                      (is (= (ks/absolute-path expected-path)
-                             (.getSetting jruby-puppet setting)))
-                      (is (fs/exists? (ks/absolute-path expected-path))))]
-
-              (test-path! "capub" (str ca-files-test-runtime-dir "/ca/ca_pub.pem"))
-              (test-path! "cakey" (str ca-files-test-runtime-dir "/ca/ca_key.pem"))
-              (test-path! "cacert" (str ca-files-test-runtime-dir "/ca/ca_crt.pem"))
-              (test-path! "localcacert" (str ca-files-test-runtime-dir "/ca/ca.pem"))
-              (test-path! "cacrl" (str ca-files-test-runtime-dir "/ca/ca_crl.pem"))
-              (test-path! "hostcrl" (str ca-files-test-runtime-dir "/ca/crl.pem"))
-              (test-path! "hostpubkey" (str ca-files-test-runtime-dir "/public_keys/localhost.pem"))
-              (test-path! "hostprivkey" (str ca-files-test-runtime-dir "/private_keys/localhost.pem"))
-              (test-path! "hostcert" (str ca-files-test-runtime-dir "/certs/localhost.pem"))
-              (test-path! "serial" (str ca-files-test-runtime-dir "/certs/serial"))
-              (test-path! "cert_inventory" (str ca-files-test-runtime-dir "/inventory.txt")))))))))))
-
 (def graphite-enabled-config
   {:metrics {:server-id "localhost"
              :reporters {:graphite {:update-interval-seconds 5000
@@ -729,34 +690,36 @@
        (is (= 404 (:status resp)))))))
 
 (deftest ^:integration facts-upload-api
-  (bootstrap-testutils/with-puppetserver-running
-   app
-   {:jruby-puppet {:gem-path gem-path
-                   :max-active-instances 2 ; we need 2 jruby-instances since processing the upload uses an instance
-                   :server-code-dir test-resources-code-dir
-                   :server-conf-dir master-service-test-runtime-dir
-                   :server-var-dir (fs/tmpdir)}}
-   (let [jruby-service (tk-app/get-service app :JRubyPuppetService)
-         jruby-instance (jruby-testutils/borrow-instance jruby-service :facts-upload-endpoint-test)
-         container (:scripting-container jruby-instance)]
-     (try
-       (let [facts (.runScriptlet container "facts = Puppet::Node::Facts.new('puppet.node.test')
-                                              facts.values['foo'] = 'bar'
-                                              facts.to_json")
-             response (http-put "/puppet/v3/facts/puppet.node.test?environment=production" facts)]
-
-         (testing "Puppet Server responds to PUT requests for /puppet/v3/facts"
-           (is (= 200 (:status response))))
-
-         (testing "Puppet Server saves facts to the configured facts terminus"
-           ;; Ensure the test is configured properly
-           (is (= "yaml" (.runScriptlet container "Puppet::Node::Facts.indirection.terminus_class")))
-           (let [stored-facts (-> (.runScriptlet container "facts = Puppet::Node::Facts.indirection.find('puppet.node.test')
-                                                             (facts.nil? ? {} : facts).to_json")
-                                  (json/parse-string))]
-             (is (= "bar" (get-in stored-facts ["values" "foo"]))))))
-       (finally
-         (jruby-testutils/return-instance jruby-service jruby-instance :facts-upload-endpoint-test))))))
+  (let [tmpdir (fs/tmpdir)]
+    (fs/mkdir (str tmpdir "/yaml"))
+    (bootstrap-testutils/with-puppetserver-running
+     app
+     {:jruby-puppet {:gem-path gem-path
+                     :max-active-instances 2 ; we need 2 jruby-instances since processing the upload uses an instance
+                     :server-code-dir test-resources-code-dir
+                     :server-conf-dir master-service-test-runtime-dir
+                     :server-var-dir (fs/tmpdir)}}
+     (let [jruby-service (tk-app/get-service app :JRubyPuppetService)
+           jruby-instance (jruby-testutils/borrow-instance jruby-service :facts-upload-endpoint-test)
+           container (:scripting-container jruby-instance)]
+       (try
+         (let [facts (.runScriptlet container "facts = Puppet::Node::Facts.new('puppet.node.test')
+                                                facts.values['foo'] = 'bar'
+                                                facts.to_json")
+               response (http-put "/puppet/v3/facts/puppet.node.test?environment=production" facts)]
+
+           (testing "Puppet Server responds to PUT requests for /puppet/v3/facts"
+             (is (= 200 (:status response))))
+
+           (testing "Puppet Server saves facts to the configured facts terminus"
+             ;; Ensure the test is configured properly
+             (is (= "yaml" (.runScriptlet container "Puppet::Node::Facts.indirection.terminus_class")))
+             (let [stored-facts (-> (.runScriptlet container "facts = Puppet::Node::Facts.indirection.find('puppet.node.test')
+                                                               (facts.nil? ? {} : facts).to_json")
+                                    (json/parse-string))]
+               (is (= "bar" (get-in stored-facts ["values" "foo"]))))))
+         (finally
+           (jruby-testutils/return-instance jruby-service jruby-instance :facts-upload-endpoint-test)))))))
 
 (deftest ^:integration v4-queue-limit
   (bootstrap-testutils/with-puppetserver-running