(PE-45697) Fall back to the ssl check when PuppetDB's cleartext status port is unavailable

lib/beaker-puppet/helpers/puppet_helpers.rb

@@ -662,11 +662,40 @@ def sleep_until_puppetdb_started(host, nonssl_port = nil, ssl_port = nil)
             endpoint = 'status/v1/services/puppetdb-status'
             expected_regex = '\"state\" \{0,\}: \{0,\}\"running\"'
           end
-          retry_on(host,
-                   "curl -m 1 http://localhost:#{nonssl_port}/#{endpoint} | grep '#{expected_regex}'",
-                   { max_retries: 120 })
-          curl_with_retries('start puppetdb (ssl)',
-                            host, "https://#{host.node_name}:#{ssl_port}", [35, 60])
+          # The ssl status port is always enabled and is the check this method
+          # ultimately reports the result of, so use it as the primary
+          # liveness gate. Checking the nonssl port FIRST (as this method used
+          # to) meant every host with it disabled -- the default for fresh
+          # installs as of PE-45384/PE-44906 (SECVULN-1792) -- burned through
+          # 120 failed retries (~2 minutes, at retry_on's default 1s
+          # retry_interval) before ever reaching the check that actually
+          # matters (PE-45697).
+          result = curl_with_retries('start puppetdb (ssl)',
+                                     host, "https://#{host.node_name}:#{ssl_port}", [35, 60])
+
+          # Now that ssl liveness is already confirmed, the nonssl check is
+          # just a quick, best-effort secondary confirmation: a small retry
+          # budget is enough, and its absence -- by design, on a host with
+          # the cleartext listener disabled -- shouldn't fail this method.
+          nonssl_status_command = "curl -m 1 http://localhost:#{nonssl_port}/#{endpoint} | grep '#{expected_regex}'"
+          begin
+            retry_on(host, nonssl_status_command, { max_retries: 5 })
+          rescue RuntimeError => e
+            # Only treat this as the known "exhausted all retries" failure
+            # retry_on itself raises for THIS command (a plain RuntimeError
+            # with this exact templated message, see beaker's
+            # Beaker::DSL::Helpers::HostHelpers#retry_on) -- not any other
+            # RuntimeError, or a same-shaped error for a different command,
+            # that might surface from deeper in the retry loop (e.g. an
+            # SSH/connection error from the underlying on() call). Re-raise
+            # anything else unrecognized rather than silently reclassifying
+            # it below (PE-45697).
+            raise unless e.message == "Command `#{nonssl_status_command}` failed."
+
+            logger.warn("sleep_until_puppetdb_started: nonssl status check on port #{nonssl_port} did not succeed (#{e.message}), but ssl status check already confirmed puppetdb is running")
+          end
+
+          result
         end
 
         # Waits until a successful curl check has happened against puppetserver

spec/beaker-puppet/helpers/puppet_helpers_spec.rb

@@ -1034,35 +1034,80 @@ def stub_post_setup
       allow(subject).to receive(:version_is_less).and_return(true)
     end
 
-    it 'uses the default ports if none given' do
+    it 'uses the default ports if none given, checking ssl before nonssl' do
       host = hosts[0]
-      expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
       expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+      expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
       subject.sleep_until_puppetdb_started(host)
     end
 
     it 'allows setting the nonssl_port' do
       host = hosts[0]
-      expect(subject).to receive(:retry_on).with(anything, /8084/, anything).once.ordered
       expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+      expect(subject).to receive(:retry_on).with(anything, /8084/, anything).once.ordered
 
       subject.sleep_until_puppetdb_started(host, 8084)
     end
 
     it 'allows setting the ssl_port' do
       host = hosts[0]
-      expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
       expect(subject).to receive(:curl_with_retries).with(anything, anything, /8085/, anything).once.ordered
+      expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
 
       subject.sleep_until_puppetdb_started(host, nil, 8085)
     end
 
+    context 'when the cleartext status port never responds (e.g. disabled by default)' do
+      it 'swallows the failure once ssl liveness is already confirmed' do
+        host = hosts[0]
+        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+        expect(subject).to receive(:retry_on).with(anything, anything, { max_retries: 5 }).once.ordered do |_host, command, _opts|
+          raise "Command `#{command}` failed."
+        end
+
+        expect { subject.sleep_until_puppetdb_started(host) }.not_to raise_error
+      end
+    end
+
+    context 'when the ssl check itself fails' do
+      it 'raises without ever attempting the nonssl check' do
+        host = hosts[0]
+        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once
+                                                      .and_raise('Command `curl -m 1 https://.../` failed.')
+        expect(subject).not_to receive(:retry_on)
+
+        expect { subject.sleep_until_puppetdb_started(host) }.to raise_error(/https/)
+      end
+    end
+
+    context 'when retry_on raises a RuntimeError unrelated to exhausted retries' do
+      it 'propagates the error rather than treating it as a disabled port' do
+        host = hosts[0]
+        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+        expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
+                                             .and_raise('Host unreachable: no route to host')
+
+        expect { subject.sleep_until_puppetdb_started(host) }.to raise_error(/Host unreachable/)
+      end
+    end
+
+    context 'when retry_on raises a RuntimeError formatted like the templated failure but for a different command' do
+      it 'propagates the error rather than treating it as a disabled port' do
+        host = hosts[0]
+        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+        expect(subject).to receive(:retry_on).with(anything, /8080/, anything).once.ordered
+                                             .and_raise('Command `some other command` failed.')
+
+        expect { subject.sleep_until_puppetdb_started(host) }.to raise_error(/some other command/)
+      end
+    end
+
     context 'when pe_ver is less than 2016.1.0' do
       it 'uses the version endpoint' do
         host = hosts[0]
         host['pe_ver'] = '2015.3.3'
-        expect(subject).to receive(:retry_on).with(anything, %r{pdb/meta/v1/version}, anything).once.ordered
         expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
+        expect(subject).to receive(:retry_on).with(anything, %r{pdb/meta/v1/version}, anything).once.ordered
 
         expect(subject).to receive(:version_is_less).with(host['pe_ver'], '2016.1.0').and_return(true)
         subject.sleep_until_puppetdb_started(host)
@@ -1073,9 +1118,9 @@ def stub_post_setup
       it 'uses the status endpoint' do
         host = hosts[0]
         host['pe_ver'] = '2016.1.0'
+        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
         expect(subject).to receive(:retry_on).with(anything, %r{status/v1/services/puppetdb-status},
                                                    anything).once.ordered
-        expect(subject).to receive(:curl_with_retries).with(anything, anything, /8081/, anything).once.ordered
 
         expect(subject).to receive(:version_is_less).with(host['pe_ver'], '2016.1.0').and_return(false)
         subject.sleep_until_puppetdb_started(host)