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

assert_proxies_disabled: Fix failures #6304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Nov 5, 2015

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.

[test] [extended:gear]

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`.
@Miciah Miciah force-pushed the fix-assert_proxies_disabled branch from 9d35126 to edb9ea0 Compare November 5, 2015 04:43
@openshift-bot
Copy link

Evaluated for online test up to edb9ea0

@openshift-bot
Copy link

Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9020/) (Extended Tests: gear)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants