Skip to content

Commit

Permalink
Enable CF API to present routable field for app processes (#3500)
Browse files Browse the repository at this point in the history 
Add `routable` field to the process_stats object

Shows whether or not the app's readiness check is passing on an app instance,
and whether or not if routes will be sent to it.

Allow the `routable` process stat field to be null if diego doesn't support
readiness checks.

Signed-off-by: Geoff Franks <[email protected]>
Signed-off-by: Brandon Roberson <[email protected]>
  • Loading branch information
@geofffranks
geofffranks authored Nov 2, 2023
1 parent 5c6c640 commit 110b23d
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 2 deletions.
2 changes: 2 additions & 0 deletions app/presenters/v3/process_stats_presenter.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def found_instance_stats_hash(index, stats)
type: @type,
index: index,
state: stats[:state],
routable: stats[:routable],
host: stats[:stats][:host],
instance_internal_ip: stats[:stats][:net_info][:instance_address],
uptime: stats[:stats][:uptime],
Expand All @@ -55,6 +56,7 @@ def down_instance_stats_hash(index, stats)
type: @type,
index: index,
state: stats[:state],
routable: stats[:routable],
uptime: stats.dig(:stats, :uptime),
isolation_segment: stats[:isolation_segment],
details: stats[:details]
Expand Down
1 change: 1 addition & 0 deletions docs/v3/source/includes/resources/processes/_stats_object.md.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Name | Type | Description
**type** | _string_ | Process type; a unique identifier for processes belonging to an app
**index** | _integer_ | The zero-based index of running instances
**state** | _string_ | The state of the instance; valid values are `RUNNING`, `CRASHED`, `STARTING`, `DOWN`
**routable** | _boolean_ | Whether or not the instance is routable (determined by the readiness check of the app). If app readiness checks and routability are unsupported by Diego, this will return as `null`.
**usage** | _object_ | Object containing actual usage data for the instance; the value is `{}` when usage data is unavailable
**usage.time** | _[timestamp](#timestamps)_ | The time when the usage was requested
**usage.cpu** | _number_ | The current cpu usage of the instance
Expand Down
4 changes: 3 additions & 1 deletion lib/cloud_controller/diego/reporters/instances_stats_reporter.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ def stats_for_app(process)
fds_quota: process.file_descriptors
}.merge(metrics_data_for_instance(stats, quota_stats, log_cache_errors, formatted_current_time, index))
}
info[:details] = actual_lrp.placement_error if actual_lrp.placement_error.present?
info[:details] = actual_lrp.placement_error if actual_lrp.placement_error.present?

info[:routable] = (actual_lrp.routable if actual_lrp.optional_routable)
result[actual_lrp.actual_lrp_key.index] = info
end

