-
Notifications
You must be signed in to change notification settings - Fork 7
feat: add status endpoint pro vsphere provider #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Then, this route would be simplified to something like: 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment.
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?