openstack/ovn-octavia-provider
Code Issues Proposed changes

Add maintenance task to update entities on component start

Browse Source 
Some changes require updating the existing entities in a
clear and transparent way for the user.

This patch adds a mechanism to create separate tasks that
can run periodically or just once in order to update or
modify existing entities that require changes after a new
patch or RFE.

As an example, a first task has been included for updating
existing OVN LB HM ports, changing their device_owner, and
adding their device_id.

Closes-Bug: 2038091
Change-Id: I0d4feb1e5c128d5a768d1b87deb2dcb3ab6d1ea1
This commit is contained in:
Fernando Royo 2023-10-17 10:49:23 +02:00
parent e2dbc59be5
commit 1661f3815c
5 changed files with 278 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ovn_octavia_provider/agent.py
View File

@ -17,6 +17,7 @@ from oslo_log import log as logging
from ovn_octavia_provider.common import config as ovn_conf
from ovn_octavia_provider import event as ovn_event
from ovn_octavia_provider import helper as ovn_helper
from ovn_octavia_provider import maintenance
from ovn_octavia_provider.ovsdb import impl_idl_ovn
@ -50,6 +51,15 @@ def OvnProviderAgent(exit_event):
ovn_sb_idl_for_events.notify_handler.watch_events(sb_events)
ovn_sb_idl_for_events.start()
# NOTE(froyo): Maintenance task initialization added here
# as it will be a long life task managed through the Octavia
# driver agent -- unlike the OVNProviderDriver which is a
# short life service invocated by Octavia API.
maintenance_thread = maintenance.MaintenanceThread()
maintenance_thread.add_periodics(
maintenance.DBInconsistenciesPeriodics())
maintenance_thread.start()
LOG.info('OVN provider agent has started.')
exit_event.wait()
LOG.info('OVN provider agent is exiting.')
@ -57,3 +67,4 @@ def OvnProviderAgent(exit_event):
ovn_nb_idl_for_events.stop()
ovn_sb_idl_for_events.notify_handler.unwatch_events(sb_events)
ovn_sb_idl_for_events.stop()
maintenance_thread.stop()

120
ovn_octavia_provider/maintenance.py Normal file
View File