Expand Down
2 changes: 2 additions & 0 deletions spec/request/processes_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@
{
0 => {
state: 'RUNNING',
routable: true,
details: 'some-details',
isolation_segment: 'very-isolated',
stats: {
Expand Down Expand Up @@ -554,6 +555,7 @@
'type' => 'worker',
'index' => 0,
'state' => 'RUNNING',
'routable' => true,
'isolation_segment' => 'very-isolated',
'details' => 'some-details',
'usage' => {
Expand Down
27 changes: 26 additions & 1 deletion spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ module Diego

let(:two_days_ago_since_epoch_ns) { 2.days.ago.to_f * 1e9 }
let(:two_days_in_seconds) { 60 * 60 * 24 * 2 }
let(:is_routable) { true }

def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
::Diego::Bbs::Models::ActualLRP.new(
lrp = ::Diego::Bbs::Models::ActualLRP.new(
actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(index:),
actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid:),
state: state,
placement_error: error,
since: since
)
lrp.routable = is_routable unless is_routable.nil?
lrp
end

before { Timecop.freeze(Time.at(1.day.ago.to_i)) }
Expand Down Expand Up @@ -78,6 +81,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
{
0 => {
state: 'RUNNING',
routable: is_routable,
isolation_segment: 'isolation-segment-name',
stats: {
name: process.name,
Expand Down Expand Up @@ -115,6 +119,25 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
expect(instances_reporter.stats_for_app(process)).to eq([expected_stats_response, []])
end

context 'when process is not routable' do
let(:is_routable) { false }

it 'sets the routable property to false' do
response, = instances_reporter.stats_for_app(process)
expect(response[0][:routable]).to be(false)
end
end

context 'when diego does not send the routable property' do
let(:is_routable) { nil }

it 'does not include the routable property in stats' do
response, = instances_reporter.stats_for_app(process)
expect(response[0]).to have_key(:routable)
expect(response[0][:routable]).to be_nil
end
end

it 'passes a process_id filter' do
filter = nil

Expand Down Expand Up @@ -257,6 +280,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
{
0 => {
state: 'RUNNING',
routable: true,
isolation_segment: 'isolation-segment-name',
stats: {
name: process.name,
Expand Down Expand Up @@ -388,6 +412,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
{
0 => {
state: 'RUNNING',
routable: true,
isolation_segment: 'isolation-segment-name',
stats: {
name: process.name,
Expand Down
81 changes: 81 additions & 0 deletions spec/unit/presenters/v3/process_stats_presenter_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ module VCAP::CloudController::Presenters::V3
{
0 => {
state: 'RUNNING',
routable: true,
isolation_segment: 'hecka-compliant',
stats: {
name: process.name,
Expand All @@ -82,6 +83,7 @@ module VCAP::CloudController::Presenters::V3
1 => {
state: 'CRASHED',
details: 'some-details',
routable: false,
stats: {
name: process.name,
uris: process.uris,
Expand All @@ -103,6 +105,7 @@ module VCAP::CloudController::Presenters::V3
},
2 => {
state: 'DOWN',
routable: false,
details: 'you must construct additional pylons',
stats: {
uptime: 0
Expand All @@ -117,6 +120,7 @@ module VCAP::CloudController::Presenters::V3
expect(result[0][:type]).to eq(process.type)
expect(result[0][:index]).to eq(0)
expect(result[0][:state]).to eq('RUNNING')
expect(result[0][:routable]).to be(true)
expect(result[0][:details]).to be_nil
expect(result[0][:isolation_segment]).to eq('hecka-compliant')
expect(result[0][:host]).to eq('myhost')
Expand All @@ -136,6 +140,7 @@ module VCAP::CloudController::Presenters::V3
expect(result[1][:type]).to eq(process.type)
expect(result[1][:index]).to eq(1)
expect(result[1][:state]).to eq('CRASHED')
expect(result[1][:routable]).to be(false)
expect(result[1][:details]).to eq('some-details')
expect(result[1][:isolation_segment]).to be_nil
expect(result[1][:host]).to eq('toast')
Expand All @@ -151,12 +156,88 @@ module VCAP::CloudController::Presenters::V3
type: process.type,
index: 2,
state: 'DOWN',
routable: false,
uptime: 0,
isolation_segment: nil,
details: 'you must construct additional pylons'
)
end

context 'diego does not provide the "routable" field' do
let(:stats_for_process) do
{
0 => {
state: 'RUNNING',
routable: nil,
isolation_segment: 'hecka-compliant',
stats: {
name: process.name,
uris: process.uris,
host: 'myhost',
net_info: net_info_1,
uptime: 12_345,
mem_quota: process[:memory] * 1024 * 1024,
disk_quota: process[:disk_quota] * 1024 * 1024,
log_rate_limit: process[:log_rate_limit],
fds_quota: process.file_descriptors,
usage: {
time: '2015-12-08 16:54:48 -0800',
cpu: 80,
mem: 128,
disk: 1024,
log_rate: 2048
}
}
}
}
end

it 'does not set routable, with state of RUNNING' do
result = presenter.present_stats_hash

expect(result[0][:state]).to eq('RUNNING')
expect(result[0]).to have_key(:routable)
expect(result[0][:routable]).to be_nil
end
end

context 'the process is running, but not routable due to failed readiness check in diego' do
let(:stats_for_process) do
{
0 => {
state: 'RUNNING',
routable: false,
isolation_segment: 'hecka-compliant',
stats: {
name: process.name,
uris: process.uris,
host: 'myhost',
net_info: net_info_1,
uptime: 12_345,
mem_quota: process[:memory] * 1024 * 1024,
disk_quota: process[:disk_quota] * 1024 * 1024,
log_rate_limit: process[:log_rate_limit],
fds_quota: process.file_descriptors,
usage: {
time: '2015-12-08 16:54:48 -0800',
cpu: 80,
mem: 128,
disk: 1024,
log_rate: 2048
}
}
}
}
end

it 'sets routable to false, with state of RUNNING' do
result = presenter.present_stats_hash

expect(result[0][:state]).to eq('RUNNING')
expect(result[0][:routable]).to be(false)
end
end

context 'the process is running on opi and not diego, so *_tls_proxy_ports are not included in the port struct' do
let(:net_info_1) do
{
Expand Down

0 comments on commit 110b23d

Please sign in to comment.