Fix a python3 issue in the amphora-agent

An exception handler in the amphora-agent has a python3 string
comparison bug that will cause a TypeError.
This patch fixes that bug and adds test coverage for the
start_stop_listener.

Change-Id: I6f5d95c5f875edda530f54ae72386d6495235ca6
Story: 2005898
Task: 33760
(cherry picked from commit dc459e2213)
This commit is contained in:
Michael Johnson 2019-06-14 16:58:04 -07:00 committed by Nir Magnezi
parent 4b912e4829
commit d31b47fd0d
3 changed files with 151 additions and 6 deletions

View File

@ -259,7 +259,7 @@ class Listener(object):
try:
subprocess.check_output(cmd.split(), stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
if 'Job is already running' not in e.output:
if b'Job is already running' not in e.output:
LOG.debug(
"Failed to %(action)s haproxy-%(list)s service: %(err)s "
"%(out)s", {'action': action, 'list': listener_id,

View File

@ -39,7 +39,7 @@ import octavia.tests.unit.base as base
AMP_AGENT_CONF_PATH = '/etc/octavia/amphora-agent.conf'
RANDOM_ERROR = 'random error'
RANDOM_ERROR = b'random error'
OK = dict(message='OK')
@ -330,7 +330,7 @@ class TestServerTestCase(base.TestCase):
self.assertEqual(
{
'message': 'Error starting haproxy',
'details': RANDOM_ERROR,
'details': RANDOM_ERROR.decode('utf-8'),
}, jsonutils.loads(rv.data.decode('utf-8')))
mock_subprocess.assert_called_with(
['/usr/sbin/service', 'haproxy-123', 'start'], stderr=-2)
@ -1390,7 +1390,7 @@ class TestServerTestCase(base.TestCase):
data=jsonutils.dumps(port_info))
self.assertEqual(500, rv.status_code)
self.assertEqual(
{'details': RANDOM_ERROR,
{'details': RANDOM_ERROR.decode('utf-8'),
'message': 'Error plugging network'},
jsonutils.loads(rv.data.decode('utf-8')))
@ -1929,7 +1929,7 @@ class TestServerTestCase(base.TestCase):
data=jsonutils.dumps(subnet_info))
self.assertEqual(500, rv.status_code)
self.assertEqual(
{'details': RANDOM_ERROR,
{'details': RANDOM_ERROR.decode('utf-8'),
'message': 'Error plugging VIP'},
jsonutils.loads(rv.data.decode('utf-8')))
@ -2308,7 +2308,7 @@ class TestServerTestCase(base.TestCase):
data=jsonutils.dumps(subnet_info))
self.assertEqual(500, rv.status_code)
self.assertEqual(
{'details': RANDOM_ERROR,
{'details': RANDOM_ERROR.decode('utf-8'),
'message': 'Error plugging VIP'},
jsonutils.loads(rv.data.decode('utf-8')))

View File

@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import subprocess
import mock
from oslo_utils import uuidutils
@ -190,3 +192,146 @@ class ListenerTestCase(base.TestCase):
self.assertEqual(
consts.OFFLINE,
self.test_listener._check_haproxy_status(LISTENER_ID1))
@mock.patch('octavia.amphorae.backends.agent.api_server.listener.Listener.'
'_check_haproxy_status')
@mock.patch('octavia.amphorae.backends.agent.api_server.listener.Listener.'
'vrrp_check_script_update')
@mock.patch('os.path.exists')
@mock.patch('octavia.amphorae.backends.agent.api_server.listener.Listener.'
'_check_listener_exists')
@mock.patch('subprocess.check_output')
def test_start_stop_listener(self, mock_check_output, mock_list_exists,
mock_path_exists, mock_vrrp_update,
mock_check_status):
listener_id = uuidutils.generate_uuid()
mock_path_exists.side_effect = [False, True, True, False, False]
mock_check_status.side_effect = ['bogus', consts.OFFLINE]
# Happy path - No VRRP
ref_command_split = ['/usr/sbin/service']
ref_command_split.append('haproxy-{}'.format(listener_id))
ref_command_split.append(consts.AMP_ACTION_START)
result = self.test_listener.start_stop_listener(
listener_id, consts.AMP_ACTION_START)
mock_check_output.assert_called_once_with(ref_command_split,
stderr=subprocess.STDOUT)
mock_list_exists.assert_called_once_with(listener_id)
mock_vrrp_update.assert_not_called()
self.assertEqual(202, result.status_code)
self.assertEqual('OK', result.json['message'])
ref_details = ('Configuration file is valid\n'
'haproxy daemon for {0} started'.format(listener_id))
self.assertEqual(ref_details, result.json['details'])
# Happy path - VRRP - RELOAD
mock_list_exists.reset_mock()
mock_vrrp_update.reset_mock()
mock_check_output.reset_mock()
ref_command_split = ['/usr/sbin/service']
ref_command_split.append('haproxy-{}'.format(listener_id))
ref_command_split.append(consts.AMP_ACTION_RELOAD)
result = self.test_listener.start_stop_listener(
listener_id, consts.AMP_ACTION_RELOAD)
mock_check_output.assert_called_once_with(ref_command_split,
stderr=subprocess.STDOUT)
mock_list_exists.assert_called_once_with(listener_id)
mock_vrrp_update.assert_called_once_with(listener_id,
consts.AMP_ACTION_RELOAD)
self.assertEqual(202, result.status_code)
self.assertEqual('OK', result.json['message'])
ref_details = ('Listener {0} {1}ed'.format(listener_id,
consts.AMP_ACTION_RELOAD))
self.assertEqual(ref_details, result.json['details'])
# Happy path - VRRP - RELOAD - OFFLINE
mock_list_exists.reset_mock()
mock_vrrp_update.reset_mock()
mock_check_output.reset_mock()
ref_command_split = ['/usr/sbin/service']
ref_command_split.append('haproxy-{}'.format(listener_id))
ref_command_split.append(consts.AMP_ACTION_START)
result = self.test_listener.start_stop_listener(
listener_id, consts.AMP_ACTION_RELOAD)
mock_check_output.assert_called_once_with(ref_command_split,
stderr=subprocess.STDOUT)
mock_list_exists.assert_called_once_with(listener_id)
mock_vrrp_update.assert_called_once_with(listener_id,
consts.AMP_ACTION_RELOAD)
self.assertEqual(202, result.status_code)
self.assertEqual('OK', result.json['message'])
ref_details = ('Configuration file is valid\n'
'haproxy daemon for {0} started'.format(listener_id))
self.assertEqual(ref_details, result.json['details'])
# Unhappy path - Not already running
mock_list_exists.reset_mock()
mock_vrrp_update.reset_mock()
mock_check_output.reset_mock()
ref_command_split = ['/usr/sbin/service']
ref_command_split.append('haproxy-{}'.format(listener_id))
ref_command_split.append(consts.AMP_ACTION_START)
mock_check_output.side_effect = subprocess.CalledProcessError(
output=b'bogus', returncode=-2, cmd='sit')
result = self.test_listener.start_stop_listener(
listener_id, consts.AMP_ACTION_START)
mock_check_output.assert_called_once_with(ref_command_split,
stderr=subprocess.STDOUT)
mock_list_exists.assert_called_once_with(listener_id)
mock_vrrp_update.assert_not_called()
self.assertEqual(500, result.status_code)
self.assertEqual('Error {}ing haproxy'.format(consts.AMP_ACTION_START),
result.json['message'])
self.assertEqual('bogus', result.json['details'])
# Unhappy path - Already running
mock_list_exists.reset_mock()
mock_vrrp_update.reset_mock()
mock_check_output.reset_mock()
ref_command_split = ['/usr/sbin/service']
ref_command_split.append('haproxy-{}'.format(listener_id))
ref_command_split.append(consts.AMP_ACTION_START)
mock_check_output.side_effect = subprocess.CalledProcessError(
output=b'Job is already running', returncode=-2, cmd='sit')
result = self.test_listener.start_stop_listener(
listener_id, consts.AMP_ACTION_START)
mock_check_output.assert_called_once_with(ref_command_split,
stderr=subprocess.STDOUT)
mock_list_exists.assert_called_once_with(listener_id)
mock_vrrp_update.assert_not_called()
self.assertEqual(202, result.status_code)
self.assertEqual('OK', result.json['message'])
ref_details = ('Configuration file is valid\n'
'haproxy daemon for {0} started'.format(listener_id))
self.assertEqual(ref_details, result.json['details'])
# Invalid action
mock_check_output.reset_mock()
mock_list_exists.reset_mock()
mock_path_exists.reset_mock()
mock_vrrp_update.reset_mock()
result = self.test_listener.start_stop_listener(listener_id, 'bogus')
self.assertEqual(400, result.status_code)
self.assertEqual('Invalid Request', result.json['message'])
self.assertEqual('Unknown action: bogus', result.json['details'])
mock_list_exists.assert_not_called()
mock_path_exists.assert_not_called()
mock_vrrp_update.assert_not_called()
mock_check_output.assert_not_called()