In "test_restart_rpc_on_sighup_multiple_workers", the test needs to
wait until the RPC workers have been properly launched by
``oslo_service.service.ProcessLauncher.launch_service``. Once this
method returns, it is guaranteed that the child worker processes
are running and the signal process handlers are attending the
SIGHUP signal that will reset them.
Closes-Bug: #1938428
Change-Id: I1dc56092d099223accc3aefa8e303310c4f6787e
Instead of using a file to log the processes status and actions,
"TestNeutronServer" now uses a "multiprocessing.Queue" that is
safer than writting a single file accross multiple processes.
Change-Id: I6d04df180cd9b2d593bb99c8d22a60a3534f22a0
Closes-Bug: #1930367
After switch of the default Neutron backend to the ML2/OVN
in the Devstack we had 2 rally jobs running actually with the
same Neutron backend.
This patch configures explicitly ML2/OVS as a backend in the
'neutron-rally-task' job so it is like it was before the change in
the Devstack repo.
In order to fix the CI, this patch also skips "test_models_sync" test
cases. Once [1] is fixed and released, we'll be able to create SQL
tables specifying the "_constraints_included" flag. When creating a
table, this flag should be set to "False" in order to be able to set
the "index" and "unique" parameters for each column in the table
definition, as we do now.
"test_update_router_admin_state" is excluded from
"neutron-ovn-tempest-slow" until LP#1890445 is fixed.
Set "test_restart_wsgi_on_sighup_multiple_workers" as unstable.
CI job "neutron-ovn-tempest-slow" has been set to non-voting until
LP#1930402 is fixed.
Related-Bug: #1929518
Related-Bug: #1890445
Related-Bug: #1930402
Related-Bug: #1930367
[1]https://github.com/sqlalchemy/alembic/issues/848
Change-Id: I38ed7931169434fecbbefa3762f51a522ef3c509
Class "Timer" is not being used and the related unit tests
are failing quite often in py38 CI job.
Change-Id: Ib1d8f2f4adeadbd59c3dccc7be1546baedc4b978
Related-Bug: #1922563
In the functional tests module which is testing WSGI Server, there
is used function to assert that file created by the WSGI service is
created and has got correct size.
This function wasn't logging anything so in case of test failure
there was no way to check why it really failed.
This patch adds some log messages to the check so it will be clear
if file wasn't created at all or if size was not as expected.
Change-Id: Ib610b8513b4ea888a540873b26c3f205ac575b17
Related-Bug: #1886956
Now that we are python3 only, we should move to using the built
in version of mock that supports all of our testing needs and
remove the dependency on the "mock" package.
This completes removal of all references to "import mock",
changing to "from unittest import mock" in fullstack and
functional tests.
Added a hacking check to enforce it in future patches.
Change-Id: Ifcaf1c21bea0ec3c35278e49cecc90a101a82113
This is combined patch to fix couple of issues which
we recently had in gate.
1. [Functional tests] Fix SIGHUP handling tests
Tests in neutron.functional.test_server module are testing how
PluginWorker, WSGIWorker and RPCWorker are handling SIGHUP signal.
Recently this was changed in Oslo.service with [1] and our tests
were failing because they were still expecting that after sending
SIGHUP to the process, stop() and than start() method will be called.
But as our services uses "mutate" as restart method, since [1] such
process don't executes stop() and start() after SIGHUP. It now executes
only reset() method.
This patch reflects that change in Neutron functional tests.
2. Veth pair "IFLA_LINK" populated since kernel 4.15.0-60-generic
Since kernel_version=4.15.0-60-generic, "iproute2" provides the veth
pair index, even if the pair interface is in other namespace. In
previous versions, the parameter 'IFLA_LINK' was not present. We need
to handle both cases
[1] https://review.opendev.org/#/c/641907/
Co-Authored-By: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Change-Id: I7a3f20a795c89ab1ab037d046a1101cd5c0287d6
Closes-Bug: #1842659
Closes-Bug: #1842482
The test cases implemented calling "_test_restart_service_on_sighup",
mock the service "start" function. This function appends a temporary
file and this file is read by the main to check the correctness of
the service and workers executing.
In case of failure waiting for the file to be created and populated,
the main function will report now:
- If the file was not created.
- The current size of the file, compared to the expected one (the number
of times the "start" function was called)
The polling time to check the status of the file was increased to 1
second.
Change-Id: I68452408920ad9a1ee7252dd9012f898ddfa7e09
Related-Bug: #1833279
Some functional tests clases didn't inherit from
neutron.tests.functional.base.BaseLoggingTestCase class
and because of that there was no logs from such tests
in DEFAULT_LOG_DIR.
This patch changes it and logs should be available for all
functional tests.
TrivialFix
Change-Id: Ia71390afe9c8191179c626375ed683f242d35b3e
Fix W503 (line break before binary operator) pep8 warnings
and no longer ignore new failures.
Trivialfix
Change-Id: I7539f3b7187f2ad40681781f74b6e05a01bac474
We're depending on much newer psutil (3.x at least), so the 1.x
code path doesn't get executed anymore.
Change-Id: I335ebc0e8e49f20be7d05c968f0ddc79e5307820
neutron-lib 1.6 is out and among other things contains the rehomed
NeutronWorker class; now named BaseWorker in neutron-lib. This patch
updates references to use the lib version.
Since there's only 1 other consumer of these changes [1], this
patch depends on it so that it's free to merge after the single
consumer is updated.
[1] http://codesearch.openstack.org/?q=from%20neutron%20import%20worker
NeutronLibImpact
Change-Id: Id6bd414ae90119bf527bcaca6b88cc33ff79f685
Depends-On: Ifc0ecf6d5d3d4f8c406caad233d3648646724b12
Neutron Manager is loaded at the very startup of the neutron
server process and with it plugins are loaded and stored for
lookup purposes as their references are widely used across the
entire neutron codebase.
Rather than holding these references directly in NeutronManager
this patch refactors the code so that these references are held
by a plugin directory.
This allows subprojects and other parts of the Neutron codebase
to use the directory in lieu of the manager. The result is a
leaner, cleaner, and more decoupled code.
Usage pattern [1,2] can be translated to [3,4] respectively.
[1] manager.NeutronManager.get_service_plugins()[FOO]
[2] manager.NeutronManager.get_plugin()
[3] directory.get_plugin(FOO)
[4] directory.get_plugin()
The more entangled part is in the neutron unit tests, where the
use of the manager can be simplified as mocking is typically
replaced by a call to the directory add_plugin() method. This is
safe as each test case gets its own copy of the plugin directory.
That said, unit tests that look more like API tests and that rely on
the entire plugin machinery, need some tweaking to avoid stumbling
into plugin loading failures.
Due to the massive use of the manager, deprecation warnings are
considered impractical as they cause logs to bloat out of proportion.
Follow-up patches that show how to adopt the directory in neutron
subprojects are tagged with topic:plugin-directory.
NeutronLibImpact
Partially-implements: blueprint neutron-lib
Change-Id: I7331e914234c5f0b7abe836604fdd7e4067551cf
Stevedore documentation suggest that full import paths are not supposed
to be user visible. Since unit tests emulate users when configuring
oslo.config, we better off relying on well known plugin aliases than
internal details.
For in-tree that may be not a big deal, but with it we set a bad example
for third parties that may later find their tests broken eg. when we
decide to move code around.
TrivialFix
Change-Id: I7bd036ac3df7e7f4c678356d0a793e7d38599dda
We need to be able to re-use wait_until_true in tempest scenario tests.
There is tempest bug https://bugs.launchpad.net/tempest/+bug/1592345
that prevents us to do so.
Also wait_until_true is not linux specific so it makes more sense to
have it in common package.
Change-Id: Ib8b0e51dbd9edaa58391774d428a737836dfdf77
This reverts commit b1cdba1696
Original patch was reverted because it broke neutron plugin's
backward compatibility and needed more work.
This patch fixes that problems:
1) original behaviour of add_agent_status_check,
start_periodic_l3_agent_status_check and
start_periodic_dhcp_agent_status_check methods is deprecated but kept
for using in third part plugins for backward compatibility
2) new add_agent_status_check_worker, add_periodic_l3_agent_status_check
and add_periodic_dhcp_agent_status_check method are implemented
instead and are used for implementing plugins in neutron codebase
Closes-Bug: #1569404
Change-Id: I3a32a95489831f0d862930384309eefdc881d8f6
I think this needs a little more thought. This broke OVN at least and
I don't think that's good. We need to figure out how to be compatible
with existing plugins, even out of tree ones.
This reverts commit 1cafff0871.
Change-Id: Ie087fb11213cc85911483c2d32c463fa9c973e54
Forking a process when multiple threads are running is an unsafe
operation and could cause a lot of problems because only current
thread will continue working in child thread. Any locked by other
thread resource will remain locked forever.
We faced with this problem during oslo.messaging development and
added workaround to hide this problem:
https://review.openstack.org/#/c/274255/
I tried to fix this problem in oslo.service:
https://review.openstack.org/#/c/270832/
but oslo folks said that this fix is ugly and it is wrong way to add
workarounds to common libraries because projects use them incorrectly.
I think that is fair.
So this patch fixes incorrect usage of oslo libraries. In this patch
I extended functionality of NeutronWorker and add there
`worker_process_count` parameter which determines how many processes
should be spawned for this worker. If `worker_process_count` = 0 - don't
create process and spawn thread in scope of current process for worker
Then I moved all background tasks to workers and return them by
`get_workers` method. start_plugin_workers collects plugin's workers
using `get_workers` method and starts in ProcessLauncher first workers
with `worker_process_count` > 0 and only after this starts threaded
workers by simple Launcher
Closes-bug: #1569404
Change-Id: I0544f1d47ae53d572adda872847a56fa0b202d2e
Reactor code:
* Reuse oslo_utils.encodeutils.to_utf8() instead of existing
isinstance(text, six.text_type) test
* Replace jsonutils.dumps(obj).encode('utf-8') with
jsonutils.dump_as_bytes(obj).
* Other minor bytes/Unicode changes
Change-Id: I03b8eff0fd70ab65ac66d6f3221e8ced0a56db17
* Launching services with 0 workers makes no sense and would soon
be prohibited in oslo.service. Passing workers=1 means that a
service will be run in a current process.
* Extract the dummy _check_active method to the base class.
Change-Id: Ib687bf1b321845098e0564ada46e68a7850b01c6
By default spawn one additional rpc worker to process
state report queue.
State report queue will also be processed by regular
rpc workers, but in case these workers are busy with
processing heavy requests, state reports queue will
automatically be consumed by dedicated rpc workers.
This change applies to ML2 plugin only.
Other plugins should implement start_rpc_state_reports_listener
to enable additional rpc workers.
Change-Id: I5f8df6a478f7c82382049274b34b07109eeafbdb
Closes-Bug: #1505217
Change I18a11283925369bc918002477774f196010a1bc3 fixed the test for
oslo.service >= 0.10.0, but it also broke it for older versions of
oslo.service. Since the library has minimal version of >= 0.7.0 in
requirements.txt, test should pass for those versions too.
Now, instead of validating that either reset() or restart() of workers
are triggered on SIGHUP, just validate that .start() is triggered the
expected number of times (either way, no matter how oslo.service decide
to clean up the children, they exit and then are respawned).
Change-Id: I41f9d3af780b3178b075bc1e7084f417a2bd1378
Closes-Bug: #1505645
Now oslo.service 0.10.0 no longer sends SIGHUP to parent and
children services.
This was a chance introduced by 286a6ea, and since it invalidated
the very logic under test, this must be revised.
Change-Id: I18a11283925369bc918002477774f196010a1bc3
Closes-bug: #1505438
There are several cases where plugin initialization should be
handled after neutron-server forks API/RPC workers. For example,
starting a client connection to an SDN controller before forking
copies the fd of the socket to the child process, but then you have
multiple processes trying to read/write the same socket connection.
It is also useful for a plugin to be able to do something in only
one process, regardless of how many workers are forked. One example
would be handling syncing from an external system to the neutron
database.
This patch does 3 things:
1) Treats rpc_workers=0 as = 1. This simplifies the code for
handling notification that forking has completed. In the
existing code, calling the notification in the Worker object's
start() method would happen twice in the case where both api
and rpc workers were 0, despite there being only one process.
An earlier patch already changed the default api_workers to be
the number of processors.
2) Adds notification of forking via the callbacks mechanism.
Plugins can subscribe to resources.PROCESS, event.AFTER_CREATE
and do any post-fork initialization that needs to be done for
every spawned process.
3) Adds core/service plugin calls to get_workers() which defaults
to returning (). Plugins that need additional processes to spawn
should just return an iterable of NeutronWorkers that will be
spawned in their own process.
DocImpact
Closes-Bug: #1463129
Change-Id: Ib99954678c2b4f32f486b537979d446aafbea07b
All launchers implemented in common.service require each service to
implement reset method because it is called in case a process
receives a SIGHUP.
This change adds the reset method to neutron.service.RpcWorker and
neutron.wsgi.WorkerService which are used to wrap rpc and api
workers correspondingly.
Now neutron-server running in multiprocess mode (api_workers > 0 and
rpc_workers > 0) and metadata agent don't die on receiving SIGHUP and support
reloading policy_path and logging options in config.
Note that reset is called only in case a service is running in daemon mode.
Other changes made in the scope of this patch that need to be mentioned:
* Don't empty self._servers list in RpcWorker's stop method
When a service is restarted all services are gracefully shutdowned,
resetted and started again (see openstack.common.service code).
As graceful shutdown implies calling service.stop() and then
service.wait() we don't want to clean self._servers list because
it would be impossible to wait for them to stop processing
requests and cleaning up their resources.
Otherwise, this would lead to problems with rpc after starting
the rpc server again.
* Create a duplicate socket each time WorkerService starts
When api worker is stopped it kills the eventlet wsgi server
which internally closes the wsgi server socket object. This server
socket object becomes not usable which leads to "Bad file
descriptor" errors on service restart.
Added functional and unit tests.
DocImpact
Partial-Bug: #1276694
Change-Id: I75b00946b7cae891c6eb192e853118e7d49e4a24
Rodolfo Alonso Hernandez