From feabab6ca5b3630c9c59c5631544701c65e39c7d Mon Sep 17 00:00:00 2001 From: Sam Gunaratne Date: Mon, 15 Jul 2024 18:38:22 +0000 Subject: [PATCH] Add revision version to app logs * Add new configuration to all job schemas that interact with diego. * Even if the app has the revision feature disabled we still show the revision in the logs if it has a revision associated with it. --- config/cloud_controller.yml | 2 + .../config_schemas/vms/api_schema.rb | 4 +- .../config_schemas/vms/clock_schema.rb | 4 +- .../vms/deployment_updater_schema.rb | 4 +- .../config_schemas/vms/worker_schema.rb | 4 +- .../diego/main_lrp_action_builder.rb | 10 +++- .../diego/main_lrp_action_builder_spec.rb | 52 +++++++++++++++++++ 7 files changed, 75 insertions(+), 5 deletions(-) diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index dfa23b460c1..b550eaa5435 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -390,3 +390,5 @@ custom_metric_tag_prefix_list: ["metric.tag.cloudfoundry.org"] max_manifest_service_binding_poll_duration_in_seconds: 60 update_metric_tags_on_rename: true + +app_log_revision: true diff --git a/lib/cloud_controller/config_schemas/vms/api_schema.rb b/lib/cloud_controller/config_schemas/vms/api_schema.rb index e942b95271b..dfc7ff505ff 100644 --- a/lib/cloud_controller/config_schemas/vms/api_schema.rb +++ b/lib/cloud_controller/config_schemas/vms/api_schema.rb @@ -38,7 +38,9 @@ class ApiSchema < VCAP::Config docker_staging_stack: String, optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass), enable_declarative_asset_downloads: bool - } + }, + + app_log_revision: bool } end diff --git a/lib/cloud_controller/config_schemas/vms/clock_schema.rb b/lib/cloud_controller/config_schemas/vms/clock_schema.rb index 9130887c95a..c9ad630c193 100644 --- a/lib/cloud_controller/config_schemas/vms/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/vms/clock_schema.rb @@ -35,7 +35,9 @@ class ClockSchema < VCAP::Config use_privileged_containers_for_staging: bool, optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass), enable_declarative_asset_downloads: bool - } + }, + + app_log_revision: bool } end diff --git a/lib/cloud_controller/config_schemas/vms/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/vms/deployment_updater_schema.rb index 3386498d893..3433837489b 100644 --- a/lib/cloud_controller/config_schemas/vms/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/vms/deployment_updater_schema.rb @@ -35,7 +35,9 @@ class DeploymentUpdaterSchema < VCAP::Config use_privileged_containers_for_staging: bool, optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass), enable_declarative_asset_downloads: bool - } + }, + + app_log_revision: bool } end diff --git a/lib/cloud_controller/config_schemas/vms/worker_schema.rb b/lib/cloud_controller/config_schemas/vms/worker_schema.rb index 3853b944d61..ba1bdb68d53 100644 --- a/lib/cloud_controller/config_schemas/vms/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/vms/worker_schema.rb @@ -35,7 +35,9 @@ class WorkerSchema < VCAP::Config use_privileged_containers_for_staging: bool, optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass), enable_declarative_asset_downloads: bool - } + }, + + app_log_revision: bool } end diff --git a/lib/cloud_controller/diego/main_lrp_action_builder.rb b/lib/cloud_controller/diego/main_lrp_action_builder.rb index 44bf80a26f3..95a4a8122d3 100644 --- a/lib/cloud_controller/diego/main_lrp_action_builder.rb +++ b/lib/cloud_controller/diego/main_lrp_action_builder.rb @@ -60,11 +60,19 @@ def generate_app_action(start_command, user, environment_variables) path: '/tmp/lifecycle/launcher', args: launcher_args, env: environment_variables, - log_source: "APP/PROC/#{process.type.upcase}", + log_source: app_log_source, resource_limits: ::Diego::Bbs::Models::ResourceLimits.new(nofile: process.file_descriptors) )) end + def app_log_source + if VCAP::CloudController::Config.config.get(:app_log_revision) && process.revision + "APP/REV/#{process.revision.version}/PROC/#{process.type.upcase}" + else + "APP/PROC/#{process.type.upcase}" + end + end + def allow_ssh? process.enable_ssh end diff --git a/spec/unit/lib/cloud_controller/diego/main_lrp_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/main_lrp_action_builder_spec.rb index 9d3c391614a..f47e2995b9d 100644 --- a/spec/unit/lib/cloud_controller/diego/main_lrp_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/main_lrp_action_builder_spec.rb @@ -103,6 +103,58 @@ module Diego each { |env_vars| expect(env_vars).not_to include(an_object_satisfying { |var| var.name == 'VCAP_PLATFORM_OPTIONS' }) } end + context 'revisions' do + let(:revision) { RevisionModel.make(app: app_model, version: 99) } + + before do + process.revision = revision + end + + context 'when the app_log_revision config is enabled' do + before do + TestConfig.config[:app_log_revision] = true + end + + it 'shows the revision id in the log source' do + action = MainLRPActionBuilder.build(process, lrp_builder, ssh_key) + expect(action.codependent_action.actions.first.run_action.log_source).to eq('APP/REV/99/PROC/WEB') + end + + context 'and the revision is not present' do + before do + process.update(revision: nil) + end + + it 'does not show revision in the log source' do + action = MainLRPActionBuilder.build(process, lrp_builder, ssh_key) + expect(action.codependent_action.actions.first.run_action.log_source).to eq('APP/PROC/WEB') + end + end + + context 'and when the app feature revisions is disabled' do + before do + app_model.update(revisions_enabled: false) + end + + it 'still shows existing revisions in the log source' do + action = MainLRPActionBuilder.build(process, lrp_builder, ssh_key) + expect(action.codependent_action.actions.first.run_action.log_source).to eq('APP/REV/99/PROC/WEB') + end + end + end + + context 'when the app_log_revision config is disabled' do + before do + TestConfig.config[:app_log_revision] = false + end + + it 'does not show the revision in the log source' do + action = MainLRPActionBuilder.build(process, lrp_builder, ssh_key) + expect(action.codependent_action.actions.first.run_action.log_source).to eq('APP/PROC/WEB') + end + end + end + context 'sidecars' do let(:sidecar_action_environment_variables) do [::Diego::Bbs::Models::EnvironmentVariable.new(name: 'PORT', value: '4444'),