Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Commit

Permalink
assert_proxies_disabled: Fix failures
Browse files Browse the repository at this point in the history
Change `assert_proxies_disabled` and `assert_proxies_not_disabled` to call
`assert_gear_status_in_proxy` once for each proxy gear and pass
`assert_gear_status_in_proxy` a list of target gears.

Factor `assert_proxies_status` out of `assert_proxies_disabled` and
`assert_proxies_not_disabled`.

Change `assert_gear_status_in_proxy` to accept a list of target gears, use
that list to construct a list of gear names to look for in the proxy's
HAProxy status page, and pass the test if, and only if, the length of the
list of matching gears with the expected status in the status page is equal
to the number of target gears.

The above changes solve the following issues:

• First, `assert_gear_status_in_proxy` previously was taking a proxy gear
  and a target gear and was checking that both the specified proxy gear and
  the target gear had the specified status in the proxy gear.  However, the
  only callers of `assert_gear_status_in_proxy` were
  `assert_proxies_not_disabled` and `assert_proxies_disabled`, and both of
  these methods called `assert_gear_status_in_proxy` for each pair of gears
  in both orderings.  Consequently, `assert_gear_status_in_proxy` was
  performing redundant checks.  For example, first
  `assert_proxies_disabled` would call `assert_gear_status_in_proxy` with
  `proxy=gear1` and `target_gear=gear1`, and so
  `assert_gear_status_in_proxy` would check that `gear1` had the right
  status for `gear1`.  Later, `assert_proxies_disabled` would call
  `assert_gear_status_in_proxy` with `proxy=gear1` and `target_gear=gear2`,
  and so `assert_gear_status_in_proxy` would check that `gear1` had the
  right status for `gear1` again as well as checking that it had the right
  status for `gear2`.  This issue is addressed by reworking the logic in
  `assert_proxies_disabled` and `assert_proxies_not_disabled` to use
  a single loop and pass a list of target gears to
  `assert_gear_status_in_proxy`.

• Second, the logic in `assert_gear_status_in_proxy` that was intended to
  check the status of both gears was allowing the test to succeed even when
  only one gear had the expected status.  This issue is addressed by using
  a hash to keep track of which gears are found to have the correct status.

• Finally, in certain cases, `assert_gear_status_in_proxy` was getting the
  status from the wrong proxy gear because the primary gear was aliased by
  the secondary gear (see commit 4e6cb00).
  In test runs where this happened, the node would forward requests that
  used the primary proxy's host name to the secondary proxy, and so the
  returned status would show the secondary gear under the name "local-gear"
  rather than the expected name, causing the test to fail to find the
  secondary proxy, thus causing spurious test failures.  This issue is
  addressed by building a more inclusive list of expected gear names and
  checking only that the expected number of matching gears have the
  expected status, so the test should pass even if it gets the wrong status
  page and one of the gears that otherwise would not be under the name
  "local-gear" is under that name.

In `assert_gear_status_in_proxy`, construct the gear name by taking the
host name with the domain name removed, instead of trying to extract the
application name or gear UUID from the host name and constructing the gear
name from that.  The logic to extract the application name from the host
name worked by grabbing the first part of the host name up to the first
hyphen, but sometimes gears have host names and gear-registry entry names
of the form "<namespace>-<application>-<n>-<namespace>" instead of the
expected "<application>-<namespace>" or "<gearuuid>-<namespace>", and so
the logic would erroneously take the namespace as the application name/gear
UUID, look for a gear named "<namespace>-<namespace>", and consequently
fail.

Delete the redundant `assert_match` in `assert_proxies_disabled` that
prevented the flunk from triggering.

Fix the assertion flunk message in `assert_proxies_disabled` so that it
prints the correct gear names instead of raising a `NoMethodError`
exception for `target_gear.name`.
  • Loading branch information
Miciah committed Nov 5, 2015
1 parent 43112af commit edb9ea0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 34 deletions.
25 changes: 11 additions & 14 deletions node/test/gear_functional/multi_ha_func_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,27 +81,24 @@ def test_ha_enable
@api.assert_scales_to(app_name, @framework_cartridge, 6)
assert_proxies_not_disabled(proxy_entries)
@api.assert_scales_to(app_name, @framework_cartridge, 7)
assert_proxies_disabled(proxy_entries)
assert_proxies_disabled(proxy_entries)
end

def assert_proxies_status(proxy_entries, status)
proxy_entries.values.each do |proxy|
logger.info "Checking target gears " +
proxy_entries.values.map {|p| p.dns}.join(', ') +
" status from proxy #{proxy.dns}"
@api.assert_gear_status_in_proxy(proxy, proxy_entries.values, status)
end
end

def assert_proxies_disabled(proxy_entries)
proxy_entries.values.each do |target_gear|
proxy_entries.values.each do |proxy|
logger.info "Checking target gear #{target_gear.dns} status from proxy #{proxy.dns}"
@api.assert_gear_status_in_proxy(proxy, target_gear, 'MAINT')
end
end
assert_proxies_status(proxy_entries, 'MAINT')
end

def assert_proxies_not_disabled(proxy_entries)
proxy_entries.values.each do |target_gear|
proxy_entries.values.each do |proxy|
logger.info "Checking target gear #{target_gear.dns} status from proxy #{proxy.dns}"
@api.assert_gear_status_in_proxy(proxy, target_gear, 'UP')
end
end
assert_proxies_status(proxy_entries, 'UP')
end

end
37 changes: 17 additions & 20 deletions node/test/support/functional_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,38 +404,35 @@ def cloud_domain
::OpenShift::Config.new.get('CLOUD_DOMAIN')
end

def assert_gear_status_in_proxy(proxy, target_gear, status)
def assert_gear_status_in_proxy(proxy, target_gears, status)
passed = false
names = target_gears.inject({ "local-gear" => false}) do |h, gear|
h["gear-" + gear.dns.split('.')[0]] = false
h
end

num_tries = 3
(1..num_tries).each do |i|
proxy_status_csv = `curl "http://#{proxy.dns}/haproxy-status/;csv" 2>/dev/null`

if proxy.uuid == target_gear.uuid
names = [ 'local-gear' ]
else
gear_name = target_gear.dns.split('-')[0]
names = [ "gear-#{gear_name}-#{target_gear.namespace}" ]
gear_name = proxy.dns.split('-')[0]
names << "gear-#{gear_name}-#{target_gear.namespace}"
end

proxy_status_csv = `curl 'http://#{proxy.dns}/haproxy-status/;csv' 2>/dev/null`
proxy_status_csv.split("\n").each do |line|
names.each do |name|
names.keys.each do |name|
if line =~ /#{name}/
if line =~ /#{status}/
passed = true
elsif i == num_tries
assert_match /#{status}/, line
end
break
names[name] = line =~ /#{status}/
end
end
end
passed = target_gears.length == names.inject(0) do |len, (k, v)|
v ? len+1 : len
end
break if passed
sleep 2
end

flunk("Target gear #{target_gear.name} did not have expected status #{status}") unless passed
if !passed
failed = names.inject([]) {|list, (k, v)| list << k if !v; list }
flunk("Target gears " + failed.join(', ') +
" did not have expected status #{status}")
end
end

def restart_cartridge(app_name, cartridge)
Expand Down

0 comments on commit edb9ea0

Please sign in to comment.