Skip to content

Commit

Permalink
Revert "Submit services to fleet in sorted order"
Browse files Browse the repository at this point in the history
This reverts commit bffce96.
  • Loading branch information
alexwelch committed Feb 25, 2015
1 parent 1a5f14f commit 8d5d8a6
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 191 deletions.
53 changes: 6 additions & 47 deletions app/models/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,16 @@ class App < ActiveRecord::Base

before_save :resolve_name_conflicts

def sorted_services
@sorted_services ||= ServiceSorter.sort(services)
end

def run
sorted_services.each(&:submit)
sorted_services.each(&:load)
sorted_services.each(&:start)
services.each(&:submit)
services.each(&:start)
end

def restart
sorted_services.each(&:shutdown)
sorted_services.each(&:submit)
sorted_services.each(&:load)
sorted_services.each(&:start)
services.each(&:shutdown)
sleep(1)
services.each(&:submit)
services.each(&:start)
end

def add_service(params)
Expand Down Expand Up @@ -59,40 +54,4 @@ def resolve_name_conflicts
count = App.where('name LIKE ?', "#{sanitized_name}%").count
self.name = (count > 0) ? "#{sanitized_name}_#{count}" : sanitized_name
end

class ServiceSorter

def self.sort(services)
new(services).sort
end

def initialize(services)
@services = services
@unprocessed = Array.new(services)
@marked = Set.new
@sorted = []
end

def sort
@services.each { |service| visit(service) }
@sorted
end

private

def visit(service)
if @marked.include?(service)
raise 'circular link reference'
end

if @unprocessed.include?(service)
@marked.add(service)
service.links.each { |link| visit(link.linked_to_service) }
@marked.delete(service)

@unprocessed.delete(service)
@sorted << service
end
end
end
end
5 changes: 0 additions & 5 deletions app/models/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ def service_state
end

def submit
manager.submit
end

def load
manager.load
end

Expand All @@ -77,7 +73,6 @@ def shutdown
def restart
self.shutdown
self.submit
self.load
self.start
end

Expand Down
37 changes: 1 addition & 36 deletions app/services/service_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

class ServiceManager

POLL_LENGTH = 5
POLL_INTERVAL = 0.5

def self.load(service)
new(service).load
end
Expand All @@ -17,22 +14,8 @@ def initialize(service)
@service = service
end

def submit
fleet_client.submit(@service.unit_name, service_def_from_service(@service))
end

def load
fleet_client.load(@service.unit_name)

# Poll unit state until it is successfully loaded
poll do
begin
state = fleet_client.get_unit_state(@service.unit_name)
state['systemdLoadState'] == 'loaded'
rescue Fleet::NotFound
false
end
end
fleet_client.load(@service.unit_name, service_def_from_service(@service))
end

def start
Expand All @@ -45,16 +28,6 @@ def stop

def destroy
fleet_client.destroy(@service.unit_name)

# Poll unit state until it can't be found
poll do
begin
fleet_client.get_unit_state(@service.unit_name)
false
rescue Fleet::NotFound
true
end
end
end

def get_state
Expand Down Expand Up @@ -101,12 +74,4 @@ def service_def_from_service(service)
'Service' => service_block
}
end

def poll(length=POLL_LENGTH, &block)
result = (length / POLL_INTERVAL).to_i.times do
break :success if block.call
sleep(POLL_INTERVAL)
end
result == :success
end
end
43 changes: 6 additions & 37 deletions spec/models/app_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,6 @@
it { should have_many(:services) }
it { should have_many(:categories).class_name('AppCategory').dependent(:destroy) }

describe '#sorted_services' do
let(:service_a) { Service.new(name: 'a') }
let(:service_b) { Service.new(name: 'b') }
let(:service_c) { Service.new(name: 'c') }

before do
service_a.links << ServiceLink.new(linked_to_service: service_b)
service_b.links << ServiceLink.new(linked_to_service: service_c)
allow(subject).to receive(:services).and_return(
[service_a, service_c, service_b])
end

it 'should return the same number of services as the original set' do
result = subject.sorted_services
expect(result.count).to eq subject.services.count
end

it 'sorts services so that dependent services come first' do
result = subject.sorted_services
expect(result).to match_array([service_c, service_b, service_a])
end
end

describe '#run' do

