nm provider: Refactor the down action of network connection

When deactivating a profile in libNM, we should:

 * Check `NM.ActionConnection` existence
 * Check `NM.ActionConnection.props.state` not DEACTIVATED
 * Use signal `state-changed` of `NM.ActionConnection`.
 * Only invoke `NM.Client.deactivate_connection_async()` if not
   in DEACTIVATING state.
 * Ignore `NM.ManagerError.CONNECTIONNOTACTIVE` error.

This patch also introduced a new class `NetworkManagerProvider`
in `module_utils/network_lsr/nm`:

 * Independent from Ansible but need to use absolute import due to
   limitation of ansible 2.8.
 * Provide sync function wrapping async calls of libNM.
 * Use stable logging method of python.
 * Only load this module when provider is nm.

This patch also changed how logging is handling in
`Cmd_nm.run_action_down()` as initial step on isolate ansible log
mechanism from provider module.

By moving provider codes to `module_utils` folder, we can eventually
simplify the bloated `library/network_connections.py`.

Signed-off-by: Gris Ge <fge@redhat.com>
This commit is contained in:
Gris Ge 2020-11-12 21:42:01 +08:00
parent 6812aab616
commit c4643e56bb
7 changed files with 319 additions and 101 deletions

View file

@ -11,6 +11,7 @@ import socket
import subprocess
import time
import traceback
import logging
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.basic import AnsibleModule
@ -66,6 +67,17 @@ class LogLevel:
INFO = "info"
DEBUG = "debug"
_LOGGING_LEVEL_MAP = {
logging.DEBUG: DEBUG,
logging.INFO: INFO,
logging.WARN: WARN,
logging.ERROR: ERROR,
}
@staticmethod
def from_logging_level(logging_level):
return LogLevel._LOGGING_LEVEL_MAP.get(logging_level, LogLevel.ERROR)
@staticmethod
def fmt(level):
return "<%-6s" % (str(level) + ">")
@ -1387,61 +1399,6 @@ class NMUtil:
if failure_reason:
raise MyError("connection not activated: %s" % (failure_reason))
def active_connection_deactivate(self, ac, timeout=10, wait_time=None):
def deactivate_cb(client, result, cb_args):
success = False
try:
success = client.deactivate_connection_finish(result)
except Exception as e:
if Util.error_is_cancelled(e):
return
cb_args["error"] = str(e)
cb_args["success"] = success
Util.GMainLoop().quit()
cancellable = Util.create_cancellable()
cb_args = {}
self.nmclient.deactivate_connection_async(
ac, cancellable, deactivate_cb, cb_args
)
if not Util.GMainLoop_run(timeout):
cancellable.cancel()
raise MyError("failure to deactivate connection: %s" % (timeout))
if not cb_args.get("success", False):
raise MyError(
"failure to deactivate connection: %s"
% (cb_args.get("error", "unknown error"))
)
self.active_connection_deactivate_wait(ac, wait_time)
return True
def active_connection_deactivate_wait(self, ac, wait_time):
if not wait_time:
return
NM = Util.NM()
def check_deactivated(ac):
return ac.get_state() >= NM.ActiveConnectionState.DEACTIVATED
if not check_deactivated(ac):
def check_deactivated_cb():
if check_deactivated(ac):
Util.GMainLoop().quit()
ac_id = ac.connect(
"notify::state", lambda source, pspec: check_deactivated_cb()
)
try:
if not Util.GMainLoop_run(wait_time):
raise MyError("connection not fully deactivated after timeout")
finally:
ac.handler_disconnect(ac_id)
def reapply(self, device, connection=None):
version_id = 0
flags = 0
@ -1629,6 +1586,21 @@ class RunEnvironmentAnsible(RunEnvironment):
###############################################################################
class NmLogHandler(logging.Handler):
def __init__(self, log_func, idx):
self._log = log_func
self._idx = idx
super(NmLogHandler, self).__init__()
def filter(self, record):
return True
def emit(self, record):
self._log(
self._idx, LogLevel.from_logging_level(record.levelno), record.getMessage()
)
class Cmd(object):
def __init__(
self,
@ -1954,6 +1926,14 @@ class Cmd_nm(Cmd):
self._nmutil = None
self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_NM
self._checkpoint = None
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr.nm import ( # noqa: E501
NetworkManagerProvider,
)
# pylint: enable=import-error, no-name-in-module
self._nm_provider = NetworkManagerProvider()
@property
def nmutil(self):
@ -2265,51 +2245,17 @@ class Cmd_nm(Cmd):
def run_action_down(self, idx):
connection = self.connections[idx]
cons = self.nmutil.connection_list(name=connection["name"])
changed = False
if cons:
seen = set()
while True:
ac = Util.first(
self.nmutil.active_connection_list(
connections=cons, black_list=seen
)
)
if ac is None:
break
seen.add(ac)
self.log_info(
idx, "down connection %s: %s" % (connection["name"], ac.get_path())
)
changed = True
self.connections_data_set_changed(idx)
if self.check_mode == CheckMode.REAL_RUN:
try:
self.nmutil.active_connection_deactivate(ac)
except MyError as e:
self.log_error(idx, "down connection failed: %s" % (e))
wait_time = connection["wait"]
if wait_time is None:
wait_time = 10
try:
self.nmutil.active_connection_deactivate_wait(ac, wait_time)
except MyError as e:
self.log_error(
idx, "down connection failed while waiting: %s" % (e)
)
cons = self.nmutil.connection_list(name=connection["name"])
if not changed:
message = "down connection %s failed: connection not found" % (
connection["name"]
)
if connection[PERSISTENT_STATE] == ABSENT_STATE:
self.log_info(idx, message)
else:
self.log_error(idx, message)
logger = logging.getLogger()
log_handler = NmLogHandler(self.log, idx)
logger.addHandler(log_handler)
timeout = connection["wait"]
if self._nm_provider.deactivate_connection(
connection["name"],
10 if timeout is None else timeout,
self.check_mode != CheckMode.REAL_RUN,
):
self.connections_data_set_changed(idx)
logger.removeHandler(log_handler)
###############################################################################

View file

@ -0,0 +1,9 @@
# Relative import is not support by ansible 2.8 yet
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr.nm.provider import ( # noqa:E501
NetworkManagerProvider,
)
# pylint: enable=import-error, no-name-in-module
NetworkManagerProvider

View file

@ -0,0 +1,125 @@
# SPDX-License-Identifier: BSD-3-Clause
# Handle NM.ActiveConnection
import logging
# Relative import is not support by ansible 2.8 yet
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr.nm.client import GLib # noqa:E501
from ansible.module_utils.network_lsr.nm.client import NM # noqa:E501
from ansible.module_utils.network_lsr.nm.client import get_mainloop # noqa:E501
from ansible.module_utils.network_lsr.nm.client import get_client # noqa:E501
from ansible.module_utils.network_lsr.nm.error import LsrNetworkNmError # noqa:E501
# pylint: enable=import-error, no-name-in-module
NM_AC_STATE_CHANGED_SIGNAL = "state-changed"
def deactivate_active_connection(nm_ac, timeout, check_mode):
if not nm_ac or nm_ac.props.state == NM.ActiveConnectionState.DEACTIVATED:
logging.info("Connection is not active, no need to deactivate")
return False
if not check_mode:
main_loop = get_mainloop(timeout)
logging.debug(
"Deactivating {id} with timeout {timeout}".format(
id=nm_ac.get_id(), timeout=timeout
)
)
user_data = main_loop
handler_id = nm_ac.connect(
NM_AC_STATE_CHANGED_SIGNAL, _nm_ac_state_change_callback, user_data
)
logging.debug(
"Registered {signal} on NM.ActiveConnection {id}".format(
signal=NM_AC_STATE_CHANGED_SIGNAL, id=nm_ac.get_id()
)
)
if nm_ac.props.state != NM.ActiveConnectionState.DEACTIVATING:
nm_client = get_client()
user_data = (main_loop, nm_ac, nm_ac.get_id(), handler_id)
nm_client.deactivate_connection_async(
nm_ac,
main_loop.cancellable,
_nm_ac_deactivate_call_back,
user_data,
)
logging.debug("Deactivating NM.ActiveConnection {0}".format(nm_ac.get_id()))
main_loop.run()
return True
def _nm_ac_state_change_callback(nm_ac, state, reason, user_data):
main_loop = user_data
if main_loop.is_cancelled:
return
logging.debug(
"Got NM.ActiveConnection state change: {id}: {state} {reason}".format(
id=nm_ac.get_id(), state=state, reason=reason
)
)
if nm_ac.props.state == NM.ActiveConnectionState.DEACTIVATED:
logging.debug("NM.ActiveConnection {0} is deactivated".format(nm_ac.get_id()))
main_loop.quit()
def _nm_ac_deactivate_call_back(nm_client, result, user_data):
main_loop, nm_ac, nm_ac_id, handler_id = user_data
logging.debug("NM.ActiveConnection deactivating callback")
if main_loop.is_cancelled:
if nm_ac:
nm_ac.handler_disconnect(handler_id)
return
try:
success = nm_client.deactivate_connection_finish(result)
except GLib.Error as e:
if e.matches(NM.ManagerError.quark(), NM.ManagerError.CONNECTIONNOTACTIVE):
logging.info(
"Connection is not active on {0}, no need to deactivate".format(
nm_ac_id
)
)
if nm_ac:
nm_ac.handler_disconnect(handler_id)
main_loop.quit()
return
else:
_deactivate_fail(
main_loop,
handler_id,
nm_ac,
"Failed to deactivate connection {id}, error={error}".format(
id=nm_ac_id, error=e
),
)
return
except Exception as e:
_deactivate_fail(
main_loop,
handler_id,
nm_ac,
"Failed to deactivate connection {id}, error={error}".format(
id=nm_ac_id, error=e
),
)
return
if not success:
_deactivate_fail(
main_loop,
handler_id,
nm_ac,
"Failed to deactivate connection {0}, error='None "
"returned from deactivate_connection_finish()'".format(nm_ac_id),
)
def _deactivate_fail(main_loop, handler_id, nm_ac, msg):
if nm_ac:
nm_ac.handler_disconnect(handler_id)
logging.error(msg)
main_loop.fail(LsrNetworkNmError(msg))

View file

@ -0,0 +1,86 @@
# SPDX-License-Identifier: BSD-3-Clause
import logging
# Relative import is not support by ansible 2.8 yet
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr.nm.error import LsrNetworkNmError # noqa:E501
import gi
gi.require_version("NM", "1.0")
# It is required to state the NM version before importing it
# But this break the flake8 rule: https://www.flake8rules.com/rules/E402.html
# Use NOQA: E402 to suppress it.
from gi.repository import NM # NOQA: E402
from gi.repository import GLib # NOQA: E402
from gi.repository import Gio # NOQA: E402
# pylint: enable=import-error, no-name-in-module
NM
GLib
Gio
def get_client():
return NM.Client.new()
class _NmMainLoop(object):
def __init__(self, timeout):
self._mainloop = GLib.MainLoop()
self._cancellable = Gio.Cancellable.new()
self._timeout = timeout
self._timeout_id = None
def run(self):
logging.debug("NM mainloop running")
user_data = None
self._timeout_id = GLib.timeout_add(
int(self._timeout * 1000),
self._timeout_call_back,
user_data,
)
logging.debug("Added timeout checker")
self._mainloop.run()
def _timeout_call_back(self, _user_data):
logging.error("Timeout")
self.fail(LsrNetworkNmError("Timeout"))
@property
def cancellable(self):
return self._cancellable
@property
def is_cancelled(self):
if self._cancellable:
return self._cancellable.is_cancelled()
return True
def _clean_up(self):
logging.debug("NM mainloop cleaning up")
if self._timeout_id:
logging.debug("Removing timeout checker")
GLib.source_remove(self._timeout_id)
self._timeout_id = None
if self._cancellable:
logging.debug("Canceling all pending tasks")
self._cancellable.cancel()
self._cancellable = None
self._mainloop = None
def quit(self):
logging.debug("NM mainloop quiting")
self._mainloop.quit()
self._clean_up()
def fail(self, exception):
self.quit()
raise exception
def get_mainloop(timeout):
return _NmMainLoop(timeout)

View file

@ -0,0 +1,5 @@
# SPDX-License-Identifier: BSD-3-Clause
class LsrNetworkNmError(Exception):
pass

View file

@ -0,0 +1,29 @@
# SPDX-License-Identifier: BSD-3-Clause
import logging
# Relative import is not support by ansible 2.8 yet
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr.nm.active_connection import ( # noqa:E501
deactivate_active_connection,
)
from ansible.module_utils.network_lsr.nm.client import get_client # noqa:E501
# pylint: enable=import-error, no-name-in-module
class NetworkManagerProvider:
def deactivate_connection(self, connection_name, timeout, check_mode):
"""
Return True if changed.
"""
nm_client = get_client()
changed = False
for nm_ac in nm_client.get_active_connections():
nm_profile = nm_ac.get_connection()
if nm_profile and nm_profile.get_id() == connection_name:
changed |= deactivate_active_connection(nm_ac, timeout, check_mode)
if not changed:
logging.info("No active connection for {0}".format(connection_name))
return changed

View file

@ -92,7 +92,25 @@ def _get_ip_addresses(interface):
return ip_address.decode("UTF-8")
def test_static_ip_with_ethernet(testnic1, provider):
@pytest.fixture
def network_lsr_nm_mock():
with mock.patch.object(
sys,
"path",
[parentdir, os.path.join(parentdir, "module_utils/network_lsr/nm")] + sys.path,
):
with mock.patch.dict(
"sys.modules",
{
"ansible": mock.Mock(),
"ansible.module_utils": __import__("module_utils"),
"ansible.module_utils.basic": mock.Mock(),
},
):
yield
def test_static_ip_with_ethernet(testnic1, provider, network_lsr_nm_mock):
ip_address = "192.0.2.127/24"
connections = [
{