Skip to content

Commit 110b23d

Browse files
authored
Enable CF API to present routable field for app processes (#3500)
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 <gfranks@vmware.com> Signed-off-by: Brandon Roberson <broberson@vmware.com>
1 parent 5c6c640 commit 110b23d

6 files changed

Lines changed: 115 additions & 2 deletions

File tree

app/presenters/v3/process_stats_presenter.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ def found_instance_stats_hash(index, stats)
3535
type: @type,
3636
index: index,
3737
state: stats[:state],
38+
routable: stats[:routable],
3839
host: stats[:stats][:host],
3940
instance_internal_ip: stats[:stats][:net_info][:instance_address],
4041
uptime: stats[:stats][:uptime],
@@ -55,6 +56,7 @@ def down_instance_stats_hash(index, stats)
5556
type: @type,
5657
index: index,
5758
state: stats[:state],
59+
routable: stats[:routable],
5860
uptime: stats.dig(:stats, :uptime),
5961
isolation_segment: stats[:isolation_segment],
6062
details: stats[:details]

docs/v3/source/includes/resources/processes/_stats_object.md.erb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Name | Type | Description
1414
**type** | _string_ | Process type; a unique identifier for processes belonging to an app
1515
**index** | _integer_ | The zero-based index of running instances
1616
**state** | _string_ | The state of the instance; valid values are `RUNNING`, `CRASHED`, `STARTING`, `DOWN`
17+
**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`.
1718
**usage** | _object_ | Object containing actual usage data for the instance; the value is `{}` when usage data is unavailable
1819
**usage.time** | _[timestamp](#timestamps)_ | The time when the usage was requested
1920
**usage.cpu** | _number_ | The current cpu usage of the instance

lib/cloud_controller/diego/reporters/instances_stats_reporter.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ def stats_for_app(process)
4040
fds_quota: process.file_descriptors
4141
}.merge(metrics_data_for_instance(stats, quota_stats, log_cache_errors, formatted_current_time, index))
4242
}
43-
info[:details] = actual_lrp.placement_error if actual_lrp.placement_error.present?
43+
info[:details] = actual_lrp.placement_error if actual_lrp.placement_error.present?
44+
45+
info[:routable] = (actual_lrp.routable if actual_lrp.optional_routable)
4446
result[actual_lrp.actual_lrp_key.index] = info
4547
end
4648

spec/request/processes_spec.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@
521521
{
522522
0 => {
523523
state: 'RUNNING',
524+
routable: true,
524525
details: 'some-details',
525526
isolation_segment: 'very-isolated',
526527
stats: {
@@ -554,6 +555,7 @@
554555
'type' => 'worker',
555556
'index' => 0,
556557
'state' => 'RUNNING',
558+
'routable' => true,
557559
'isolation_segment' => 'very-isolated',
558560
'details' => 'some-details',
559561
'usage' => {

spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,18 @@ module Diego
1313

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

1718
def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
18-
::Diego::Bbs::Models::ActualLRP.new(
19+
lrp = ::Diego::Bbs::Models::ActualLRP.new(
1920
actual_lrp_key: ::Diego::Bbs::Models::ActualLRPKey.new(index:),
2021
actual_lrp_instance_key: ::Diego::Bbs::Models::ActualLRPInstanceKey.new(instance_guid:),
2122
state: state,
2223
placement_error: error,
2324
since: since
2425
)
26+
lrp.routable = is_routable unless is_routable.nil?
27+
lrp
2528
end
2629

2730
before { Timecop.freeze(Time.at(1.day.ago.to_i)) }
@@ -78,6 +81,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
7881
{
7982
0 => {
8083
state: 'RUNNING',
84+
routable: is_routable,
8185
isolation_segment: 'isolation-segment-name',
8286
stats: {
8387
name: process.name,
@@ -115,6 +119,25 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
115119
expect(instances_reporter.stats_for_app(process)).to eq([expected_stats_response, []])
116120
end
117121

122+
context 'when process is not routable' do
123+
let(:is_routable) { false }
124+
125+
it 'sets the routable property to false' do
126+
response, = instances_reporter.stats_for_app(process)
127+
expect(response[0][:routable]).to be(false)
128+
end
129+
end
130+
131+
context 'when diego does not send the routable property' do
132+
let(:is_routable) { nil }
133+
134+
it 'does not include the routable property in stats' do
135+
response, = instances_reporter.stats_for_app(process)
136+
expect(response[0]).to have_key(:routable)
137+
expect(response[0][:routable]).to be_nil
138+
end
139+
end
140+
118141
it 'passes a process_id filter' do
119142
filter = nil
120143

@@ -257,6 +280,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
257280
{
258281
0 => {
259282
state: 'RUNNING',
283+
routable: true,
260284
isolation_segment: 'isolation-segment-name',
261285
stats: {
262286
name: process.name,
@@ -388,6 +412,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
388412
{
389413
0 => {
390414
state: 'RUNNING',
415+
routable: true,
391416
isolation_segment: 'isolation-segment-name',
392417
stats: {
393418
name: process.name,

spec/unit/presenters/v3/process_stats_presenter_spec.rb

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ module VCAP::CloudController::Presenters::V3
5959
{
6060
0 => {
6161
state: 'RUNNING',
62+
routable: true,
6263
isolation_segment: 'hecka-compliant',
6364
stats: {
6465
name: process.name,
@@ -82,6 +83,7 @@ module VCAP::CloudController::Presenters::V3
8283
1 => {
8384
state: 'CRASHED',
8485
details: 'some-details',
86+
routable: false,
8587
stats: {
8688
name: process.name,
8789
uris: process.uris,
@@ -103,6 +105,7 @@ module VCAP::CloudController::Presenters::V3
103105
},
104106
2 => {
105107
state: 'DOWN',
108+
routable: false,
106109
details: 'you must construct additional pylons',
107110
stats: {
108111
uptime: 0
@@ -117,6 +120,7 @@ module VCAP::CloudController::Presenters::V3
117120
expect(result[0][:type]).to eq(process.type)
118121
expect(result[0][:index]).to eq(0)
119122
expect(result[0][:state]).to eq('RUNNING')
123+
expect(result[0][:routable]).to be(true)
120124
expect(result[0][:details]).to be_nil
121125
expect(result[0][:isolation_segment]).to eq('hecka-compliant')
122126
expect(result[0][:host]).to eq('myhost')
@@ -136,6 +140,7 @@ module VCAP::CloudController::Presenters::V3
136140
expect(result[1][:type]).to eq(process.type)
137141
expect(result[1][:index]).to eq(1)
138142
expect(result[1][:state]).to eq('CRASHED')
143+
expect(result[1][:routable]).to be(false)
139144
expect(result[1][:details]).to eq('some-details')
140145
expect(result[1][:isolation_segment]).to be_nil
141146
expect(result[1][:host]).to eq('toast')
@@ -151,12 +156,88 @@ module VCAP::CloudController::Presenters::V3
151156
type: process.type,
152157
index: 2,
153158
state: 'DOWN',
159+
routable: false,
154160
uptime: 0,
155161
isolation_segment: nil,
156162
details: 'you must construct additional pylons'
157163
)
158164
end
159165

166+
context 'diego does not provide the "routable" field' do
167+
let(:stats_for_process) do
168+
{
169+
0 => {
170+
state: 'RUNNING',
171+
routable: nil,
172+
isolation_segment: 'hecka-compliant',
173+
stats: {
174+
name: process.name,
175+
uris: process.uris,
176+
host: 'myhost',
177+
net_info: net_info_1,
178+
uptime: 12_345,
179+
mem_quota: process[:memory] * 1024 * 1024,
180+
disk_quota: process[:disk_quota] * 1024 * 1024,
181+
log_rate_limit: process[:log_rate_limit],
182+
fds_quota: process.file_descriptors,
183+
usage: {
184+
time: '2015-12-08 16:54:48 -0800',
185+
cpu: 80,
186+
mem: 128,
187+
disk: 1024,
188+
log_rate: 2048
189+
}
190+
}
191+
}
192+
}
193+
end
194+
195+
it 'does not set routable, with state of RUNNING' do
196+
result = presenter.present_stats_hash
197+
198+
expect(result[0][:state]).to eq('RUNNING')
199+
expect(result[0]).to have_key(:routable)
200+
expect(result[0][:routable]).to be_nil
201+
end
202+
end
203+
204+
context 'the process is running, but not routable due to failed readiness check in diego' do
205+
let(:stats_for_process) do
206+
{
207+
0 => {
208+
state: 'RUNNING',
209+
routable: false,
210+
isolation_segment: 'hecka-compliant',
211+
stats: {
212+
name: process.name,
213+
uris: process.uris,
214+
host: 'myhost',
215+
net_info: net_info_1,
216+
uptime: 12_345,
217+
mem_quota: process[:memory] * 1024 * 1024,
218+
disk_quota: process[:disk_quota] * 1024 * 1024,
219+
log_rate_limit: process[:log_rate_limit],
220+
fds_quota: process.file_descriptors,
221+
usage: {
222+
time: '2015-12-08 16:54:48 -0800',
223+
cpu: 80,
224+
mem: 128,
225+
disk: 1024,
226+
log_rate: 2048
227+
}
228+
}
229+
}
230+
}
231+
end
232+
233+
it 'sets routable to false, with state of RUNNING' do
234+
result = presenter.present_stats_hash
235+
236+
expect(result[0][:state]).to eq('RUNNING')
237+
expect(result[0][:routable]).to be(false)
238+
end
239+
end
240+
160241
context 'the process is running on opi and not diego, so *_tls_proxy_ports are not included in the port struct' do
161242
let(:net_info_1) do
162243
{

0 commit comments

Comments
 (0)