@ -0,0 +1,120 @@
# Copyright 2023 Red Hat, Inc. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import inspect
import threading
from futurist import periodics
from neutron_lib import constants as n_const
from oslo_config import cfg
from oslo_log import log as logging
from ovn_octavia_provider.common import clients
from ovn_octavia_provider.common import constants as ovn_const
from ovn_octavia_provider.ovsdb import impl_idl_ovn
CONF = cfg.CONF # Gets Octavia Conf as it runs under o-api domain
LOG = logging.getLogger(__name__)
class MaintenanceThread(object):
def __init__(self):
self._callables = []
self._thread = None
self._worker = None
def add_periodics(self, obj):
for name, member in inspect.getmembers(obj):
if periodics.is_periodic(member):
LOG.info('Periodic task found: %(owner)s.%(member)s',
{'owner': obj.__class__.__name__, 'member': name})
self._callables.append((member, (), {}))
def start(self):
if self._thread is None:
self._worker = periodics.PeriodicWorker(self._callables)
self._thread = threading.Thread(target=self._worker.start)
self._thread.daemon = True
self._thread.start()
def stop(self):
self._worker.stop()
self._worker.wait()
self._thread.join()
self._worker = self._thread = None
class DBInconsistenciesPeriodics(object):
def __init__(self):
self.ovn_nbdb = impl_idl_ovn.OvnNbIdlForLb()
self.ovn_nbdb_api = self.ovn_nbdb.start()
@periodics.periodic(spacing=600, run_immediately=True)
def change_device_owner_lb_hm_ports(self):
"""Change the device_owner for the OVN LB HM port existing.
The OVN LB HM port used for send the health checks to the backend
members has a new device_owner, it will use the value
onv-lb-hm:distributed in order to keep the behaviour on Neutron as a
LOCALPORT. Also this change will add device-id as ovn-lb-hm:{subnet}
to get more robust.
"""
LOG.debug('Maintenance task: checking device_owner for OVN LB HM '
'ports.')
neutron_client = clients.get_neutron_client()
ovn_lb_hm_ports = neutron_client.ports(
device_owner=n_const.DEVICE_OWNER_DISTRIBUTED)
check_neutron_support_new_device_owner = True
for port in ovn_lb_hm_ports:
if port.name.startswith('ovn-lb-hm'):
LOG.debug('Maintenance task: updating device_owner and '
'adding device_id for port id %s', port.id)
neutron_client.update_port(
port.id, device_owner=ovn_const.OVN_LB_HM_PORT_DISTRIBUTED,
device_id=port.name)
# NOTE(froyo): Check that the port is now of type LOCALPORT in
# the OVN NB DB or perform a rollback in other cases. Such
# cases could indicate that Neutron is in the process of being
# updated or that the user has forgotten to update Neutron to a
# version that supports this change
if check_neutron_support_new_device_owner:
port_ovn = self.ovn_nbdb_api.db_find_rows(
"Logical_Switch_Port", ("name", "=", port.id)).execute(
check_error=True)
if len(port_ovn) and port_ovn[0].type != 'localport':
LOG.debug('Maintenance task: port %s updated but '
'looks like Neutron does not support this '
'new device_owner, or maybe is updating '
'version, so restoring to old values and '
'waiting another iteration of this task',
port.id)
neutron_client.update_port(
port.id,
device_owner=n_const.DEVICE_OWNER_DISTRIBUTED,
device_id='')
# Break the loop as do not make sense change the rest
break
check_neutron_support_new_device_owner = False
else:
# NOTE(froyo): No ports found to update, or all of them done.
LOG.debug('Maintenance task: no more ports left, stopping the '
'periodic task.')
raise periodics.NeverAgain()
LOG.debug('Maintenance task: device_owner and device_id checked for '
'OVN LB HM ports.')

2
ovn_octavia_provider/tests/unit/test_agent.py
View File

@ -24,5 +24,5 @@ class TestOvnProviderAgent(ovn_base.TestOvnOctaviaBase):
mock_exit_event.is_set.side_effect = [False, False, False, False, True]
ovn_agent.OvnProviderAgent(mock_exit_event)
self.assertEqual(1, mock_exit_event.wait.call_count)
self.assertEqual(2, self.mock_ovn_nb_idl.call_count)
self.assertEqual(3, self.mock_ovn_nb_idl.call_count)
self.assertEqual(1, self.mock_ovn_sb_idl.call_count)

134
ovn_octavia_provider/tests/unit/test_maintenance.py Normal file
View File

