Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/lab_manager/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require 'lab_manager/app/endpoints/base'
require 'lab_manager/app/endpoints/uptime'
require 'lab_manager/app/endpoints/computes'
require 'lab_manager/app/endpoints/status'

module LabManager
# base rest service class
Expand All @@ -29,6 +30,10 @@ def initialize
map '/computes' do
run LabManager::App::Endpoints::Compute.new
end

map '/status' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot give me rest (pun intended).

I am not sure about singular in route name, what do you think?

run LabManager::App::Endpoints::Status.new
end
end
end

Expand Down
22 changes: 22 additions & 0 deletions lib/lab_manager/app/endpoints/status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require 'lab_manager/app/endpoints/base'
require 'json'

module LabManager
class App
module Endpoints
# class responsible for provider statuses
class Status < Base
get '/v_sphere' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just note, not merge blocker. When you look at the code in the .to_json block, you can clearly see it is parametrised.
We could have internal functions get_status or get_status_json, which would take provider_name as parameter.

Then, this route would be simplified to something like:
get '/v_sphere' do
get_status(provider_name).to_json

Benefit now? Not so big. Benefit for future? Definitely, once new providers pop in.

provider_name = 'v_sphere'
{
max_running_machines: ::Provider::VSphereConfig.scheduler.max_vm,
running_machines: ::Compute.where(provider_name: provider_name, state: 'running').count,
errored_machines: ::Compute.where(provider_name: provider_name, state: 'errored').count,
created_machines: ::Compute.where(provider_name: provider_name, state: 'created').count,
queued_machines: ::Compute.where(provider_name: provider_name, state: 'queued').count
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to pass come possibility to specify what states should be returned; these hardcoded: running, errored, created and queued might be default option. The other thing: v_sphere specific measure is only max_running_machines: the rest is the same for all possible providers :) what about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can think about this improvement if this route starts to run 'slowly' but for now i think it's ok to return all information.

}.to_json
end
end
end
end
end
67 changes: 67 additions & 0 deletions spec/integration/endpoints/status_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
require 'spec_helper'
require 'rack/test'
require 'lab_manager/app'

describe 'Status' do
include Rack::Test::Methods

def app
LabManager::App.new
end

describe 'GET /status/v_sphere' do
before(:each) do
allow(::Provider::VSphereConfig).to receive_message_chain(:scheduler, :max_vm) { 3 }

create(:compute, provider_name: 'v_sphere', name: 'one').update_column(:state, 'errored')
create(:compute, provider_name: 'v_sphere', name: 'two').update_column(:state, 'errored')

create(:compute, provider_name: 'v_sphere', name: 'three').update_column(:state, 'created')
create(:compute, provider_name: 'v_sphere', name: 'four').update_column(:state, 'created')
create(:compute, provider_name: 'v_sphere', name: 'five').update_column(:state, 'created')

create(:compute, provider_name: 'v_sphere', name: 'six').update_column(:state, 'running')

create(:compute, provider_name: 'v_sphere', name: 'seven').update_column(:state, 'queued')
create(:compute, provider_name: 'v_sphere', name: 'eight').update_column(:state, 'queued')
end

def perform_request
get '/status/v_sphere'
expect(last_response.status).to eq 200
MultiJson.load(last_response.body)
end

it 'reads max_vm machines from vsphere configuration' do
expect(::Provider::VSphereConfig).to receive_message_chain(:scheduler, :max_vm) { 3 }

response_json = perform_request

expect(response_json['max_running_machines']).to eq 3
end

it 'returns errored machines count' do
response_json = perform_request

expect(response_json['errored_machines']).to eq 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you add expects for specific machines as well (like "one" and "two" present)

end

it 'returns running machines count' do
response_json = perform_request

expect(response_json['running_machines']).to eq 1
end

it 'returns created machines count' do
response_json = perform_request

expect(response_json['created_machines']).to eq 3
end

it 'returns queued machines count' do
response_json = perform_request

expect(response_json['queued_machines']).to eq 2
end
end
end