diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d8b4ab7a415..c5351b686db 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -85,8 +85,6 @@ Lint/UnusedMethodArgument: Lint/UselessMethodDefinition: Exclude: - 'app/messages/route_destination_update_message.rb' - - 'app/presenters/v3/buildpack_presenter.rb' - - 'app/presenters/v3/process_presenter.rb' - 'spec/support/fake_front_controller.rb' # Offense count: 791 diff --git a/app/controllers/v3/processes_controller.rb b/app/controllers/v3/processes_controller.rb index ecfdce92dfc..876e48a6277 100644 --- a/app/controllers/v3/processes_controller.rb +++ b/app/controllers/v3/processes_controller.rb @@ -1,6 +1,8 @@ require 'presenters/v3/paginated_list_presenter' require 'presenters/v3/process_presenter' require 'presenters/v3/process_stats_presenter' +require 'presenters/v3/process_instances_presenter' +require 'decorators/embed_process_instances_decorator' require 'cloud_controller/paging/pagination_options' require 'actions/process_delete' require 'fetchers/process_list_fetcher' @@ -9,6 +11,7 @@ require 'actions/process_terminate' require 'actions/process_update' require 'messages/process_scale_message' +require 'messages/process_show_message' require 'messages/process_update_message' require 'messages/processes_list_message' require 'controllers/v3/mixins/app_sub_resource' @@ -40,17 +43,30 @@ def index end end + decorators = [] + decorators << EmbedProcessInstancesDecorator if EmbedProcessInstancesDecorator.match?(message.embed) + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( presenter: Presenters::V3::ProcessPresenter, paginated_result: SequelPaginator.new.get_page(dataset, message.try(:pagination_options)), path: base_url(resource: 'processes'), - message: message + message: message, + decorators: decorators ) end def show - # TODO - render status: :ok, json: Presenters::V3::ProcessPresenter.new(@process, show_secrets: permission_queryer.can_read_secrets_in_space?(@space.id, @space.organization_id)) + message = ProcessShowMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + + decorators = [] + decorators << EmbedProcessInstancesDecorator if EmbedProcessInstancesDecorator.match?(message.embed) + + render status: :ok, json: Presenters::V3::ProcessPresenter.new( + @process, + show_secrets: permission_queryer.can_read_secrets_in_space?(@space.id, @space.organization_id), + decorators: decorators + ) end def update @@ -106,6 +122,12 @@ def stats render status: :ok, json: Presenters::V3::ProcessStatsPresenter.new(@process.type, process_stats) end + def process_instances + instances = instances_reporters.instances_for_processes([@process]) + + render status: :ok, json: Presenters::V3::ProcessInstancesPresenter.new(instances[@process.guid], @process) + end + private def find_process_and_space diff --git a/app/decorators/embed_process_instances_decorator.rb b/app/decorators/embed_process_instances_decorator.rb new file mode 100644 index 00000000000..19ac77deba6 --- /dev/null +++ b/app/decorators/embed_process_instances_decorator.rb @@ -0,0 +1,48 @@ +module VCAP::CloudController + class EmbedProcessInstancesDecorator + class << self + def match?(embed) + embed&.include?('process_instances') + end + + def decorate(hash, processes) + instances_reporters = CloudController::DependencyLocator.instance.instances_reporters + instances = instances_reporters.instances_for_processes(processes) + + if hash.key?(:resources) + # Decorate PaginatedListPresenter + processes.each do |process| + resource_index = hash[:resources].find_index { |resource| resource[:guid] == process.guid } + next unless resource_index # Should not happen... + + hash[:resources][resource_index] = embed_process_instances(hash[:resources][resource_index], process_instances(instances, process.guid)) + end + else + # Decorate ProcessPresenter + hash = embed_process_instances(hash, process_instances(instances, hash[:guid])) + end + + hash + end + + private + + def process_instances(instances, process_guid) + instances[process_guid].map do |index, instance| + { + index: index, + state: instance[:state], + since: instance[:since] + } + end + end + + def embed_process_instances(resource_hash, process_instances) + hash_as_array = resource_hash.to_a + before_relationships = hash_as_array.index { |k, _| k == :relationships } || hash_as_array.length + hash_as_array.insert(before_relationships, [:process_instances, process_instances]) + hash_as_array.to_h + end + end + end +end diff --git a/app/messages/base_message.rb b/app/messages/base_message.rb index 39cf4b25120..635146543f7 100644 --- a/app/messages/base_message.rb +++ b/app/messages/base_message.rb @@ -139,6 +139,22 @@ def validate(record) end end + class EmbedParamValidator < ActiveModel::Validator + def validate(record) + return unless record.requested?(:embed) + + key_counts = Hash.new(0) + record.embed.each do |embed_candidate| + if options[:valid_values].member?(embed_candidate) + key_counts[embed_candidate] += 1 + record.errors.add(:base, message: "Duplicate embedded resource: '#{embed_candidate}'") if key_counts[embed_candidate] == 2 + else + record.errors.add(:base, message: "Invalid embedded resource: '#{embed_candidate}'. Valid embedded resources are: '#{options[:valid_values].join("', '")}'") + end + end + end + end + class LifecycleTypeParamValidator < ActiveModel::Validator def validate(record) return unless record.requested?(:lifecycle_type) diff --git a/app/messages/process_show_message.rb b/app/messages/process_show_message.rb new file mode 100644 index 00000000000..c3fecafa603 --- /dev/null +++ b/app/messages/process_show_message.rb @@ -0,0 +1,14 @@ +require 'messages/base_message' + +module VCAP::CloudController + class ProcessShowMessage < BaseMessage + register_allowed_keys [:embed] + + validates_with NoAdditionalParamsValidator + validates_with EmbedParamValidator, valid_values: ['process_instances'] + + def self.from_params(params) + super(params, %w[embed]) + end + end +end diff --git a/app/messages/processes_list_message.rb b/app/messages/processes_list_message.rb index 270e3380ea2..133f7be3ff6 100644 --- a/app/messages/processes_list_message.rb +++ b/app/messages/processes_list_message.rb @@ -8,15 +8,19 @@ class ProcessesListMessage < MetadataListMessage space_guids organization_guids app_guids + embed ] validates_with NoAdditionalParamsValidator # from BaseMessage + validates_with EmbedParamValidator, valid_values: ['process_instances'] + # validates :space_guids, array: true, allow_nil: true + # validates :organization_guids, array: true, allow_nil: true validates :app_guids, array: true, allow_nil: true validate :app_nested_request, if: -> { app_guid.present? } def self.from_params(params) - super(params, %w[types space_guids organization_guids app_guids]) + super(params, %w[types space_guids organization_guids app_guids embed]) end def to_param_hash diff --git a/app/presenters/v3/buildpack_presenter.rb b/app/presenters/v3/buildpack_presenter.rb index ec32547b700..7d004beaa83 100644 --- a/app/presenters/v3/buildpack_presenter.rb +++ b/app/presenters/v3/buildpack_presenter.rb @@ -26,13 +26,6 @@ def to_hash } end - class << self - # :labels and :annotations come from MetadataPresentationHelpers - def associated_resources - super - end - end - private def buildpack diff --git a/app/presenters/v3/process_instances_presenter.rb b/app/presenters/v3/process_instances_presenter.rb new file mode 100644 index 00000000000..218e689de67 --- /dev/null +++ b/app/presenters/v3/process_instances_presenter.rb @@ -0,0 +1,47 @@ +require 'presenters/v3/base_presenter' +require 'presenters/mixins/metadata_presentation_helpers' + +module VCAP::CloudController + module Presenters + module V3 + class ProcessInstancesPresenter < BasePresenter + attr_reader :process + + def initialize(instances, process) + super(instances) + @process = process + end + + def to_hash + { + resources: build_instances, + links: build_links + } + end + + private + + def instances + @resource + end + + def build_instances + instances.map do |index, instance| + { + index: index, + state: instance[:state], + since: instance[:since] + } + end + end + + def build_links + { + self: { href: url_builder.build_url(path: "/v3/processes/#{process.guid}/process_instances") }, + process: { href: url_builder.build_url(path: "/v3/processes/#{process.guid}") } + } + end + end + end + end +end diff --git a/app/presenters/v3/process_presenter.rb b/app/presenters/v3/process_presenter.rb index b83f955f3ba..25e4b19c836 100644 --- a/app/presenters/v3/process_presenter.rb +++ b/app/presenters/v3/process_presenter.rb @@ -8,20 +8,14 @@ module V3 class ProcessPresenter < BasePresenter include VCAP::CloudController::Presenters::Mixins::MetadataPresentationHelpers - class << self - # :labels and :annotations come from MetadataPresentationHelpers - def associated_resources - super - end - end - def to_hash health_check_data = { timeout: process.health_check_timeout, invocation_timeout: process.health_check_invocation_timeout, interval: process.health_check_interval } health_check_data[:endpoint] = process.health_check_http_endpoint if process.health_check_type == HealthCheckTypes::HTTP readiness_health_check_data = { invocation_timeout: process.readiness_health_check_invocation_timeout, interval: process.readiness_health_check_interval } readiness_health_check_data[:endpoint] = process.readiness_health_check_http_endpoint if process.readiness_health_check_type == HealthCheckTypes::HTTP - { + + hash = { guid: process.guid, created_at: process.created_at, updated_at: process.updated_at, @@ -51,6 +45,8 @@ def to_hash }, links: build_links } + + @decorators.reduce(hash) { |memo, d| d.decorate(memo, [process]) } end private diff --git a/config/routes.rb b/config/routes.rb index ccdd5788eaf..d1723e474e0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,7 @@ get '/processes', to: 'processes#index' get '/processes/:process_guid', to: 'processes#show' patch '/processes/:process_guid', to: 'processes#update' + get '/processes/:process_guid/process_instances', to: 'processes#process_instances' delete '/processes/:process_guid/instances/:index', to: 'processes#terminate' post '/processes/:process_guid/actions/scale', to: 'processes#scale' get '/processes/:process_guid/stats', to: 'processes#stats' diff --git a/lib/cloud_controller/backends/instances_reporters.rb b/lib/cloud_controller/backends/instances_reporters.rb index 6c9db092857..7eeacacc446 100644 --- a/lib/cloud_controller/backends/instances_reporters.rb +++ b/lib/cloud_controller/backends/instances_reporters.rb @@ -28,6 +28,7 @@ def stats_for_app(app) end delegate :number_of_starting_and_running_instances_for_processes, :instance_count_summary, to: :diego_reporter + delegate :instances_for_processes, to: :diego_stats_reporter private @@ -36,7 +37,7 @@ def diego_reporter end def diego_stats_reporter - Diego::InstancesStatsReporter.new(dependency_locator.bbs_instances_client, dependency_locator.log_cache_metrics_client) + @diego_stats_reporter ||= Diego::InstancesStatsReporter.new(dependency_locator.bbs_instances_client, dependency_locator.log_cache_metrics_client) end def dependency_locator diff --git a/lib/cloud_controller/diego/bbs_instances_client.rb b/lib/cloud_controller/diego/bbs_instances_client.rb index aff9cd5f897..2a878ef1c1e 100644 --- a/lib/cloud_controller/diego/bbs_instances_client.rb +++ b/lib/cloud_controller/diego/bbs_instances_client.rb @@ -11,7 +11,7 @@ def lrp_instances(process) process_guid = ProcessGuid.from_process(process) logger.info('lrp.instances.request', process_guid:) - actual_lrps_response = handle_diego_errors(process_guid) do + actual_lrps_response = handle_diego_errors do response = @client.actual_lrps_by_process_guid(process_guid) logger.info('lrp.instances.response', process_guid: process_guid, error: response.error) response @@ -20,9 +20,22 @@ def lrp_instances(process) actual_lrps_response.actual_lrps end + def actual_lrps_by_processes(processes) + process_guids = processes.map { |process| ProcessGuid.from_process(process) } + logger.info('actual.lrps.by.processes.request', process_guids:) + + actual_lrps_response = handle_diego_errors do + response = @client.actual_lrps_by_process_guids(process_guids) + logger.info('actual.lrps.by.processes.response', process_guids: process_guids, error: response.error) + response + end + + actual_lrps_response.actual_lrps + end + def desired_lrp_instance(process) process_guid = ProcessGuid.from_process(process) - response = handle_diego_errors(process_guid) do + response = handle_diego_errors(handle_resource_not_found: true, process_guid: process_guid) do @client.desired_lrp_by_process_guid(process_guid) end response.desired_lrp @@ -30,7 +43,7 @@ def desired_lrp_instance(process) private - def handle_diego_errors(process_guid) + def handle_diego_errors(handle_resource_not_found: false, process_guid: nil) begin response = yield rescue ::Diego::Error => e @@ -38,12 +51,11 @@ def handle_diego_errors(process_guid) end if response.error - if response.error.type == ::Diego::Bbs::ErrorTypes::ResourceNotFound + if handle_resource_not_found && response.error.type == ::Diego::Bbs::ErrorTypes::ResourceNotFound raise CloudController::Errors::NoRunningInstances.new("No running instances found for process guid #{process_guid}") end raise CloudController::Errors::InstancesUnavailable.new(response.error.message) - end response diff --git a/lib/cloud_controller/diego/reporters/instances_stats_reporter.rb b/lib/cloud_controller/diego/reporters/instances_stats_reporter.rb index c4ea6e4eb73..ce17657eadc 100644 --- a/lib/cloud_controller/diego/reporters/instances_stats_reporter.rb +++ b/lib/cloud_controller/diego/reporters/instances_stats_reporter.rb @@ -29,13 +29,48 @@ def stats_for_app(process) raise exception end + def instances_for_processes(processes) + logger.debug('instances_for_processes.fetching_actual_lrps') + + # Fetch actual_lrps for all processes + actual_lrps = bbs_instances_client.actual_lrps_by_processes(processes) + + lrps_by_process_guid = actual_lrps.group_by { |lrp| (pg = lrp.actual_lrp_key&.process_guid) && ProcessGuid.cc_process_guid(pg) } + + current_time_since_epoch_ns = Time.now.utc.to_f * 1e9 + results = {} + processes.each do |process| + newest_lrp_by_index = (lrps_by_process_guid[process.guid] || []). + group_by { |lrp| lrp.actual_lrp_key&.index }. + transform_values { |lrps| lrps.max_by { |lrp| lrp.since || 0 } } + + instances = {} + # Fill in the instances up to the max of desired instances and actual instances + [process.instances, newest_lrp_by_index.length].max.times do |idx| + lrp = newest_lrp_by_index[idx] + instances[idx] = if lrp + { + state: LrpStateTranslator.translate_lrp_state(lrp), + since: nanoseconds_to_seconds(current_time_since_epoch_ns - lrp.since) + } + else + { state: VCAP::CloudController::Diego::LRP_DOWN } + end + end + + results[process.guid] = instances + end + + results + end + private attr_reader :bbs_instances_client def get_stats(desired_lrp, process) log_cache_data, log_cache_errors = envelopes(desired_lrp, process) - stats = formatted_process_stats(log_cache_data, Time.now.to_datetime.rfc3339) + stats = formatted_process_stats(log_cache_data, Time.now.utc.to_datetime.rfc3339) quota_stats = formatted_quota_stats(log_cache_data) isolation_segment = desired_lrp.PlacementTags.first [log_cache_errors, stats, quota_stats, isolation_segment] @@ -79,9 +114,9 @@ def build_info(state, actual_lrp, process, stats, quota_stats, log_cache_errors) instance_guid: actual_lrp.actual_lrp_instance_key.instance_guid, port: get_default_port(actual_lrp.actual_lrp_net_info), net_info: actual_lrp_net_info_to_hash(actual_lrp.actual_lrp_net_info), - uptime: nanoseconds_to_seconds((Time.now.to_f * 1e9) - actual_lrp.since), + uptime: nanoseconds_to_seconds((Time.now.utc.to_f * 1e9) - actual_lrp.since), fds_quota: process.file_descriptors - }.merge(metrics_data_for_instance(stats, quota_stats, log_cache_errors, Time.now.to_datetime.rfc3339, actual_lrp.actual_lrp_key.index)) + }.merge(metrics_data_for_instance(stats, quota_stats, log_cache_errors, Time.now.utc.to_datetime.rfc3339, actual_lrp.actual_lrp_key.index)) } info[:details] = actual_lrp.placement_error if actual_lrp.placement_error.present? diff --git a/lib/diego/client.rb b/lib/diego/client.rb index b310bdc405d..790c6b63376 100644 --- a/lib/diego/client.rb +++ b/lib/diego/client.rb @@ -152,6 +152,17 @@ def actual_lrps_by_process_guid(process_guid) protobuf_decode!(response.body, Bbs::Models::ActualLRPsResponse) end + def actual_lrps_by_process_guids(process_guids) + request = protobuf_encode!({ process_guids: }, Bbs::Models::ActualLRPsByProcessGuidsRequest) + + response = with_request_error_handling do + client.post(Routes::ACTUAL_LRPS_BY_PROCESS_GUIDS, request, headers) + end + + validate_status_200!(response) + protobuf_decode!(response.body, Bbs::Models::ActualLRPsByProcessGuidsResponse) + end + def with_request_error_handling delay = 0.25 max_delay = 5 diff --git a/lib/diego/routes.rb b/lib/diego/routes.rb index a38de4c3852..e1f194c5c3b 100644 --- a/lib/diego/routes.rb +++ b/lib/diego/routes.rb @@ -13,5 +13,6 @@ module Routes REMOVE_DESIRED_LRP = '/v1/desired_lrp/remove'.freeze RETIRE_ACTUAL_LRP = '/v1/actual_lrps/retire'.freeze ACTUAL_LRPS = '/v1/actual_lrps/list'.freeze + ACTUAL_LRPS_BY_PROCESS_GUIDS = '/v1/actual_lrps/list_by_process_guids'.freeze end end diff --git a/spec/diego/client_spec.rb b/spec/diego/client_spec.rb index aadcb821293..720225f88a1 100644 --- a/spec/diego/client_spec.rb +++ b/spec/diego/client_spec.rb @@ -668,6 +668,34 @@ module Diego end end + describe '#actual_lrps_by_process_guids' do + let(:actual_lrps) { [::Diego::Bbs::Models::ActualLRP.new, ::Diego::Bbs::Models::ActualLRP.new] } + let(:response_status) { 200 } + let(:response_body) do + Bbs::Models::ActualLRPsByProcessGuidsResponse.encode( + Bbs::Models::ActualLRPsByProcessGuidsResponse.new(error: nil, actual_lrps: actual_lrps) + ).to_s + end + let(:process_guids) { %w[process-guid another-process-guid] } + + before do + stub_request(:post, "#{bbs_url}/v1/actual_lrps/list_by_process_guids").to_return(status: response_status, body: response_body) + end + + it 'returns a LRP instances by process_guids response' do + expected_request = Bbs::Models::ActualLRPsByProcessGuidsRequest.new(process_guids:) + + response = client.actual_lrps_by_process_guids(process_guids) + expect(response).to be_a(Bbs::Models::ActualLRPsByProcessGuidsResponse) + expect(response.error).to be_nil + expect(response.actual_lrps).to eq(actual_lrps) + expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/list_by_process_guids").with( + body: Bbs::Models::ActualLRPsByProcessGuidsRequest.encode(expected_request).to_s, + headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id } + )).to have_been_made.once + end + end + describe '#desired_lrps_scheduling_infos' do let(:scheduling_infos) { [::Diego::Bbs::Models::DesiredLRPSchedulingInfo.new] } let(:response_body) do diff --git a/spec/request/processes_spec.rb b/spec/request/processes_spec.rb index 5e647610b9c..b23859c61fb 100644 --- a/spec/request/processes_spec.rb +++ b/spec/request/processes_spec.rb @@ -2,6 +2,26 @@ require 'request_spec_shared_examples' RSpec.describe 'Processes' do + RSpec.shared_examples 'process resources with no process_instances' do + it 'shows an empty array' do + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + + if parsed_response['resources'].nil? + # Single resource + expect(parsed_response['process_instances']).to eq([]) + expect(parsed_response.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + else + # Paginated list + parsed_response['resources'].each do |process| + expect(process['process_instances']).to eq([]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + end + end + end + let(:org) { VCAP::CloudController::Organization.make } let(:space) { VCAP::CloudController::Space.make(organization: org) } let(:app_model) { VCAP::CloudController::AppModel.make(space: space, name: 'my_app', droplet: droplet) } @@ -21,6 +41,9 @@ annotations: { 'checksum' => 'SHA' } } end + let(:instances_reporters) { instance_double(VCAP::CloudController::InstancesReporters) } + let(:instances_for_processes) { {} } + let(:keys_in_order) { %w[readiness_health_check process_instances relationships] } before do allow_any_instance_of(Diego::Client).to receive(:build_client).and_return(build_client) @@ -55,6 +78,11 @@ ) end + before do + CloudController::DependencyLocator.instance.register(:instances_reporters, instances_reporters) + allow(instances_reporters).to receive(:instances_for_processes).and_return(instances_for_processes) + end + it_behaves_like 'list query endpoint' do let(:message) { VCAP::CloudController::ProcessesListMessage } let(:request) { '/v3/processes' } @@ -72,6 +100,7 @@ organization_guids: %w[foo bar], types: %w[foo bar], app_guids: %w[foo bar], + embed: 'process_instances', page: '2', per_page: '10', order_by: 'updated_at', @@ -384,6 +413,46 @@ end end + context 'with embed=process_instances' do + let(:instances_for_processes) do + { + web_process.guid => { + 0 => { state: 'RUNNING', since: 111 }, + 1 => { state: 'STARTING', since: 222 } + }, + worker_process.guid => { + 0 => { state: 'RUNNING', since: 333 }, + 1 => { state: 'DOWN', since: 444 } + } + } + end + + it 'shows the embedded process_instances arrays' do + get '/v3/processes?embed=process_instances', nil, developer_headers + + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + + parsed_response['resources'].find { |resource| resource['guid'] == web_process.guid }.tap do |process| + expect(process['process_instances']).to eq([{ 'index' => 0, 'state' => 'RUNNING', 'since' => 111 }, { 'index' => 1, 'state' => 'STARTING', 'since' => 222 }]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + parsed_response['resources'].find { |resource| resource['guid'] == worker_process.guid }.tap do |process| + expect(process['process_instances']).to eq([{ 'index' => 0, 'state' => 'RUNNING', 'since' => 333 }, { 'index' => 1, 'state' => 'DOWN', 'since' => 444 }]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + end + + context 'when there are no instances' do + let(:instances_for_processes) { { web_process.guid => {}, worker_process.guid => {} } } + + before { get '/v3/processes?embed=process_instances', nil, developer_headers } + + it_behaves_like 'process resources with no process_instances' + end + end + context 'permissions' do let(:api_call) { ->(user_headers) { get '/v3/processes', nil, user_headers } } @@ -479,6 +548,49 @@ expect(parsed_response['command']).to eq('[PRIVATE DATA HIDDEN]') end + context 'with embed=process_instances' do + let(:instances_for_processes) do + { + process.guid => { + 0 => { state: 'RUNNING', since: 111 }, + 1 => { state: 'STARTING', since: 222 } + } + } + end + let(:expected_response) do + a = super().to_a + before_relationships = a.index { |k, _| k == 'relationships' } || a.length + a.insert(before_relationships, ['process_instances', [ + { 'index' => 0, 'state' => 'RUNNING', 'since' => 111 }, + { 'index' => 1, 'state' => 'STARTING', 'since' => 222 } + ]]) + a.to_h + end + + before do + CloudController::DependencyLocator.instance.register(:instances_reporters, instances_reporters) + allow(instances_reporters).to receive(:instances_for_processes).and_return(instances_for_processes) + end + + it 'shows the embedded process_instances array' do + get "/v3/processes/#{process.guid}?embed=process_instances", nil, developer_headers + + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + expect(parsed_response).to be_a_response_like(expected_response) + expect(parsed_response.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + + context 'when there are no instances' do + let(:instances_for_processes) { { process.guid => {} } } + + before { get "/v3/processes/#{process.guid}?embed=process_instances", nil, developer_headers } + + it_behaves_like 'process resources with no process_instances' + end + end + context 'permissions' do let(:api_call) { ->(user_headers) { get "/v3/processes/#{process.guid}", nil, user_headers } } @@ -657,6 +769,77 @@ end end + describe 'GET /v3/processes/:guid/process_instances' do + let(:process) { VCAP::CloudController::ProcessModel.make(:process, app: app_model) } + let(:two_days_ago_since_epoch_ns) { 2.days.ago.to_f * 1e9 } + let(:two_days_in_seconds) { 60 * 60 * 24 * 2 } + let(:second_in_ns) { 1_000_000_000 } + let(:actual_lrp_0) do + Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 0), + actual_lrp_instance_key: Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-a'), + state: Diego::ActualLRPState::RUNNING, + placement_error: '', + since: two_days_ago_since_epoch_ns + ) + end + let(:actual_lrp_1) do + Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 1), + actual_lrp_instance_key: Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-b'), + state: Diego::ActualLRPState::CLAIMED, + placement_error: '', + since: two_days_ago_since_epoch_ns + (1 * second_in_ns) + ) + end + let(:bbs_response) { Diego::Bbs::Models::ActualLRPsByProcessGuidsResponse.new(actual_lrps: [actual_lrp_0, actual_lrp_1]) } + let(:bbs_client) { double(:bbs_client) } + + let(:expected_response) do + { + 'resources' => [{ + 'index' => 0, + 'state' => 'RUNNING', + 'since' => two_days_in_seconds + }, { + 'index' => 1, + 'state' => 'STARTING', + 'since' => two_days_in_seconds - 1 + }], + 'links' => { + 'self' => { 'href' => "#{link_prefix}/v3/processes/#{process.guid}/process_instances" }, + 'process' => { 'href' => "#{link_prefix}/v3/processes/#{process.guid}" } + } + } + end + + before do + CloudController::DependencyLocator.instance.register(:bbs_instances_client, VCAP::CloudController::Diego::BbsInstancesClient.new(bbs_client)) + allow(bbs_client).to receive(:actual_lrps_by_process_guids).and_return(bbs_response) + end + + it 'retrieves all process instances for the process' do + get "/v3/processes/#{process.guid}/process_instances", nil, developer_headers + + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + expect(parsed_response).to be_a_response_like(expected_response) + end + + context 'permissions' do + let(:api_call) { ->(user_headers) { get "/v3/processes/#{process.guid}/process_instances", nil, user_headers } } + + let(:expected_codes_and_responses) do + h = Hash.new({ code: 200, response_object: expected_response }.freeze) + h['org_auditor'] = h['org_billing_manager'] = h['no_role'] = { code: 404, response_object: [] } + h + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end + end + describe 'PATCH /v3/processes/:guid' do let(:revision) { VCAP::CloudController::RevisionModel.make } let(:process) do @@ -1368,6 +1551,47 @@ end end + context 'with embed=process_instances' do + let(:instances_for_processes) do + { + process1.guid => { 0 => { state: 'RUNNING', since: 111 } }, + process2.guid => { 0 => { state: 'STARTING', since: 222 } }, + process3.guid => { 0 => { state: 'DOWN', since: 333 } }, + deployment_process.guid => {} + } + end + + before do + CloudController::DependencyLocator.instance.register(:instances_reporters, instances_reporters) + allow(instances_reporters).to receive(:instances_for_processes).and_return(instances_for_processes) + end + + it 'shows the embedded process_instances arrays (empty if there are no instances)' do + get "/v3/apps/#{app_model.guid}/processes?embed=process_instances", nil, developer_headers + + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + + parsed_response['resources'].find { |resource| resource['guid'] == process1.guid }.tap do |process| + expect(process['process_instances']).to eq([{ 'index' => 0, 'state' => 'RUNNING', 'since' => 111 }]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + parsed_response['resources'].find { |resource| resource['guid'] == process2.guid }.tap do |process| + expect(process['process_instances']).to eq([{ 'index' => 0, 'state' => 'STARTING', 'since' => 222 }]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + parsed_response['resources'].find { |resource| resource['guid'] == process3.guid }.tap do |process| + expect(process['process_instances']).to eq([{ 'index' => 0, 'state' => 'DOWN', 'since' => 333 }]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + parsed_response['resources'].find { |resource| resource['guid'] == deployment_process.guid }.tap do |process| + expect(process['process_instances']).to eq([]) + expect(process.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + end + end + context 'permissions' do let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/processes", nil, user_headers } } let(:expected_guids) { [process1.guid, process2.guid, process3.guid, deployment_process.guid] } @@ -1442,6 +1666,11 @@ } end + before do + CloudController::DependencyLocator.instance.register(:instances_reporters, instances_reporters) + allow(instances_reporters).to receive(:instances_for_processes).and_return(instances_for_processes) + end + it 'retrieves the process for an app with the requested type' do get "/v3/apps/#{app_model.guid}/processes/web", nil, developer_headers @@ -1466,6 +1695,36 @@ expect(parsed_response['command']).to eq('[PRIVATE DATA HIDDEN]') end + context 'with embed=process_instances' do + let(:instances_for_processes) do + { + process.guid => { + 0 => { state: 'RUNNING', since: 111 }, + 1 => { state: 'STARTING', since: 222 } + } + } + end + + it 'shows the embedded process_instances arrays' do + get "/v3/apps/#{app_model.guid}/processes/web?embed=process_instances", nil, developer_headers + + parsed_response = Oj.load(last_response.body) + + expect(last_response.status).to eq(200) + + expect(parsed_response['process_instances']).to eq([{ 'index' => 0, 'state' => 'RUNNING', 'since' => 111 }, { 'index' => 1, 'state' => 'STARTING', 'since' => 222 }]) + expect(parsed_response.keys.each_cons(keys_in_order.size)).to include(keys_in_order) + end + + context 'when there are no instances' do + let(:instances_for_processes) { { process.guid => {} } } + + before { get "/v3/apps/#{app_model.guid}/processes/web?embed=process_instances", nil, developer_headers } + + it_behaves_like 'process resources with no process_instances' + end + end + context 'permissions' do let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/processes/web", nil, user_headers } } diff --git a/spec/unit/lib/cloud_controller/diego/bbs_instances_client_spec.rb b/spec/unit/lib/cloud_controller/diego/bbs_instances_client_spec.rb index 3dcbb78fbf5..9ca05948828 100644 --- a/spec/unit/lib/cloud_controller/diego/bbs_instances_client_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/bbs_instances_client_spec.rb @@ -21,41 +21,67 @@ module VCAP::CloudController::Diego expect(bbs_client).to have_received(:actual_lrps_by_process_guid).with(process_guid) end - context 'when the response contains a ResourceNotFound error' do + context 'when a Diego error is thrown' do + before do + allow(bbs_client).to receive(:actual_lrps_by_process_guid).with(process_guid).and_raise(::Diego::Error.new('boom')) + end + + it 're-raises with a CC Error' do + expect do + client.lrp_instances(process) + end.to raise_error(CloudController::Errors::InstancesUnavailable, 'boom') + end + end + + context 'when the response contains an unknown error' do let(:bbs_response) do - ::Diego::Bbs::Models::ActualLRPGroupsResponse.new(error: ::Diego::Bbs::Models::Error.new( - message: 'error-message', - type: ::Diego::Bbs::Models::Error::Type::ResourceNotFound - )) + ::Diego::Bbs::Models::ActualLRPsResponse.new(error: ::Diego::Bbs::Models::Error.new(message: 'error-message')) end it 'raises' do expect do client.lrp_instances(process) - end.to raise_error(CloudController::Errors::NoRunningInstances) + end.to raise_error(CloudController::Errors::InstancesUnavailable, 'error-message') end end + end + + describe '#actual_lrps_by_processes' do + let(:processes) { [VCAP::CloudController::ProcessModelFactory.make] } + let(:process_guids) { [ProcessGuid.from_process(processes[0])] } + let(:actual_lrp) { ::Diego::Bbs::Models::ActualLRP.new(state: 'potato') } + let(:actual_lrps) { [actual_lrp] } + let(:bbs_response) { ::Diego::Bbs::Models::ActualLRPsByProcessGuidsResponse.new(actual_lrps:) } + + before do + allow(bbs_client).to receive(:actual_lrps_by_process_guids).with(process_guids).and_return(bbs_response) + end + + it 'sends the lrp instances py process_guids request to diego' do + client.actual_lrps_by_processes(processes) + expect(bbs_client).to have_received(:actual_lrps_by_process_guids).with(process_guids) + end context 'when a Diego error is thrown' do before do - allow(bbs_client).to receive(:actual_lrps_by_process_guid).with(process_guid).and_raise(::Diego::Error.new('boom')) + allow(bbs_client).to receive(:actual_lrps_by_process_guids).with(process_guids).and_raise(::Diego::Error.new('boom')) end it 're-raises with a CC Error' do expect do - client.lrp_instances(process) + client.actual_lrps_by_processes(processes) end.to raise_error(CloudController::Errors::InstancesUnavailable, 'boom') end end context 'when the response contains an unknown error' do let(:bbs_response) do - ::Diego::Bbs::Models::ActualLRPsResponse.new(error: ::Diego::Bbs::Models::Error.new(message: 'error-message')) + ::Diego::Bbs::Models::ActualLRPsByProcessGuidsResponse.new(error: ::Diego::Bbs::Models::Error.new(message: 'error-message')) end it 'raises' do expect do - client.lrp_instances(process) + client.actual_lrps_by_processes(processes) end.to raise_error(CloudController::Errors::InstancesUnavailable, 'error-message') end end @@ -83,7 +109,7 @@ module VCAP::CloudController::Diego context 'when the response contains a ResourceNotFound error' do let(:bbs_response) do - ::Diego::Bbs::Models::ActualLRPGroupsResponse.new(error: ::Diego::Bbs::Models::Error.new( + ::Diego::Bbs::Models::DesiredLRPResponse.new(error: ::Diego::Bbs::Models::Error.new( message: 'error-message', type: ::Diego::Bbs::Models::Error::Type::ResourceNotFound )) diff --git a/spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb b/spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb index 2c702118907..969a082c01b 100644 --- a/spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb @@ -692,6 +692,159 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) end end end + + describe '#instances_for_processes' do + let(:second_in_ns) { 1_000_000_000 } + let(:actual_lrp_0) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 0), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-a'), + state: ::Diego::ActualLRPState::RUNNING, + placement_error: '', + since: two_days_ago_since_epoch_ns + ) + end + let(:actual_lrp_1) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 1), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-b'), + state: ::Diego::ActualLRPState::CLAIMED, + placement_error: '', + since: two_days_ago_since_epoch_ns + (1 * second_in_ns) + ) + end + let(:actual_lrp_2a) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 2), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-c'), + state: ::Diego::ActualLRPState::RUNNING, + placement_error: '', + since: two_days_ago_since_epoch_ns + (2 * second_in_ns) + ) + end + let(:actual_lrp_2b) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: process.guid + 'version', index: 2), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-d'), + state: ::Diego::ActualLRPState::UNCLAIMED, + placement_error: '', + since: two_days_ago_since_epoch_ns + (3 * second_in_ns) + ) + end + + before do + allow(bbs_instances_client).to receive_messages(actual_lrps_by_processes: bbs_actual_lrps_response) + end + + context 'with multiple actual lrps' do + let(:bbs_actual_lrps_response) { [actual_lrp_1, actual_lrp_0, actual_lrp_2a] } # unordered to test sorting by index + + it 'returns all instances sorted by index' do + instances = subject.instances_for_processes([process]) + expect(instances).to eq({ + process.guid => { + 0 => { state: 'RUNNING', since: two_days_in_seconds }, + 1 => { state: 'STARTING', since: two_days_in_seconds - 1 }, + 2 => { state: 'RUNNING', since: two_days_in_seconds - 2 } + } + }) + end + end + + context 'with multiple actual lrps for the same index' do + let(:bbs_actual_lrps_response) { [actual_lrp_0, actual_lrp_1, actual_lrp_2a, actual_lrp_2b] } + + it 'returns the newest instance per index' do + instances = subject.instances_for_processes([process]) + expect(instances).to eq({ + process.guid => { + 0 => { state: 'RUNNING', since: two_days_in_seconds }, + 1 => { state: 'STARTING', since: two_days_in_seconds - 1 }, + 2 => { state: 'STARTING', since: two_days_in_seconds - 3 } + } + }) + end + end + + context 'with number of desired instances being greater than number of actual lrps' do + let(:bbs_actual_lrps_response) { [actual_lrp_0, actual_lrp_1] } + let(:desired_instances) { 4 } + + it 'fills in missing instances as DOWN' do + instances = subject.instances_for_processes([process]) + expect(instances).to eq({ + process.guid => { + 0 => { state: 'RUNNING', since: two_days_in_seconds }, + 1 => { state: 'STARTING', since: two_days_in_seconds - 1 }, + 2 => { state: 'DOWN' }, + 3 => { state: 'DOWN' } + } + }) + end + end + + context 'with multiple processes' do + let(:second_process) { ProcessModel.make } + let(:second_process_actual_lrp_0) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: second_process.guid + 'version', index: 0), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-e'), + state: ::Diego::ActualLRPState::RUNNING, + placement_error: '', + since: two_days_ago_since_epoch_ns + (4 * second_in_ns) + ) + end + let(:second_process_actual_lrp_1) do + ::Diego::Bbs::Models::ActualLRP.new( + actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(process_guid: second_process.guid + 'version', index: 1), + actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid: 'instance-f'), + state: ::Diego::ActualLRPState::CRASHED, + placement_error: '', + since: two_days_ago_since_epoch_ns + (5 * second_in_ns) + ) + end + let(:bbs_actual_lrps_response) { [actual_lrp_0, second_process_actual_lrp_0, actual_lrp_1, second_process_actual_lrp_1] } # unordered to test grouping + + it 'returns instances grouped by process guid' do + instances = subject.instances_for_processes([process, second_process]) + expect(instances).to eq({ + process.guid => { + 0 => { state: 'RUNNING', since: two_days_in_seconds }, + 1 => { state: 'STARTING', since: two_days_in_seconds - 1 } + }, + second_process.guid => { + 0 => { state: 'RUNNING', since: two_days_in_seconds - 4 }, + 1 => { state: 'CRASHED', since: two_days_in_seconds - 5 } + } + }) + end + end + + context 'with no actual lrps but desired instances' do + let(:bbs_actual_lrps_response) { [] } + let(:desired_instances) { 2 } + + it 'fills in missing instances as DOWN' do + instances = subject.instances_for_processes([process]) + expect(instances).to eq({ + process.guid => { + 0 => { state: 'DOWN' }, + 1 => { state: 'DOWN' } + } + }) + end + end + + context 'with no actual lrps and no desired instances' do + let(:bbs_actual_lrps_response) { [] } + let(:desired_instances) { 0 } + + it 'returns an empty map for the instances' do + instances = subject.instances_for_processes([process]) + expect(instances).to eq({ process.guid => {} }) + end + end + end end end end diff --git a/spec/unit/messages/process_show_message_spec.rb b/spec/unit/messages/process_show_message_spec.rb new file mode 100644 index 00000000000..be040dbfb7c --- /dev/null +++ b/spec/unit/messages/process_show_message_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe ProcessShowMessage do + it 'does not accept fields not in the set' do + message = ProcessShowMessage.from_params({ 'foo' => 'bar' }) + expect(message).not_to be_valid + expect(message.errors[:base][0]).to include("Unknown query parameter(s): 'foo'") + end + + it 'does not accept embed other than process_instances' do + message = ProcessShowMessage.from_params({ 'embed' => 'process_instances' }) + expect(message).to be_valid + message = ProcessShowMessage.from_params({ 'embed' => 'stats' }) + expect(message).not_to be_valid + end + end +end diff --git a/spec/unit/messages/processes_list_message_spec.rb b/spec/unit/messages/processes_list_message_spec.rb index 1425412a67f..4346e772dc6 100644 --- a/spec/unit/messages/processes_list_message_spec.rb +++ b/spec/unit/messages/processes_list_message_spec.rb @@ -14,6 +14,7 @@ module VCAP::CloudController 'organization_guids' => 'the_organization_guid, another-org-guid', 'app_guids' => 'the-app-guid, the-app-guid2', 'guids' => 'process-guid,process-guid2', + 'embed' => 'process_instances', 'order_by' => 'created_at', 'label_selector' => 'key=value', 'created_ats' => "#{Time.now.utc.iso8601},#{Time.now.utc.iso8601}", @@ -33,6 +34,7 @@ module VCAP::CloudController expect(message.organization_guids).to eq(%w[the_organization_guid another-org-guid]) expect(message.app_guids).to eq(%w[the-app-guid the-app-guid2]) expect(message.guids).to eq(%w[process-guid process-guid2]) + expect(message.embed).to eq(%w[process_instances]) expect(message.label_selector).to eq('key=value') end @@ -47,6 +49,7 @@ module VCAP::CloudController expect(message).to be_requested(:organization_guids) expect(message).to be_requested(:app_guids) expect(message).to be_requested(:guids) + expect(message).to be_requested(:embed) expect(message).to be_requested(:order_by) expect(message).to be_requested(:updated_ats) expect(message).to be_requested(:created_ats) @@ -62,6 +65,7 @@ module VCAP::CloudController organization_guids: %w[organizationguid1 organizationguid2], guids: ['processguid1'], app_guid: 'appguid', + embed: 'process_instances', page: 1, label_selector: 'key=value', per_page: 5, @@ -78,6 +82,7 @@ module VCAP::CloudController space_guids organization_guids guids + embed label_selector created_ats updated_ats @@ -98,6 +103,7 @@ module VCAP::CloudController organization_guids: %w[organizationguid1 organizationguid2], guids: ['processguid'], app_guid: 'appguid', + embed: 'process_instances', page: 1, per_page: 5, order_by: 'created_at' diff --git a/spec/unit/presenters/v3/process_instances_presenter_spec.rb b/spec/unit/presenters/v3/process_instances_presenter_spec.rb new file mode 100644 index 00000000000..06abfbd73c9 --- /dev/null +++ b/spec/unit/presenters/v3/process_instances_presenter_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' +require 'presenters/v3/process_instances_presenter' + +module VCAP::CloudController::Presenters::V3 + RSpec.describe ProcessInstancesPresenter do + let(:process) { VCAP::CloudController::ProcessModel.make } + let(:instances) do + { + 0 => { state: 'RUNNING', since: 111 }, + 1 => { state: 'STARTING', since: 222 }, + 2 => { state: 'CRASHED', since: 333 } + } + end + + subject(:presenter) { ProcessInstancesPresenter.new(instances, process) } + + describe '#to_hash' do + it 'returns a hash with resources and links' do + result = presenter.to_hash + expect(result).to have_key(:resources) + expect(result).to have_key(:links) + end + + it 'builds instances with correct structure' do + resources = presenter.to_hash[:resources] + expect(resources).to be_an(Array) + expect(resources.length).to eq(3) + + expect(resources[0]).to eq({ index: 0, state: 'RUNNING', since: 111 }) + expect(resources[1]).to eq({ index: 1, state: 'STARTING', since: 222 }) + expect(resources[2]).to eq({ index: 2, state: 'CRASHED', since: 333 }) + end + + it 'builds correct links' do + links = presenter.to_hash[:links] + expect(links[:self][:href]).to eq("#{link_prefix}/v3/processes/#{process.guid}/process_instances") + expect(links[:process][:href]).to eq("#{link_prefix}/v3/processes/#{process.guid}") + end + + context 'with empty instances' do + let(:instances) { {} } + + it 'returns an empty resources array' do + resources = presenter.to_hash[:resources] + expect(resources).to eq([]) + end + end + end + end +end