Make clear distinction between health check and health monitor

There was quite a mix between (Octavia) Health Monitors and
(OVN) Load Balancer Health Checks. This patch tries to make a
more clear distinction between Octavia HM and OVN LB HCs.

This patch also add a reference to the Octavia Health Monitors
Ids to the external_ids of the OVN NB Load_Balancer entries.

Related-Bug: #1997418

Change-Id: Ib8499d7c4ea102e183ead31f063a3c0a70af6e23
(cherry picked from commit 9cb8cd5054)
This commit is contained in:
Luis Tomas Bolivar 2022-11-21 14:23:37 +01:00
parent ccfcf6e2e6
commit 130ed10bed
4 changed files with 80 additions and 62 deletions

View File

@ -38,6 +38,7 @@ LB_EXT_IDS_POOL_PREFIX = 'pool_'
LB_EXT_IDS_LISTENER_PREFIX = 'listener_'
LB_EXT_IDS_MEMBER_PREFIX = 'member_'
LB_EXT_IDS_HM_KEY = 'octavia:healthmonitor'
LB_EXT_IDS_HMS_KEY = 'octavia:healthmonitors'
LB_EXT_IDS_VIP_KEY = 'neutron:vip'
LB_EXT_IDS_VIP_FIP_KEY = 'neutron:vip_fip'
LB_EXT_IDS_VIP_PORT_ID_KEY = 'neutron:vip_port_id'

View File

@ -81,6 +81,6 @@ class ServiceMonitorUpdateEvent(row_event.RowEvent):
{'event': event,
'row': row})
if event == self.ROW_DELETE:
self.driver.hm_update_event_handler(row, sm_delete_event=True)
self.driver.sm_update_event_handler(row, sm_delete_event=True)
elif event == self.ROW_UPDATE:
self.driver.hm_update_event_handler(row)
self.driver.sm_update_event_handler(row)

View File

