diff --git a/changelog/58986.added b/changelog/58986.added new file mode 100644 index 00000000000..ba03c9324b7 --- /dev/null +++ b/changelog/58986.added @@ -0,0 +1,5 @@ +Added `openvswitch_db` state module and functions `bridge_to_parent`, +`bridge_to_vlan`, `db_get`, and `db_set` to the `openvswitch` execution module. +Also added optional `parent` and `vlan` parameters to the +`openvswitch_bridge.present` state module function and the +`openvswitch.bridge_create` execution module function. diff --git a/doc/ref/states/all/index.rst b/doc/ref/states/all/index.rst index 7a70019c225..89bc3794f2a 100644 --- a/doc/ref/states/all/index.rst +++ b/doc/ref/states/all/index.rst @@ -229,6 +229,7 @@ state modules nxos_upgrade openstack_config openvswitch_bridge + openvswitch_db openvswitch_port opsgenie pagerduty diff --git a/doc/ref/states/all/salt.states.openvswitch_db.rst b/doc/ref/states/all/salt.states.openvswitch_db.rst new file mode 100644 index 00000000000..f6ca8b2bdce --- /dev/null +++ b/doc/ref/states/all/salt.states.openvswitch_db.rst @@ -0,0 +1,5 @@ +salt.states.openvswitch_db module +================================= + +.. automodule:: salt.states.openvswitch_db + :members: diff --git a/salt/modules/openvswitch.py b/salt/modules/openvswitch.py index 57995dafc20..eb34cb82eb0 100644 --- a/salt/modules/openvswitch.py +++ b/salt/modules/openvswitch.py @@ -9,6 +9,8 @@ Suitable for setting up Openstack Neutron. import logging import salt.utils.path +from salt.exceptions import ArgumentValueError, CommandExecutionError +from salt.utils import json log = logging.getLogger(__name__) @@ -89,6 +91,53 @@ def _stdout_list_split(retcode, stdout="", splitstring="\n"): return False +def _convert_json(obj): + """ + Converts from the JSON output provided by ovs-vsctl into a usable Python + object tree. In particular, sets and maps are converted from lists to + actual sets or maps. + + Args: + obj: Object that shall be recursively converted. + + Returns: + Converted version of object. + """ + if isinstance(obj, dict): + return {_convert_json(key): _convert_json(val) for (key, val) in obj.items()} + elif isinstance(obj, list) and len(obj) == 2: + first = obj[0] + second = obj[1] + if first == "set" and isinstance(second, list): + return [_convert_json(elem) for elem in second] + elif first == "map" and isinstance(second, list): + for elem in second: + if not isinstance(elem, list) or len(elem) != 2: + return obj + return {elem[0]: _convert_json(elem[1]) for elem in second} + else: + return obj + elif isinstance(obj, list): + return [_convert_json(elem) for elem in obj] + else: + return obj + + +def _stdout_parse_json(stdout): + """ + Parses JSON output from ovs-vsctl and returns the corresponding object + tree. + + Args: + stdout: Output that shall be parsed. + + Returns: + Object represented by the output. + """ + obj = json.loads(stdout) + return _convert_json(obj) + + def bridge_list(): """ Lists all existing real and fake bridges. @@ -132,13 +181,23 @@ def bridge_exists(br): return _retcode_to_bool(retcode) -def bridge_create(br, may_exist=True): +def bridge_create(br, may_exist=True, parent=None, vlan=None): """ Creates a new bridge. Args: - br: A string - bridge name - may_exist: Bool, if False - attempting to create a bridge that exists returns False. + br : string + bridge name + may_exist : bool + if False - attempting to create a bridge that exists returns False. + parent : string + name of the parent bridge (if the bridge shall be created as a fake + bridge). If specified, vlan must also be specified. + .. versionadded:: 3006 + vlan : int + VLAN ID of the bridge (if the bridge shall be created as a fake + bridge). If specified, parent must also be specified. + .. versionadded:: 3006 Returns: True on success, else False. @@ -152,7 +211,15 @@ def bridge_create(br, may_exist=True): salt '*' openvswitch.bridge_create br0 """ param_may_exist = _param_may_exist(may_exist) - cmd = "ovs-vsctl {1}add-br {0}".format(br, param_may_exist) + if parent is not None and vlan is None: + raise ArgumentValueError("If parent is specified, vlan must also be specified.") + if vlan is not None and parent is None: + raise ArgumentValueError("If vlan is specified, parent must also be specified.") + param_parent = "" if parent is None else " {}".format(parent) + param_vlan = "" if vlan is None else " {}".format(vlan) + cmd = "ovs-vsctl {1}add-br {0}{2}{3}".format( + br, param_may_exist, param_parent, param_vlan + ) result = __salt__["cmd.run_all"](cmd) return _retcode_to_bool(result["retcode"]) @@ -183,6 +250,61 @@ def bridge_delete(br, if_exists=True): return _retcode_to_bool(retcode) +def bridge_to_parent(br): + """ + .. versionadded:: 3006 + + Returns the parent bridge of a bridge. + + Args: + br : string + bridge name + + Returns: + Name of the parent bridge. This is the same as the bridge name if the + bridge is not a fake bridge. If the bridge does not exist, False is + returned. + + CLI Example: + + .. code-block:: bash + + salt '*' openvswitch.bridge_to_parent br0 + """ + cmd = "ovs-vsctl br-to-parent {}".format(br) + result = __salt__["cmd.run_all"](cmd) + if result["retcode"] != 0: + return False + return result["stdout"].strip() + + +def bridge_to_vlan(br): + """ + .. versionadded:: 3006 + + Returns the VLAN ID of a bridge. + + Args: + br : string + bridge name + + Returns: + VLAN ID of the bridge. The VLAN ID is 0 if the bridge is not a fake + bridge. If the bridge does not exist, False is returned. + + CLI Example: + + .. code-block:: bash + + salt '*' openvswitch.bridge_to_parent br0 + """ + cmd = "ovs-vsctl br-to-vlan {}".format(br) + result = __salt__["cmd.run_all"](cmd) + if result["retcode"] != 0: + return False + return int(result["stdout"]) + + def port_add(br, port, may_exist=False, internal=False): """ Creates on bridge a new port named port. @@ -473,3 +595,80 @@ def port_create_vxlan(br, port, id, remote, dst_port=None): ) result = __salt__["cmd.run_all"](cmd) return _retcode_to_bool(result["retcode"]) + + +def db_get(table, record, column, if_exists=False): + """ + .. versionadded:: 3006 + + Gets a column's value for a specific record. + + Args: + table : string + name of the database table + record : string + identifier of the record + column : string + name of the column + if_exists : boolean + if True, it is not an error if the record does not exist. + + Returns: + The column's value. + + CLI Example: + + .. code-block:: bash + + salt '*' openvswitch.db_get Port br0 vlan_mode + """ + cmd = ["ovs-vsctl", "--format=json", "--columns={}".format(column)] + if if_exists: + cmd += ["--if-exists"] + cmd += ["list", table, record] + result = __salt__["cmd.run_all"](cmd) + if result["retcode"] != 0: + raise CommandExecutionError(result["stderr"]) + output = _stdout_parse_json(result["stdout"]) + if output["data"] and output["data"][0]: + return output["data"][0][0] + else: + return None + + +def db_set(table, record, column, value, if_exists=False): + """ + .. versionadded:: 3006 + + Sets a column's value for a specific record. + + Args: + table : string + name of the database table + record : string + identifier of the record + column : string + name of the column + value : string + the value to be set + if_exists : boolean + if True, it is not an error if the record does not exist. + + Returns: + None on success and an error message on failure. + + CLI Example: + + .. code-block:: bash + + salt '*' openvswitch.db_set Interface br0 mac 02:03:04:05:06:07 + """ + cmd = ["ovs-vsctl"] + if if_exists: + cmd += ["--if-exists"] + cmd += ["set", table, record, "{}={}".format(column, json.dumps(value))] + result = __salt__["cmd.run_all"](cmd) + if result["retcode"] != 0: + return result["stderr"] + else: + return None diff --git a/salt/states/openvswitch_bridge.py b/salt/states/openvswitch_bridge.py index 654a1cbc1c6..4ee3b3ef880 100644 --- a/salt/states/openvswitch_bridge.py +++ b/salt/states/openvswitch_bridge.py @@ -12,12 +12,21 @@ def __virtual__(): return (False, "openvswitch module could not be loaded") -def present(name): +def present(name, parent=None, vlan=None): """ Ensures that the named bridge exists, eventually creates it. Args: - name: The name of the bridge. + name : string + name of the bridge + parent : string + name of the parent bridge (if the bridge shall be created as a fake + bridge). If specified, vlan must also be specified. + .. versionadded:: 3006 + vlan: int + VLAN ID of the bridge (if the bridge shall be created as a fake + bridge). If specified, parent must also be specified. + .. versionadded:: 3006 """ ret = {"name": name, "changes": {}, "result": False, "comment": ""} @@ -26,6 +35,9 @@ def present(name): comment_bridge_created = "Bridge {} created.".format(name) comment_bridge_notcreated = "Unable to create bridge: {}.".format(name) comment_bridge_exists = "Bridge {} already exists.".format(name) + comment_bridge_mismatch = ( + "Bridge {} already exists, but has a different" " parent or VLAN ID." + ).format(name) changes_bridge_created = { name: { "old": "Bridge {} does not exist.".format(name), @@ -34,12 +46,23 @@ def present(name): } bridge_exists = __salt__["openvswitch.bridge_exists"](name) + if bridge_exists: + current_parent = __salt__["openvswitch.bridge_to_parent"](name) + if current_parent == name: + current_parent = None + current_vlan = __salt__["openvswitch.bridge_to_vlan"](name) + if current_vlan == 0: + current_vlan = None # Dry run, test=true mode if __opts__["test"]: if bridge_exists: - ret["result"] = True - ret["comment"] = comment_bridge_exists + if current_parent == parent and current_vlan == vlan: + ret["result"] = True + ret["comment"] = comment_bridge_exists + else: + ret["result"] = False + ret["comment"] = comment_bridge_mismatch else: ret["result"] = None ret["comment"] = comment_bridge_created @@ -47,10 +70,16 @@ def present(name): return ret if bridge_exists: - ret["result"] = True - ret["comment"] = comment_bridge_exists + if current_parent == parent and current_vlan == vlan: + ret["result"] = True + ret["comment"] = comment_bridge_exists + else: + ret["result"] = False + ret["comment"] = comment_bridge_mismatch else: - bridge_create = __salt__["openvswitch.bridge_create"](name) + bridge_create = __salt__["openvswitch.bridge_create"]( + name, parent=parent, vlan=vlan + ) if bridge_create: ret["result"] = True ret["comment"] = comment_bridge_created diff --git a/salt/states/openvswitch_db.py b/salt/states/openvswitch_db.py new file mode 100644 index 00000000000..c874ca2b53f --- /dev/null +++ b/salt/states/openvswitch_db.py @@ -0,0 +1,71 @@ +""" +Management of Open vSwitch database records. + +.. versionadded:: 3006 +""" + + +def __virtual__(): + """ + Only make these states available if Open vSwitch module is available. + """ + return "openvswitch.db_get" in __salt__ + + +def managed(name, table, data, record=None): + """ + Ensures that the specified columns of the named record have the specified + values. + + Args: + name : string + name of the record + table : string + name of the table to which the record belongs. + data : dict + dictionary containing a mapping from column names to the desired + values. Columns that exist, but are not specified in this + dictionary are not touched. + record : string + name of the record (optional). Replaces name if specified. + """ + ret = {"name": name, "changes": {}, "result": False, "comment": ""} + if record is None: + record = name + current_data = { + column: __salt__["openvswitch.db_get"](table, record, column, True) + for column in data + } + + # Comment and change messages + comment_changes = "Columns have been updated." + comment_no_changes = "All columns are already up to date." + comment_error = "Error while updating column {0}: {1}" + + # Dry run, test=true mode + if __opts__["test"]: + for column in data: + if data[column] != current_data[column]: + ret["changes"][column] = { + "old": current_data[column], + "new": data[column], + } + if ret["changes"]: + ret["result"] = None + ret["comment"] = comment_changes + else: + ret["result"] = True + ret["comment"] = comment_no_changes + return ret + + for column in data: + if data[column] != current_data[column]: + result = __salt__["openvswitch.db_set"](table, record, column, data[column]) + if result is not None: + ret["comment"] = comment_error.format(column, result) + ret["result"] = False + return ret + ret["changes"][column] = {"old": current_data[column], "new": data[column]} + ret["result"] = True + ret["comment"] = comment_no_changes + return ret diff --git a/tests/pytests/unit/modules/test_openvswitch.py b/tests/pytests/unit/modules/test_openvswitch.py new file mode 100644 index 00000000000..70fc42619a1 --- /dev/null +++ b/tests/pytests/unit/modules/test_openvswitch.py @@ -0,0 +1,134 @@ +""" +Test cases for salt.modules.openvswitch. +""" + +import pytest + +import salt.modules.openvswitch as openvswitch +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def configure_loader_modules(): + return {openvswitch: {}} + + +def test_bridge_create_may_not_exist(): + """ + Test bridge_create function. + + This tests the case where neither a parent nor the may-exists flag are + specified. + """ + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.bridge_create("br0", False) + assert ret is True + mock.assert_called_with("ovs-vsctl add-br br0") + + +def test_bridge_create_may_exist(): + """ + Test bridge_create function. + + This tests the case where no parent but the may-exists flag is specified. + """ + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.bridge_create("br1", True) + assert ret is True + mock.assert_called_with("ovs-vsctl --may-exist add-br br1") + + +def test_bridge_create_with_parent_may_exist(): + """ + Test bridge_create function. + + This tests the case where a parent is specified but the may-exists flag is + false. + """ + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.bridge_create("br2", False, "br0", 42) + assert ret is True + mock.assert_called_with("ovs-vsctl add-br br2 br0 42") + + +def test_bridge_to_parent(): + """ + Test bridge_to_parent function. + """ + mock = MagicMock(return_value={"retcode": 0, "stdout": "br0\n"}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.bridge_to_parent("br1") + assert ret == "br0" + mock.assert_called_with("ovs-vsctl br-to-parent br1") + + +def test_bridge_to_vlan(): + """ + Test bridge_to_vlan function. + """ + mock = MagicMock(return_value={"retcode": 0, "stdout": "42\n"}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.bridge_to_vlan("br0") + assert ret == 42 + mock.assert_called_with("ovs-vsctl br-to-vlan br0") + + +def test_db_get(): + """ + Test db_get function. + """ + mock = MagicMock( + return_value={ + "retcode": 0, + "stdout": '{"data":[["01:02:03:04:05:06"]],' '"headings":["mac"]}', + } + ) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + ret = openvswitch.db_get("Interface", "br0", "mac") + assert ret == "01:02:03:04:05:06" + mock.assert_called_with( + [ + "ovs-vsctl", + "--format=json", + "--columns=mac", + "list", + "Interface", + "br0", + ] + ) + + +def test_db_set(): + """ + Test db_set function. + """ + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict( + openvswitch.__salt__, + {"cmd.run_all": mock}, + ): + openvswitch.db_set("Interface", "br0", "mac", "01:02:03:04:05:06") + mock.assert_called_with( + ["ovs-vsctl", "set", "Interface", "br0", 'mac="01:02:03:04:05:06"'] + ) diff --git a/tests/pytests/unit/states/test_openvswitch_bridge.py b/tests/pytests/unit/states/test_openvswitch_bridge.py new file mode 100644 index 00000000000..4259daea6ce --- /dev/null +++ b/tests/pytests/unit/states/test_openvswitch_bridge.py @@ -0,0 +1,135 @@ +""" +Test cases for salt.states.openvswitch_bridge. +""" + +import pytest + +import salt.states.openvswitch_bridge as openvswitch_bridge +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def configure_loader_modules(): + return {openvswitch_bridge: {"__opts__": {"test": False}}} + + +def test_present_no_parent_existing_no_parent(): + """ + Test present function, not specifying a parent. + + This tests the case where the bridge already exists and has no parent. + """ + create_mock = MagicMock() + exists_mock = MagicMock(return_value=True) + to_parent_mock = MagicMock(return_value="br0") + to_vlan_mock = MagicMock(return_value=0) + with patch.dict( + openvswitch_bridge.__salt__, + { + "openvswitch.bridge_create": create_mock, + "openvswitch.bridge_exists": exists_mock, + "openvswitch.bridge_to_parent": to_parent_mock, + "openvswitch.bridge_to_vlan": to_vlan_mock, + }, + ): + ret = openvswitch_bridge.present(name="br0") + create_mock.assert_not_called() + assert ret["result"] is True + + +def test_present_no_parent_existing_with_parent(): + """ + Test present function, not specifying a parent. + + This tests the case where the bridge already exists and has a parent. + """ + create_mock = MagicMock() + exists_mock = MagicMock(return_value=True) + to_parent_mock = MagicMock(return_value="br0") + to_vlan_mock = MagicMock(return_value=42) + with patch.dict( + openvswitch_bridge.__salt__, + { + "openvswitch.bridge_create": create_mock, + "openvswitch.bridge_exists": exists_mock, + "openvswitch.bridge_to_parent": to_parent_mock, + "openvswitch.bridge_to_vlan": to_vlan_mock, + }, + ): + # Bridge exists, but parent and VLAN do not match, so we expect a + # result of False. + ret = openvswitch_bridge.present(name="br1") + create_mock.assert_not_called() + assert ret["result"] is False + + +def test_present_no_parent_not_existing(): + """ + Test present function, not specifying a parent. + + This tests the case where the bridge does not exist yet. + """ + create_mock = MagicMock(return_value=True) + exists_mock = MagicMock(return_value=False) + with patch.dict( + openvswitch_bridge.__salt__, + { + "openvswitch.bridge_create": create_mock, + "openvswitch.bridge_exists": exists_mock, + }, + ): + ret = openvswitch_bridge.present(name="br0") + create_mock.assert_called_with("br0", parent=None, vlan=None) + assert ret["result"] is True + assert ret["changes"] == { + "br0": {"new": "Bridge br0 created", "old": "Bridge br0 does not exist."} + } + + +def test_present_with_parent_existing_with_parent(): + """ + Test present function, specifying a parent. + + This tests the case where the bridge already exists and has a parent that + matches the specified one. + """ + create_mock = MagicMock() + exists_mock = MagicMock(return_value=True) + to_parent_mock = MagicMock(return_value="br0") + to_vlan_mock = MagicMock(return_value=42) + with patch.dict( + openvswitch_bridge.__salt__, + { + "openvswitch.bridge_create": create_mock, + "openvswitch.bridge_exists": exists_mock, + "openvswitch.bridge_to_parent": to_parent_mock, + "openvswitch.bridge_to_vlan": to_vlan_mock, + }, + ): + # Bridge exists and parent VLAN matches + ret = openvswitch_bridge.present(name="br1", parent="br0", vlan=42) + create_mock.assert_not_called() + assert ret["result"] is True + + +def test_present_with_parent_not_existing(): + """ + Test present function, specifying a parent. + + This tests the case where the bridge does not exist yet. + """ + create_mock = MagicMock(return_value=True) + exists_mock = MagicMock(return_value=False) + with patch.dict( + openvswitch_bridge.__salt__, + { + "openvswitch.bridge_create": create_mock, + "openvswitch.bridge_exists": exists_mock, + }, + ): + ret = openvswitch_bridge.present(name="br1", parent="br0", vlan=42) + create_mock.assert_called_with("br1", parent="br0", vlan=42) + assert ret["result"] is True + assert ret["changes"] == { + "br1": {"new": "Bridge br1 created", "old": "Bridge br1 does not exist."} + } diff --git a/tests/pytests/unit/states/test_openvswitch_db.py b/tests/pytests/unit/states/test_openvswitch_db.py new file mode 100644 index 00000000000..529666dcd55 --- /dev/null +++ b/tests/pytests/unit/states/test_openvswitch_db.py @@ -0,0 +1,80 @@ +""" +Test cases for salt.states.openvswitch_db. +""" + +import pytest + +import salt.states.openvswitch_db as openvswitch_db +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def configure_loader_modules(): + return {openvswitch_db: {"__opts__": {"test": False}}} + + +def test_managed_different_entry_present(): + """ + Test managed function. + + This tests the case where there already is an entry, but it does not match. + """ + get_mock = MagicMock(return_value="01:02:03:04:05:06") + set_mock = MagicMock(return_value=None) + with patch.dict( + openvswitch_db.__salt__, + {"openvswitch.db_get": get_mock, "openvswitch.db_set": set_mock}, + ): + ret = openvswitch_db.managed( + name="br0", table="Interface", data={"mac": "01:02:03:04:05:07"} + ) + get_mock.assert_called_with("Interface", "br0", "mac", True) + set_mock.assert_called_with("Interface", "br0", "mac", "01:02:03:04:05:07") + assert ret["result"] is True + assert ret["changes"] == { + "mac": {"old": "01:02:03:04:05:06", "new": "01:02:03:04:05:07"} + } + + +def test_managed_matching_entry_present(): + """ + Test managed function. + + This tests the case where there already is a matching entry. + """ + get_mock = MagicMock(return_value="01:02:03:04:05:06") + set_mock = MagicMock(return_value=None) + with patch.dict( + openvswitch_db.__salt__, + {"openvswitch.db_get": get_mock, "openvswitch.db_set": set_mock}, + ): + ret = openvswitch_db.managed( + name="br0", table="Interface", data={"mac": "01:02:03:04:05:06"} + ) + get_mock.assert_called_with("Interface", "br0", "mac", True) + set_mock.assert_not_called() + assert ret["result"] is True + assert "changes" not in ret or not ret["changes"] + + +def test_managed_no_entry_present(): + """ + Test managed function. + + This tests the case where there is no entry yet. + """ + get_mock = MagicMock(return_value="01:02:03:04:05:06") + set_mock = MagicMock(return_value=None) + with patch.dict( + openvswitch_db.__salt__, + {"openvswitch.db_get": get_mock, "openvswitch.db_set": set_mock}, + ): + ret = openvswitch_db.managed( + name="br0", table="Interface", data={"mac": "01:02:03:04:05:07"} + ) + get_mock.assert_called_with("Interface", "br0", "mac", True) + set_mock.assert_called_with("Interface", "br0", "mac", "01:02:03:04:05:07") + assert ret["result"] is True + assert ret["changes"] == { + "mac": {"old": "01:02:03:04:05:06", "new": "01:02:03:04:05:07"} + }