Skip to content

Commit 55ee961

Browse files
authored
extra flag parameter for storage-cli and logging stderr log stream (#4795)
* feat: debug flag added for storage-cli * feat: request_id and user_guid added into logs as data * fix: log_data removed - we don't need to set since request_id already set in request.rb by either vcap_request_context_setter.rb or by logging_context_job.rb - user_guid is also available in request.rb but only set by security_context_setter.rb so not available in jobs context * fix: log level set to info * feat: storage_cli_flag_options string added into config * feat: storage_cli_flag_optionals added into schemas * refactor: extra flag option paratmeter renamed to storage_cli_optional_flags * refactor: cli_log_file function is removed * test: unit tests for additional_flags function are added * fix: extra debug logging and unused debug attribute are removed
1 parent 7374545 commit 55ee961

File tree

7 files changed

+43
-6
lines changed

7 files changed

+43
-6
lines changed

config/cloud_controller.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ storage_cli_config_file_droplets: config/storage_cli_config_droplets.json
325325
storage_cli_config_file_packages: config/storage_cli_config_packages.json
326326
storage_cli_config_file_buildpacks: config/storage_cli_config_buildpacks.json
327327
storage_cli_config_file_resource_pool: config/storage_cli_config_resource_pool.json
328+
storage_cli_optional_flags: ""
328329

329330
newrelic_enabled: false
330331

lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class StorageCliClient < BaseClient
2525
'Google' => 'gcs'
2626
}.freeze
2727

28-
IMPLEMENTED_PROVIDERS = %w[AzureRM aliyun].freeze
28+
IMPLEMENTED_PROVIDERS = %w[AzureRM aliyun Google].freeze
2929

3030
def initialize(directory_key:, resource_type:, root_dir:, min_size: nil, max_size: nil)
3131
raise 'Missing resource_type' if resource_type.nil?
@@ -167,11 +167,21 @@ def ensure_bucket_exists
167167

168168
private
169169

170+
def additional_flags
171+
flags_string = VCAP::CloudController::Config.config.get(:storage_cli_optional_flags)
172+
return [] if flags_string.nil? || flags_string.empty?
173+
174+
flags_string.split
175+
end
176+
170177
def run_cli(command, *args, allow_exit_code_three: false)
171-
logger.info("running storage-cli: #{@cli_path} -c #{@config_file} #{command} #{args.join(' ')}")
178+
logger.info("running storage-cli: #{@cli_path} -s #{@storage_type} -c #{@config_file} #{additional_flags.join(' ')} #{command} #{args.join(' ')}")
172179

173180
begin
174-
stdout, stderr, status = Open3.capture3(@cli_path, '-s', @storage_type, '-c', @config_file, command, *args)
181+
stdout, stderr, status = Open3.capture3(@cli_path, '-s', @storage_type, '-c', @config_file, *additional_flags, command, *args)
182+
stderr.split("\n").each do |line|
183+
logger.info("[INFO] storage-cli: #{line}")
184+
end
175185
rescue StandardError => e
176186
raise BlobstoreError.new(e.inspect)
177187
end

lib/cloud_controller/config_schemas/api_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ class ApiSchema < VCAP::Config
100100
optional(:storage_cli_config_file_packages) => String,
101101
optional(:storage_cli_config_file_resource_pool) => String,
102102
optional(:storage_cli_config_file_droplets) => String,
103+
optional(:storage_cli_optional_flags) => String,
103104

104105
newrelic_enabled: bool,
105106

lib/cloud_controller/config_schemas/clock_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class ClockSchema < VCAP::Config
5454
optional(:storage_cli_config_file_packages) => String,
5555
optional(:storage_cli_config_file_resource_pool) => String,
5656
optional(:storage_cli_config_file_droplets) => String,
57+
optional(:storage_cli_optional_flags) => String,
5758

5859
newrelic_enabled: bool,
5960

lib/cloud_controller/config_schemas/deployment_updater_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class DeploymentUpdaterSchema < VCAP::Config
1919
optional(:storage_cli_config_file_packages) => String,
2020
optional(:storage_cli_config_file_resource_pool) => String,
2121
optional(:storage_cli_config_file_droplets) => String,
22+
optional(:storage_cli_optional_flags) => String,
2223

2324
readiness_port: {
2425
deployment_updater: Integer

lib/cloud_controller/config_schemas/worker_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class WorkerSchema < VCAP::Config
4646
optional(:storage_cli_config_file_packages) => String,
4747
optional(:storage_cli_config_file_resource_pool) => String,
4848
optional(:storage_cli_config_file_droplets) => String,
49+
optional(:storage_cli_optional_flags) => String,
4950

5051
newrelic_enabled: bool,
5152

spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_client_spec.rb

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ module Blobstore
4949

5050
expect do
5151
StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets')
52-
end.to raise_error(RuntimeError, 'Unimplemented provider: UnknownProvider, implemented ones are: AzureRM, aliyun')
52+
end.to raise_error(RuntimeError, 'Unimplemented provider: UnknownProvider, implemented ones are: AzureRM, aliyun, Google')
5353

5454
droplets_cfg.close!
5555
end
@@ -90,7 +90,7 @@ module Blobstore
9090
end
9191
end
9292

93-
allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil))
93+
allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil, debug: nil))
9494
end
9595

9696
after do
@@ -266,7 +266,7 @@ def build_client(resource_type)
266266
tmp_cfg.path
267267
end
268268
end
269-
allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil))
269+
allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil, debug: nil))
270270
end
271271

272272
after { tmp_cfg.close! }
@@ -284,6 +284,28 @@ def build_client(resource_type)
284284
let(:deletable_blob) { StorageCliBlob.new('deletable-blob') }
285285
let(:dest_path) { File.join(Dir.mktmpdir, SecureRandom.uuid) }
286286

287+
describe 'optional flags' do
288+
context 'when there is no extra flags' do
289+
before do
290+
allow(VCAP::CloudController::Config.config).to receive(:get).with(:storage_cli_optional_flags).and_return('')
291+
end
292+
293+
it('returns empty list') {
294+
expect(client.send(:additional_flags)).to eq([])
295+
}
296+
end
297+
298+
context 'when there is extra flags' do
299+
before do
300+
allow(VCAP::CloudController::Config.config).to receive(:get).with(:storage_cli_optional_flags).and_return('-log-level warn -log-file some/path/storage-cli.log')
301+
end
302+
303+
it('returns empty list') {
304+
expect(client.send(:additional_flags)).to eq(['-log-level', 'warn', '-log-file', 'some/path/storage-cli.log'])
305+
}
306+
end
307+
end
308+
287309
describe '#exists?' do
288310
context 'when the blob exists' do
289311
before { allow(client).to receive(:run_cli).with('exists', any_args).and_return([nil, instance_double(Process::Status, exitstatus: 0)]) }

0 commit comments

Comments
 (0)