Extend OpenVSwitch modules (closes #58986).

This adds the new openvswitch_db state module. It also  adds the new
functions bridge_to_parent, bridge_to_vlan, db_get, and db_set to the
openvswitch execution module.

Besides, it adds two new optional parameters parent and vlan to the
openvswitch_bridge.present state module function and the
openvswitch.bridge_create execution module function.

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.

doc/ref/states/all/index.rst

@@ -229,6 +229,7 @@ state modules
     nxos_upgrade
     openstack_config
     openvswitch_bridge
+    openvswitch_db
     openvswitch_port
     opsgenie
     pagerduty

doc/ref/states/all/salt.states.openvswitch_db.rst

@@ -0,0 +1,5 @@
+salt.states.openvswitch_db module
+=================================
+
+.. automodule:: salt.states.openvswitch_db
+    :members:

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

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:
+            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:
+        if current_parent == parent and current_vlan == vlan:
             ret["result"] = True
             ret["comment"] = comment_bridge_exists
         else:
-        bridge_create = __salt__["openvswitch.bridge_create"](name)
+            ret["result"] = False
+            ret["comment"] = comment_bridge_mismatch
+    else:
+        bridge_create = __salt__["openvswitch.bridge_create"](
+            name, parent=parent, vlan=vlan
+        )
         if bridge_create:
             ret["result"] = True
             ret["comment"] = comment_bridge_created

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

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"']
+        )

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."}
+        }

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"}
+        }