From a49d8b1eec9399620e585f51eca0656c99ba71b3 Mon Sep 17 00:00:00 2001 From: Sven Krieger <37476281+svkrieger@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:23:48 +0100 Subject: [PATCH] Re-add `cc.experimental.use_puma_webserver` --- jobs/cloud_controller_ng/spec | 9 +- .../cloud_controller_ng/templates/bpm.yml.erb | 10 +- .../templates/cloud_controller_ng.yml.erb | 12 +- jobs/valkey/monit | 10 +- spec/cloud_controller_ng/bpm_spec.rb | 51 +++++++- .../cloud_controller_ng_spec.rb | 118 ++++++++++++++++-- spec/valkey/monit_spec.rb | 63 ++++++++-- 7 files changed, 248 insertions(+), 25 deletions(-) diff --git a/jobs/cloud_controller_ng/spec b/jobs/cloud_controller_ng/spec index 307ff1b302..fc5988cb2a 100644 --- a/jobs/cloud_controller_ng/spec +++ b/jobs/cloud_controller_ng/spec @@ -223,6 +223,7 @@ provides: - ssl.skip_cert_verify - system_domain - uaa.clients.cc_routing.secret + - cc.experimental.use_puma_webserver - cc.experimental.use_redis - cc.app_log_revision - cc.app_instance_stopping_state @@ -1277,5 +1278,11 @@ properties: default: false cc.temporary_enable_deprecated_thin_webserver: - description: "Use deprecated Thin webserver. Please note that when using Thin instead of Puma you miss out on the following benefits: Better resource utilization, well maintained and more performant. Thin will be removed in a future release." + description: "Use deprecated Thin webserver. Please note that when using Thin instead of Puma you miss out on the following benefits: Better resource utilization, well maintained and more performant. Thin will be removed in a future release. `cc.experimental.use_puma_webserver` takes precedence over this." default: false + + +# deprecated configuration + + cc.experimental.use_puma_webserver: + description: "Deprecated as Puma is now the default. This config flag will be removed in the future. Currently it takes precedence over `cc.temporary_enable_deprecated_thin_webserver` i.e. when set to false Thin will be used." diff --git a/jobs/cloud_controller_ng/templates/bpm.yml.erb b/jobs/cloud_controller_ng/templates/bpm.yml.erb index 1cc185da1a..897f9a9802 100644 --- a/jobs/cloud_controller_ng/templates/bpm.yml.erb +++ b/jobs/cloud_controller_ng/templates/bpm.yml.erb @@ -12,8 +12,16 @@ def mount_nfs_volume!(config) end end +def thin_webserver_enabled? + if_p('cc.experimental.use_puma_webserver') do |prop| + return !prop + end + + p('cc.temporary_enable_deprecated_thin_webserver') +end + def mount_valkey_volume!(config) - unless p("cc.temporary_enable_deprecated_thin_webserver") && !p("cc.experimental.use_redis") + unless thin_webserver_enabled? && !p("cc.experimental.use_redis") config['additional_volumes'] = [] unless config.key?('additional_volumes') config['additional_volumes'] << { "path" => "/var/vcap/data/valkey", diff --git a/jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb b/jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb index 62fd5235fb..132a55e70d 100644 --- a/jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb +++ b/jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb @@ -42,6 +42,14 @@ end raise "Error for database_encryption: #{active_keys.length} were marked active. Only one key may be active" end + + def thin_webserver_enabled? + if_p('cc.experimental.use_puma_webserver') do |prop| + return !prop + end + + p('cc.temporary_enable_deprecated_thin_webserver') +end %> --- #Actually NGX host and port @@ -53,7 +61,7 @@ local_route: <%= p('cc.nginx.ip') %> external_port: <%= p("cc.external_port") %> tls_port: <%= p("cc.tls_port") %> internal_service_hostname: <%= p("cc.internal_service_hostname") %> -webserver: <%= p("cc.temporary_enable_deprecated_thin_webserver") ? 'thin' : 'puma' %> +webserver: <%= thin_webserver_enabled? ? 'thin' : 'puma' %> puma: workers: <%= p("cc.puma.workers") %> max_threads: <%= p("cc.puma.max_threads") %> @@ -490,7 +498,7 @@ rate_limiter_v2_api: temporary_enable_v2: <%= p("cc.temporary_enable_v2") %> -<% unless p("cc.temporary_enable_deprecated_thin_webserver") && !p("cc.experimental.use_redis") %> +<% unless thin_webserver_enabled? && !p("cc.experimental.use_redis") %> redis: socket: "/var/vcap/data/valkey/valkey.sock" <% end %> diff --git a/jobs/valkey/monit b/jobs/valkey/monit index d71b56612b..2876fe21e2 100644 --- a/jobs/valkey/monit +++ b/jobs/valkey/monit @@ -1,6 +1,14 @@ <% + def thin_webserver_enabled?(link) + link.if_p('cc.experimental.use_puma_webserver') do |prop| + return !prop + end + + link.p('cc.temporary_enable_deprecated_thin_webserver') + end + cloud_controller_internal = link("cloud_controller_internal") - unless cloud_controller_internal.p("cc.temporary_enable_deprecated_thin_webserver") && !cloud_controller_internal.p("cc.experimental.use_redis") + unless thin_webserver_enabled?(cloud_controller_internal) && !cloud_controller_internal.p("cc.experimental.use_redis") %> check process valkey diff --git a/spec/cloud_controller_ng/bpm_spec.rb b/spec/cloud_controller_ng/bpm_spec.rb index df9345758e..ab98bbc38e 100644 --- a/spec/cloud_controller_ng/bpm_spec.rb +++ b/spec/cloud_controller_ng/bpm_spec.rb @@ -113,7 +113,7 @@ def valkey_volume_mounted?(process) end describe 'valkey config' do - context 'when the puma webserver is used' do + context 'when the puma webserver is used by default' do it 'mounts the valkey volume into the ccng job container' do template_hash = YAML.safe_load(template.render({}, consumes: {})) @@ -123,7 +123,30 @@ def valkey_volume_mounted?(process) end end - context 'when thin webserver is used' do + context 'when the puma webserver is enabled by deprecated config' do + it 'mounts the valkey volume into the ccng job container' do + template_hash = YAML.safe_load(template.render({ 'cc' => { 'experimental' => { 'use_puma_webserver' => true } } }, consumes: {})) + + results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') } + expect(results.length).to eq(1) + expect(valkey_volume_mounted?(results[0])).to be_truthy + end + + context 'when `cc.temporary_enable_deprecated_thin_webserver` is also enabled' do + it 'still uses Puma and mounts the valkey volume into the ccng job container' do + template_hash = YAML.safe_load(template.render( + { 'cc' => { 'experimental' => { 'use_puma_webserver' => true }, + 'temporary_enable_deprecated_thin_webserver' => true } }, consumes: {} + )) + + results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') } + expect(results.length).to eq(1) + expect(valkey_volume_mounted?(results[0])).to be_truthy + end + end + end + + context 'when thin webserver is explicitly enabled' do context "when 'cc.experimental.use_redis' is set to 'true'" do it 'mounts the valkey volume into the ccng job container' do template_hash = YAML.safe_load( @@ -147,6 +170,30 @@ def valkey_volume_mounted?(process) end end end + + context 'when thin webserver is implicitly enabled through `cc.experimental.use_puma_webserver` => false' do + context "when 'cc.experimental.use_redis' is set to 'true'" do + it 'mounts the valkey volume into the ccng job container' do + template_hash = YAML.safe_load( + template.render({ 'cc' => { 'experimental' => { 'use_puma_webserver' => false, 'use_redis' => true } } }, consumes: {}) + ) + + results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') } + expect(results.length).to eq(1) + expect(valkey_volume_mounted?(results[0])).to be_truthy + end + end + + context "when 'cc.experimental.use_redis' is not set'" do + it 'mounts the valkey volume into the ccng job container' do + template_hash = YAML.safe_load(template.render({ 'cc' => { 'temporary_enable_deprecated_thin_webserver' => true } }, consumes: {})) + + results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') } + expect(results.length).to eq(1) + expect(valkey_volume_mounted?(results[0])).to be_falsey + end + end + end end end end diff --git a/spec/cloud_controller_ng/cloud_controller_ng_spec.rb b/spec/cloud_controller_ng/cloud_controller_ng_spec.rb index fa3a025bba..00cdae7640 100644 --- a/spec/cloud_controller_ng/cloud_controller_ng_spec.rb +++ b/spec/cloud_controller_ng/cloud_controller_ng_spec.rb @@ -106,8 +106,7 @@ module Test 'name' => 'cflinuxfs4' }], 'staging_upload_password' => '((cc_staging_upload_password))', 'staging_upload_user' => 'staging_user', - 'temporary_enable_v2' => true, - 'experimental' => {} }, + 'temporary_enable_v2' => true }, 'ccdb' => { 'databases' => [{ 'name' => 'cloud_controller', 'tag' => 'cc' }], 'db_scheme' => 'mysql', @@ -588,29 +587,128 @@ module Test end end + describe 'webserver config' do + it 'uses Puma by default' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['webserver']).to eq('puma') + end + + context 'when the puma webserver is enabled by deprecated config' do + before do + merged_manifest_properties['cc']['experimental']['use_puma_webserver'] = true + end + + it 'uses Puma' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['webserver']).to eq('puma') + end + + context 'when `cc.temporary_enable_deprecated_thin_webserver` is also enabled' do + before do + merged_manifest_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'uses Puma' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['webserver']).to eq('puma') + end + end + end + + context 'when the puma webserver is disabled by deprecated config' do + before do + merged_manifest_properties['cc']['experimental']['use_puma_webserver'] = false + end + + it 'uses Thin' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['webserver']).to eq('thin') + end + end + + context 'when `cc.temporary_enable_deprecated_thin_webserver` is enabled' do + before do + merged_manifest_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'uses Thin' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['webserver']).to eq('thin') + end + end + end + describe 'valkey config' do - context 'when the puma webserver is used' do + context 'when the puma webserver is used by default' do it 'renders the valkey socket into the ccng config' do template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) expect(template_hash['redis']['socket']).to eq('/var/vcap/data/valkey/valkey.sock') end end - context 'when thin webserver is used' do - context "when 'cc.experimental.use_redis' is set to 'true'" do - it 'renders the valkey socket into the ccng config' do + context 'when the puma webserver is enabled by deprecated config' do + before do + merged_manifest_properties['cc']['experimental']['use_puma_webserver'] = true + end + + it 'renders the valkey socket into the ccng config' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['redis']['socket']).to eq('/var/vcap/data/valkey/valkey.sock') + end + + context 'when `cc.temporary_enable_deprecated_thin_webserver` is also enabled' do + before do merged_manifest_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'still uses Puma and renders the valkey socket into the ccng config' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash['redis']['socket']).to eq('/var/vcap/data/valkey/valkey.sock') + end + end + end + + context 'when thin webserver is explicitly enabled' do + before do + merged_manifest_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'does not render the valkey socket into the ccng config' do + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash).not_to have_key('redis') + end + + context "when 'cc.experimental.use_redis' is set to 'true'" do + before do merged_manifest_properties['cc']['experimental']['use_redis'] = true + end + + it 'renders the valkey socket into the ccng config' do template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) expect(template_hash['redis']['socket']).to eq('/var/vcap/data/valkey/valkey.sock') end end + end + + context 'when thin webserver is implicitly enabled through `cc.experimental.use_puma_webserver` => false' do + before do + merged_manifest_properties['cc']['experimental']['use_puma_webserver'] = false + end - context "when 'cc.experimental.use_redis' is not set'" do - it 'does not render the valkey socket into the ccng config' do - merged_manifest_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + it 'does not render the valkey socket into the ccng config' do + merged_manifest_properties['cc']['experimental']['use_puma_webserver'] = false + template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) + expect(template_hash).not_to have_key('redis') + end + + context "when 'cc.experimental.use_redis' is set to 'true'" do + before do + merged_manifest_properties['cc']['experimental']['use_redis'] = true + end + + it 'renders the valkey socket into the ccng config' do template_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links)) - expect(template_hash).not_to have_key('redis') + expect(template_hash['redis']['socket']).to eq('/var/vcap/data/valkey/valkey.sock') end end end diff --git a/spec/valkey/monit_spec.rb b/spec/valkey/monit_spec.rb index 9b20b5ba7f..7959dab113 100644 --- a/spec/valkey/monit_spec.rb +++ b/spec/valkey/monit_spec.rb @@ -18,28 +18,75 @@ module Test describe 'monit' do let(:template) { Template.new(spec, File.join(job_path, 'monit')) } - context 'when the puma webserver is used' do + context 'when the puma webserver is used by default' do it 'renders the monit directives' do result = template.render({}, consumes: [link]) expect(result).to include('check process valkey') end end - context 'when thin webserver is used' do - context "when 'cc.experimental.use_redis' is set to 'true'" do - it 'renders the monit directives' do + context 'when the puma webserver is enabled by deprecated config' do + before do + cc_internal_properties['cc']['experimental']['use_puma_webserver'] = true + end + + it 'renders the monit directives' do + result = template.render({}, consumes: [link]) + expect(result).to include('check process valkey') + end + + context 'when `cc.temporary_enable_deprecated_thin_webserver` is also enabled' do + before do cc_internal_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'renders the monit directives' do + result = template.render({}, consumes: [link]) + expect(result).to include('check process valkey') + end + end + end + + context 'when the puma webserver is disabled by deprecated config' do + before do + cc_internal_properties['cc']['experimental']['use_puma_webserver'] = false + end + + it 'does not render the monit directives' do + result = template.render({}, consumes: [link]) + expect(result.strip).to eq('') + end + + context "when 'cc.experimental.use_redis' is set to 'true'" do + before do cc_internal_properties['cc']['experimental']['use_redis'] = true + end + + it 'renders the monit directives' do result = template.render({}, consumes: [link]) expect(result).to include('check process valkey') end end + end - context "when 'cc.experimental.use_redis' is set to 'false'" do - it 'does not render the monit directives' do - cc_internal_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + context 'when `cc.temporary_enable_deprecated_thin_webserver` is enabled' do + before do + cc_internal_properties['cc']['temporary_enable_deprecated_thin_webserver'] = true + end + + it 'does not render the monit directives' do + result = template.render({}, consumes: [link]) + expect(result.strip).to eq('') + end + + context "when 'cc.experimental.use_redis' is set to 'true'" do + before do + cc_internal_properties['cc']['experimental']['use_redis'] = true + end + + it 'renders the monit directives' do result = template.render({}, consumes: [link]) - expect(result.strip).to eq('') + expect(result).to include('check process valkey') end end end