Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

refactor set_expected_state to use hash. Enforce completeness of the

expected state through explicit checking.

Change-Id: I270fda3e42e0731709710feff5c92bcd1de7b386
  • Loading branch information...
commit 6395e85e8eeb7d007bf70aec7c032d8817e0e397 1 parent b50c800
Bob Nugmanov bnugmanov authored
1  .gitignore
@@ -13,3 +13,4 @@ ci-working-dir
13 13 *.swp
14 14 .rvmrc
15 15 *.pid
  16 +/vendor
21 lib/health_manager/app_state.rb
@@ -58,13 +58,20 @@ def initialize(id)
58 58 reset_missing_indices
59 59 end
60 60
61   - def set_expected_state(num_instances, state, live_version, framework, runtime, last_updated)
62   - @num_instances = num_instances
63   - @state = state
64   - @live_version = live_version
65   - @framework = framework
66   - @runtime = runtime
67   - @last_updated = last_updated
  61 + def set_expected_state(values)
  62 + values = values.dup #preserve the original
  63 + [:state,
  64 + :num_instances,
  65 + :live_version,
  66 + :framework,
  67 + :runtime,
  68 + :last_updated].each do |k|
  69 +
  70 + v = values.delete(k)
  71 + raise ArgumentError.new("Value #{k} is required") unless v
  72 + self.instance_variable_set("@#{k.to_s}",v)
  73 + end
  74 + raise ArgumentError.new("unsupported keys: #{values.keys}") unless values.empty?
68 75 end
69 76
70 77 def notify(event_type, *args)
12 lib/health_manager/bulk_based_expected_state_provider.rb
@@ -14,12 +14,12 @@ def set_expected_state(known, expected)
14 14 logger.debug { "bulk: #set_expected_state: known: #{known.inspect} expected: #{expected.inspect}" }
15 15
16 16 known.set_expected_state(
17   - expected['instances'],
18   - expected['state'],
19   - "#{expected['staged_package_hash']}-#{expected['run_count']}",
20   - expected['framework'],
21   - expected['runtime'],
22   - parse_utc(expected['updated_at']))
  17 + :num_instances => expected['instances'],
  18 + :state => expected['state'],
  19 + :live_version => "#{expected['staged_package_hash']}-#{expected['run_count']}",
  20 + :framework => expected['framework'],
  21 + :runtime => expected['runtime'],
  22 + :last_updated => parse_utc(expected['updated_at']))
23 23 end
24 24
25 25 private
2  lib/health_manager/harmonizer.rb
@@ -171,7 +171,7 @@ def analyze_all_apps
171 171 varz.update_realtime_stats_for_droplet(known_droplet)
172 172 true
173 173 else
174   - # TODO: remove
  174 + # TODO: remove once ready for production
175 175 varz.set(:droplets, known_state_provider.droplets)
176 176 varz.publish_realtime_stats
177 177
4 spec/unit/app_state_spec.rb
@@ -19,7 +19,7 @@
19 19 it 'should invoke missing_instances event handler' do
20 20 future_answer = [1, 3]
21 21 event_handler_invoked = false
22   - app, expected = make_app
  22 + app, _ = make_app
23 23
24 24 #no heartbeats arrived yet, so all instances are assumed missing
25 25 app.missing_indices.should == [0, 1, 2, 3]
@@ -55,7 +55,7 @@
55 55
56 56 it 'should invoke extra_instances event handler' do
57 57 app, expected = make_app
58   - extra_instance_id = expected[2]+"-0"
  58 + extra_instance_id = expected[:live_version]+"-0"
59 59
60 60 future_answer = [[extra_instance_id, "Extra instance"]]
61 61 event_handler_invoked = false
10 spec/unit/nats_based_known_state_provider_spec.rb
@@ -27,16 +27,8 @@
27 27 it 'should forward heartbeats' do
28 28
29 29 app, expected = make_app
30   -
31 30 app1 = @nb.get_droplet(app.id)
32   -
33   - app1.set_expected_state(
34   - app.num_instances,
35   - app.state,
36   - app.live_version,
37   - app.framework,
38   - app.runtime,
39   - app.last_updated)
  31 + app1.set_expected_state(expected)
40 32
41 33 instance = app1.get_instance(app.live_version, 0)
42 34 instance['state'].should == 'DOWN'
18 spec/unit/spec_helper.rb
@@ -15,16 +15,16 @@ def in_em(timeout = 2)
15 15
16 16 def make_app(id=1)
17 17 app = AppState.new(id)
18   - expected = [
19   - 4,
20   - 'STARTED',
21   - '12345abcded',
22   - 'sinatra',
23   - 'ruby19',
24   - Time.now.to_i - 60*60*24
25   - ]
  18 + expected = {
  19 + :num_instances => 4,
  20 + :state => 'STARTED',
  21 + :live_version => '12345abcded',
  22 + :framework => 'sinatra',
  23 + :runtime => 'ruby19',
  24 + :last_updated => Time.now.to_i - 60*60*24
  25 + }
26 26
27   - app.set_expected_state(*expected)
  27 + app.set_expected_state(expected)
28 28 return app, expected
29 29 end
30 30

Git Notes

review

Code-Review+2: Matt Page <mpage@rbcon.com>
Verified+1: CI Master <cf-ci@rbcon.com>
Submitted-by: Bob Nugmanov <bnugmanov@vmware.com>
Submitted-at: Wed, 09 May 2012 21:28:19 +0000
Reviewed-on: http://reviews.cloudfoundry.org/5472
Project: health_manager
Branch: refs/heads/master

0 comments on commit 6395e85

Please sign in to comment.
Something went wrong with that request. Please try again.