let(:s1) { Service.new(name: 's1') }
Expand All @@ -42,7 +19,6 @@
before do
subject.services = [s1, s2].each do |s|
allow(s).to receive(:submit).and_return(true)
allow(s).to receive(:load).and_return(true)
allow(s).to receive(:start).and_return(true)
end
end
Expand All @@ -53,12 +29,6 @@
subject.run
end

it 'submits each service' do
expect(s1).to receive(:load)
expect(s2).to receive(:load)
subject.run
end

it 'runs each service' do
expect(s1).to receive(:start)
expect(s2).to receive(:start)
Expand All @@ -72,11 +42,11 @@
let(:s2) { Service.new(name: 's2') }

before do
allow(subject).to receive(:sleep)
subject.services = [s1, s2]
subject.services.each do |s|
allow(s).to receive(:shutdown)
allow(s).to receive(:submit)
allow(s).to receive(:load)
allow(s).to receive(:start)
end
end
Expand All @@ -87,15 +57,14 @@
subject.restart
end

it 'submits each service' do
expect(s1).to receive(:submit)
expect(s2).to receive(:submit)
it 'does some sleeping' do
expect(subject).to receive(:sleep).with(1)
subject.restart
end

it 'loads each service' do
expect(s1).to receive(:load)
expect(s2).to receive(:load)
it 'submits each service' do
expect(s1).to receive(:submit)
expect(s2).to receive(:submit)
subject.restart
end

Expand Down
18 changes: 2 additions & 16 deletions spec/models/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
describe Service do

let(:dummy_manager) do
double(
:dummy_manager,
submit: true,
double(:dummy_manager,
load: true,
start: true,
destroy: true,
Expand Down Expand Up @@ -126,16 +124,9 @@
end

describe '#submit' do
it 'invokes submit on the service manager' do
expect(dummy_manager).to receive(:submit)
subject.submit
end
end

describe '#load' do
it 'invokes load on the service manager' do
expect(dummy_manager).to receive(:load)
subject.load
subject.submit
end
end

Expand All @@ -159,11 +150,6 @@
subject.restart
end

it 'invokes submit on the service manager' do
expect(dummy_manager).to receive(:submit)
subject.restart
end

it 'invokes load on the service manager' do
expect(dummy_manager).to receive(:load)
subject.restart
Expand Down
54 changes: 4 additions & 50 deletions spec/services/service_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
end

let(:fake_fleet_client) do
double(
:fake_fleet_client,
submit: true,
double(:fake_fleet_client,
load: true,
start: true,
stop: true,
Expand Down Expand Up @@ -76,7 +74,7 @@
end
end

describe '#submit' do
describe '#load' do

let(:linked_to_service) { Service.new(name: 'linked_to_service') }
let(:docker_run_string) { 'docker run some stuff' }
Expand All @@ -90,7 +88,7 @@
end

it 'submits a service definition to the fleet service' do
expect(fake_fleet_client).to receive(:submit) do |name, service_def|
expect(fake_fleet_client).to receive(:load) do |name, service_def|
expect(name).to eq service.unit_name
expect(service_def).to eq(
{
Expand All @@ -113,28 +111,6 @@
)
end

subject.submit
end

it 'returns the result of the fleet call' do
expect(subject.submit).to eql true
end
end

describe '#load' do
before do
allow(fake_fleet_client).to receive(:get_unit_state)
.and_return('systemdLoadState' => 'loaded')
end

it 'sends a destroy message to the fleet client' do
expect(fake_fleet_client).to receive(:load).with(service.unit_name)
subject.load
end

it 'polls the unit state' do
expect(fake_fleet_client).to receive(:get_unit_state)
.and_return('systemdLoadState' => 'loaded')
subject.load
end

Expand All @@ -143,7 +119,7 @@
end
end

[:start, :stop].each do |method|
[:start, :stop, :destroy].each do |method|

describe "##{method}" do

Expand All @@ -158,28 +134,6 @@
end
end

describe '#destroy' do
before do
allow(fake_fleet_client).to receive(:get_unit_state)
.and_raise(Fleet::NotFound, 'oops')
end

it 'sends a destroy message to the fleet client' do
expect(fake_fleet_client).to receive(:destroy).with(service.unit_name)
subject.destroy
end

it 'polls the unit state' do
expect(fake_fleet_client).to receive(:get_unit_state)
.and_raise(Fleet::NotFound, 'oops')
subject.destroy
end

it 'returns the result of the fleet call' do
expect(subject.destroy).to eql true
end
end

describe '#get_state' do

let(:fleet_state) do
Expand Down

0 comments on commit 8d5d8a6

Please sign in to comment.