openshift / origin-server

OpenShift 2 (deprecated)
889 stars 516 forks source link

assert_proxies_disabled: Fix failures #6304

Open Miciah opened 8 years ago

Miciah commented 8 years ago

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 4e6cb009757fecf19cf26520645c3e0e839f38fa). 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]

openshift-bot commented 8 years ago

Evaluated for online test up to edb9ea0832c0882537453c45e9ef1473d3357ca3

openshift-bot commented 8 years ago

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