@ -0,0 +1,134 @@
# Copyright 2023 Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
#
from unittest import mock
from futurist import periodics
from neutron_lib import constants as n_const
from ovn_octavia_provider.common import config as ovn_conf
from ovn_octavia_provider.common import constants as ovn_const
from ovn_octavia_provider import maintenance
from ovn_octavia_provider.tests.unit import base as ovn_base
from ovn_octavia_provider.tests.unit import fakes
class TestDBInconsistenciesPeriodics(ovn_base.TestOvnOctaviaBase):
def setUp(self):
ovn_conf.register_opts()
super(TestDBInconsistenciesPeriodics, self).setUp()
self.maint = maintenance.DBInconsistenciesPeriodics()
self.ovn_nbdb_api = mock.patch.object(self.maint, 'ovn_nbdb_api')
self.ovn_nbdb_api.start()
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
def test_change_device_owner_lb_hm_ports(self, net_cli):
ovn_lb_hm_ports = [
fakes.FakePort.create_one_port(
attrs={
'id': 'foo',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-metadata-foo'}),
fakes.FakePort.create_one_port(
attrs={
'id': 'foo1',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-lb-hm-foo1'}),
fakes.FakePort.create_one_port(
attrs={
'id': 'foo2',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-lb-hm-foo2'})]
net_cli.return_value.ports.return_value = ovn_lb_hm_ports
self.assertRaises(periodics.NeverAgain,
self.maint.change_device_owner_lb_hm_ports)
expected_dict_1 = {
'device_owner': ovn_const.OVN_LB_HM_PORT_DISTRIBUTED,
'device_id': 'ovn-lb-hm-foo1',
}
expected_dict_2 = {
'device_owner': ovn_const.OVN_LB_HM_PORT_DISTRIBUTED,
'device_id': 'ovn-lb-hm-foo2',
}
expected_call = [
mock.call(),
mock.call().ports(device_owner=n_const.DEVICE_OWNER_DISTRIBUTED),
mock.call().update_port('foo1', **expected_dict_1),
mock.call().update_port('foo2', **expected_dict_2)]
net_cli.assert_has_calls(expected_call)
self.maint.ovn_nbdb_api.db_find_rows.assert_called_once_with(
"Logical_Switch_Port", ("name", "=", 'foo1'))
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
def test_change_device_owner_lb_hm_ports_neutron_version_doesnt_match(
self, net_cli):
ovn_lb_hm_ports = [
fakes.FakePort.create_one_port(
attrs={
'id': 'foo',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-metadata-foo'}),
fakes.FakePort.create_one_port(
attrs={
'id': 'foo1',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-lb-hm-foo1'}),
fakes.FakePort.create_one_port(
attrs={
'id': 'foo2',
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'name': 'ovn-lb-hm-foo2'})]
net_cli.return_value.ports.return_value = ovn_lb_hm_ports
self.maint.ovn_nbdb_api.db_find_rows.return_value.\
execute.return_value = [
fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={
'id': 'uuid-foo',
'type': 'foo'})]
self.maint.change_device_owner_lb_hm_ports()
expected_dict_change = {
'device_owner': ovn_const.OVN_LB_HM_PORT_DISTRIBUTED,
'device_id': 'ovn-lb-hm-foo1',
}
expected_dict_rollback = {
'device_owner': n_const.DEVICE_OWNER_DISTRIBUTED,
'device_id': '',
}
expected_call = [
mock.call(),
mock.call().ports(device_owner=n_const.DEVICE_OWNER_DISTRIBUTED),
mock.call().update_port('foo1', **expected_dict_change),
mock.call().update_port('foo1', **expected_dict_rollback)]
net_cli.assert_has_calls(expected_call)
self.maint.ovn_nbdb_api.db_find_rows.assert_called_once_with(
"Logical_Switch_Port", ("name", "=", 'foo1'))
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
def test_change_device_owner_lb_hm_ports_no_ports_to_change(self, net_cli):
ovn_lb_hm_ports = []
net_cli.return_value.ports.return_value = ovn_lb_hm_ports
self.assertRaises(periodics.NeverAgain,
self.maint.change_device_owner_lb_hm_ports)
expected_call = [
mock.call(),
mock.call().ports(device_owner=n_const.DEVICE_OWNER_DISTRIBUTED),
]
net_cli.assert_has_calls(expected_call)
self.maint.ovn_nbdb_api.db_find_rows.assert_not_called()

12
releasenotes/notes/maintenance-task-framework-a366242c89f5af11.yaml Normal file
View File

@ -0,0 +1,12 @@
---
fixes:
- |
A maintenance task process has been added to update the existing OVN LB HM
ports to the new behaviour defined. Specifically, the "device_owner" field
needs to be updated from network:distributed to ovn-lb-hm:distributed.
Additionally, the "device_id" will be populated during update action.
other:
- |
A maintenance task thread has been added to work on periodic and one-shot
tasks that also allows the future changes to perform the needed upgrades
actions.