@ -1300,8 +1300,8 @@ class OvnProviderHelper():
operating_status = constants.OFFLINE
if (ovn_lb.health_check and
not self._update_hm_vip(ovn_lb,
listener[constants.PROTOCOL_PORT])):
not self._update_lbhc_vip(ovn_lb,
listener[constants.PROTOCOL_PORT])):
operating_status = constants.ERROR
status = {
@ -2210,7 +2210,7 @@ class OvnProviderHelper():
# We found particular port
return port
def _add_hm(self, ovn_lb, pool_key, info):
def _add_lbhc(self, ovn_lb, pool_key, info):
hm_id = info[constants.ID]
status = {constants.ID: hm_id,
constants.PROVISIONING_STATUS: constants.ERROR,
@ -2257,6 +2257,10 @@ class OvnProviderHelper():
operating_status = constants.ONLINE
if not info['admin_state_up']:
operating_status = constants.OFFLINE
hms_key = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_HMS_KEY, [])
if hms_key:
hms_key = jsonutils.loads(hms_key)
try:
with self.ovn_nbdb_api.transaction(check_error=True) as txn:
health_check = txn.add(
@ -2266,6 +2270,11 @@ class OvnProviderHelper():
txn.add(self.ovn_nbdb_api.db_add(
'Load_Balancer', ovn_lb.uuid,
'health_check', health_check))
hms_key.append(hm_id)
txn.add(self.ovn_nbdb_api.db_set(
'Load_Balancer', ovn_lb.uuid,
('external_ids', {ovn_const.LB_EXT_IDS_HMS_KEY:
jsonutils.dumps(hms_key)})))
status = {constants.ID: hm_id,
constants.PROVISIONING_STATUS: constants.ACTIVE,
constants.OPERATING_STATUS: operating_status}
@ -2274,23 +2283,23 @@ class OvnProviderHelper():
LOG.exception(ovn_const.EXCEPTION_MSG, "set of health check")
return status
def _update_hm_vip(self, ovn_lb, vip_port):
hm = self._lookup_hm_by_id(ovn_lb.health_check)
if not hm:
LOG.error("Could not find HM with key: %s", ovn_lb.health_check)
def _update_lbhc_vip(self, ovn_lb, vip_port):
lbhc = self._lookup_lbhc_by_hm_id(ovn_lb.health_check)
if not lbhc:
LOG.error("Could not find HC with key: %s", ovn_lb.health_check)
return False
vip = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_KEY)
if not vip:
LOG.error("Could not find VIP for HM %s, LB external_ids: %s",
hm.uuid, ovn_lb.external_ids)
LOG.error("Could not find VIP for HC %s, LB external_ids: %s",
lbhc.uuid, ovn_lb.external_ids)
return False
vip = vip + ':' + str(vip_port)
commands = []
commands.append(
self.ovn_nbdb_api.db_set(
'Load_Balancer_Health_Check', hm.uuid,
'Load_Balancer_Health_Check', lbhc.uuid,
('vip', vip)))
self._execute_commands(commands)
return True
@ -2360,35 +2369,33 @@ class OvnProviderHelper():
self._execute_commands(commands)
return True
def _lookup_hm_by_id(self, hm_id):
hms = self.ovn_nbdb_api.db_list_rows(
def _lookup_lbhc_by_hm_id(self, hm_id):
lbhcs = self.ovn_nbdb_api.db_list_rows(
'Load_Balancer_Health_Check').execute(check_error=True)
for hm in hms:
if (ovn_const.LB_EXT_IDS_HM_KEY in hm.external_ids and
hm.external_ids[ovn_const.LB_EXT_IDS_HM_KEY] == hm_id):
return hm
for lbhc in lbhcs:
if (ovn_const.LB_EXT_IDS_HM_KEY in lbhc.external_ids and
lbhc.external_ids[ovn_const.LB_EXT_IDS_HM_KEY] == hm_id):
return lbhc
raise idlutils.RowNotFound(table='Load_Balancer_Health_Check',
col='external_ids', match=hm_id)
def _lookup_lb_by_hm_id(self, hm_id):
lbs = self.ovn_nbdb_api.db_find_rows(
'Load_Balancer', ('health_check', '=', [hm_id])).execute()
return lbs[0] if lbs else None
def _find_ovn_lb_from_hm_id(self, hm_id):
try:
hm = self._lookup_hm_by_id(hm_id)
except idlutils.RowNotFound:
LOG.debug("Loadbalancer health monitor %s not found!", hm_id)
return None, None
lbs = self.ovn_nbdb_api.db_list_rows(
'Load_Balancer').execute(check_error=True)
ovn_lb = None
for lb in lbs:
if (ovn_const.LB_EXT_IDS_HMS_KEY in lb.external_ids.keys() and
hm_id in lb.external_ids[ovn_const.LB_EXT_IDS_HMS_KEY]):
ovn_lb = lb
break
try:
ovn_lb = self._lookup_lb_by_hm_id(hm.uuid)
lbhc = self._lookup_lbhc_by_hm_id(hm_id)
except idlutils.RowNotFound:
LOG.debug("Loadbalancer not found with health_check %s !", hm.uuid)
return hm, None
LOG.debug("Loadbalancer health check %s not found!", hm_id)
return None, ovn_lb
return hm, ovn_lb
return lbhc, ovn_lb
def hm_create(self, info):
status = {
@ -2443,7 +2450,7 @@ class OvnProviderHelper():
# ${OVN_LB_ID} health_check @hc
# options here are interval, timeout, failure_count and success_count
# from info object passed-in
hm_status = self._add_hm(ovn_lb, pool_key, info)
hm_status = self._add_lbhc(ovn_lb, pool_key, info)
if hm_status[constants.PROVISIONING_STATUS] == constants.ACTIVE:
if not self._update_hm_members(ovn_lb, pool_key):
hm_status[constants.PROVISIONING_STATUS] = constants.ERROR
@ -2461,9 +2468,9 @@ class OvnProviderHelper():
hm_id = info[constants.ID]
pool_id = info[constants.POOL_ID]
hm, ovn_lb = self._find_ovn_lb_from_hm_id(hm_id)
if not hm:
LOG.debug("Loadbalancer health monitor %s not found!", hm_id)
lbhc, ovn_lb = self._find_ovn_lb_from_hm_id(hm_id)
if not lbhc:
LOG.debug("Loadbalancer health check %s not found!", hm_id)
return status
if not ovn_lb:
LOG.debug("Could not find LB with health monitor id %s", hm_id)
@ -2486,7 +2493,7 @@ class OvnProviderHelper():
commands = []
commands.append(
self.ovn_nbdb_api.db_set(
'Load_Balancer_Health_Check', hm.uuid,
'Load_Balancer_Health_Check', lbhc.uuid,
('options', options)))
self._execute_commands(commands)
@ -2514,11 +2521,12 @@ class OvnProviderHelper():
constants.OPERATING_STATUS: constants.NO_MONITOR,
constants.PROVISIONING_STATUS: constants.DELETED}]}
hm, ovn_lb = self._find_ovn_lb_from_hm_id(hm_id)
if not hm or not ovn_lb:
LOG.debug("Loadbalancer Health Check %s not found in OVN "
"Northbound DB. Setting the Loadbalancer Health "
"Monitor status to DELETED in Octavia", hm_id)
lbhc, ovn_lb = self._find_ovn_lb_from_hm_id(hm_id)
if not lbhc or not ovn_lb:
LOG.debug("Loadbalancer Health Check associated to Health Monitor "
"%s not found in OVN Northbound DB. Setting the "
"Loadbalancer Health Monitor status to DELETED in "
"Octavia", hm_id)
return status
# Need to send pool info in status update to avoid immutable objects,
@ -2532,16 +2540,26 @@ class OvnProviderHelper():
# ovn-nbctl clear load_balancer ${OVN_LB_ID} ip_port_mappings
# ovn-nbctl clear load_balancer ${OVN_LB_ID} health_check
# TODO(haleyb) remove just the ip_port_mappings for this hm
hms_key = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_HMS_KEY, [])
if hms_key:
hms_key = jsonutils.loads(hms_key)
if lbhc.uuid in hms_key:
hms_key.remove(lbhc.uuid)
commands = []
commands.append(
self.ovn_nbdb_api.db_clear('Load_Balancer', ovn_lb.uuid,
'ip_port_mappings'))
commands.append(
self.ovn_nbdb_api.db_remove('Load_Balancer', ovn_lb.uuid,
'health_check', hm.uuid))
'health_check', lbhc.uuid))
commands.append(
self.ovn_nbdb_api.db_set(
'Load_Balancer', ovn_lb.uuid,
('external_ids', {ovn_const.LB_EXT_IDS_HMS_KEY:
jsonutils.dumps(hms_key)})))
commands.append(
self.ovn_nbdb_api.db_destroy('Load_Balancer_Health_Check',
hm.uuid))
lbhc.uuid))
self._execute_commands(commands)
status = {
constants.LOADBALANCERS: [
@ -2592,24 +2610,24 @@ class OvnProviderHelper():
('protocol', '=', row.protocol[0])).execute()
return lbs if lbs else None
def hm_update_event_handler(self, row, sm_delete_event=False):
def sm_update_event_handler(self, row, sm_delete_event=False):
# NOTE(froyo): When a delete event is triggered, the Service_Monitor
# deleted row will include the last valid information, e.g. when the
# port is directly removed from the VM, the status will be 'online',
# in order to protect from this behaviour, we will set manually the
# status to 'offline' if sm_delete_event is reported as True.
try:
ovn_lb = self._get_lbs_on_hm_event(row)
ovn_lbs = self._get_lbs_on_hm_event(row)
except idlutils.RowNotFound:
LOG.debug("Load balancer information not found")
return
if not ovn_lb:
if not ovn_lbs:
LOG.debug("Load balancer not found")
return
request_info = {
"ovn_lb": ovn_lb,
"ovn_lbs": ovn_lbs,
"ip": row.ip,
"port": str(row.port),
"status": row.status
@ -2730,7 +2748,7 @@ class OvnProviderHelper():
return status
def hm_update_event(self, info):
ovn_lbs = info['ovn_lb']
ovn_lbs = info['ovn_lbs']
statuses = []
for ovn_lb in ovn_lbs:

View File

@ -98,11 +98,11 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self.ovn_hm_lb = mock.MagicMock()
self.ovn_hm_lb.protocol = ['tcp']
self.ovn_hm_lb.uuid = uuidutils.generate_uuid()
self.ovn_hm_lb.health_check = []
self.ovn_hm = mock.MagicMock()
self.ovn_hm.uuid = self.healthmonitor_id
self.ovn_hm.external_ids = {
ovn_const.LB_EXT_IDS_HM_KEY: self.ovn_hm.uuid}
self.ovn_hm_lb.health_check = [self.ovn_hm.uuid]
self.member_line = (
'member_%s_%s:%s_%s' %
(self.member_id, self.member_address,
@ -120,6 +120,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
ovn_const.LB_EXT_IDS_VIP_KEY: '10.22.33.99',
ovn_const.LB_EXT_IDS_VIP_FIP_KEY: '123.123.123.99',
ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: 'foo_hm_port',
ovn_const.LB_EXT_IDS_HMS_KEY: '["%s"]' % (self.ovn_hm.uuid),
'enabled': True,
'pool_%s' % self.pool_id: [],
'listener_%s' % self.listener_id: '80:pool_%s' % self.pool_id,
@ -3578,7 +3579,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self.assertEqual(status['healthmonitors'][0]['operating_status'],
constants.ERROR)
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_hm_by_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_lbhc_by_hm_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_get_or_create_ovn_lb')
def test_hm_create_then_listener_create(self, get_ovn_lb, lookup_hm):
get_ovn_lb.return_value = self.ovn_hm_lb
@ -3595,7 +3596,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self.assertEqual(status['listeners'][0]['operating_status'],
constants.ONLINE)
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_hm_by_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_lbhc_by_hm_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_get_or_create_ovn_lb')
def test_hm_create_then_listener_create_no_hm(self, get_ovn_lb, lookup_hm):
get_ovn_lb.return_value = self.ovn_hm_lb
@ -3609,7 +3610,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
constants.ERROR)
@mock.patch.object(ovn_helper.OvnProviderHelper, '_refresh_lb_vips')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_hm_by_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_lookup_lbhc_by_hm_id')
@mock.patch.object(ovn_helper.OvnProviderHelper, '_get_or_create_ovn_lb')
def test_hm_create_then_listener_create_no_vip(self, get_ovn_lb,
lookup_hm, refresh_vips):
@ -3690,9 +3691,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
def test_hm_delete(self):
self.helper.ovn_nbdb_api.db_list_rows.return_value.\
execute.return_value = [self.ovn_hm]
self.helper.ovn_nbdb_api.db_find_rows.return_value.\
execute.return_value = [self.ovn_hm_lb]
execute.side_effect = [[self.ovn_hm_lb], [self.ovn_hm]]
status = self.helper.hm_delete(self.health_monitor)
self.assertEqual(status['healthmonitors'][0]['provisioning_status'],
constants.DELETED)
@ -3758,7 +3757,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self.hm_update_event.run('update', row, mock.ANY)
expected = {
'info':
{'ovn_lb': [self.ovn_hm_lb],
{'ovn_lbs': [self.ovn_hm_lb],
'ip': self.member_address,
'port': self.member_port,
'status': ovn_const.HM_EVENT_MEMBER_PORT_OFFLINE},
@ -3786,7 +3785,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self.hm_update_event.run('delete', row, mock.ANY)
expected = {
'info':
{'ovn_lb': [self.ovn_hm_lb],
{'ovn_lbs': [self.ovn_hm_lb],
'ip': self.member_address,
'port': self.member_port,
'status': ovn_const.HM_EVENT_MEMBER_PORT_OFFLINE},
@ -3853,7 +3852,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
if bad_port:
port = 'bad-port'
info = {
'ovn_lb': [self.ovn_hm_lb],
'ovn_lbs': [self.ovn_hm_lb],
'ip': ip,
'logical_port': 'a-logical-port',
'src_ip': '10.22.33.4',
@ -3873,7 +3872,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
def _test_hm_update_status(self, ovn_lbs, member_id, ip, port,
member_status):
info = {
'ovn_lb': ovn_lbs,
'ovn_lbs': ovn_lbs,
'ip': ip,
'logical_port': 'a-logical-port',
'src_ip': '10.22.33.4',
@ -3975,7 +3974,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
ovn_hm_lb_2, fake_subnet, 8080, ip=member['address'])
info = {
'ovn_lb': [self.ovn_hm_lb, ovn_hm_lb_2],
'ovn_lbs': [self.ovn_hm_lb, ovn_hm_lb_2],
'ip': member['address'],
'logical_port': 'a-logical-port',
'src_ip': '10.22.33.4',
@ -4096,7 +4095,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
self._update_member_status(ovn_hm_lb2, member_lb2['id'], 'offline')
info = {
'ovn_lb': [self.ovn_hm_lb, ovn_hm_lb2],
'ovn_lbs': [self.ovn_hm_lb, ovn_hm_lb2],
'ip': ip_member,
'logical_port': 'a-logical-port',
'src_ip': '10.22.33.4',