Fix ovsdb module not being idempotent (#57735)

openvswitch_db was not parsing properly some arguments, which caused
some commands to be executed when they shouldn't. This commit fixesit
and adds unit testing for the usecase.

Closes-Bug: #55432
Closes-bug: #43858

lib/ansible/modules/network/ovs/openvswitch_db.py

@@ -157,7 +157,7 @@ def map_config_to_obj(module):
     if NON_EMPTY_MAP_RE.match(col_value):
         for kv in col_value[1:-1].split(', '):
             k, v = kv.split('=')
-            col_value_to_dict[k.strip()] = v.strip()
+            col_value_to_dict[k.strip()] = v.strip('\"')
 
     obj = {
         'table': module.params['table'],
@@ -176,6 +176,8 @@ def map_config_to_obj(module):
 
 
 def map_params_to_obj(module):
+    if module.params['value'] in ['True', 'False']:
+        module.params['value'] = module.params['value'].lower()
     obj = {
         'table': module.params['table'],
         'record': module.params['record'],

test/integration/targets/openvswitch_db/tests/basic.yaml

@@ -21,7 +21,7 @@
 
 - assert:
     that:
-      - "result.changed == true"
+      - result is changed
 
 - name: Create bridge again (idempotent)
   openvswitch_db:
@@ -35,7 +35,50 @@
 
 - assert:
     that:
-      - "result.changed == false"
+      - result is not changed
+
+- name: Change key value with quotes
+  openvswitch_db:
+    table: open_vswitch
+    record: .
+    col: other_config
+    key: pmd-cpu-mask
+    value: "0xaaa00000000"
+  become: yes
+  register: result
+
+- assert:
+    that:
+      - result is changed
+
+- name: Change keyvalue with quotes(idempotent)
+  openvswitch_db:
+    table: open_vswitch
+    record: .
+    col: other_config
+    key: pmd-cpu-mask
+    value: "0xaaa00000000"
+  become: yes
+  register: result
+
+- assert:
+    that:
+      - result is not changed
+
+- name: Remove key value with quotes
+  openvswitch_db:
+    table: open_vswitch
+    record: .
+    col: other_config
+    key: pmd-cpu-mask
+    value: "0xaaa00000000"
+    state: absent
+  become: yes
+  register: result
+
+- assert:
+    that:
+      - result is changed
 
 - name: Change column value in a map
   openvswitch_db:
@@ -49,7 +92,7 @@
 
 - assert:
     that:
-      - "result.changed == true"
+      - result is changed
 
 - name: Change column value in a map again (idempotent)
   openvswitch_db:
@@ -63,33 +106,33 @@
 
 - assert:
     that:
-      - "result.changed == false"
+      - result is not changed
 
 - name: Change column value
   openvswitch_db:
     table: Bridge
     record: br-test
     col: stp_enable
-    value: 'true'
+    value: true
   become: yes
   register: result
 
 - assert:
     that:
-      - "result.changed == true"
+      - result is changed
 
 - name: Change column value again (idempotent)
   openvswitch_db:
     table: Bridge
     record: br-test
     col: stp_enable
-    value: 'true'
+    value: true
   become: yes
   register: result
 
 - assert:
     that:
-      - "result.changed == false"
+      - result is not changed
 
 - name: Try to set value on a map type without a key (negative)
   ignore_errors: true
@@ -103,7 +146,7 @@
 
 - assert:
     that:
-      - "result.failed == true"
+      - result is failed
 
 - name: Remove bridge
   openvswitch_db:
@@ -118,7 +161,7 @@
 
 - assert:
     that:
-      - "result.changed == true"
+      - result is changed
 
 - name: Remove bridge again (idempotent)
   openvswitch_db:
@@ -133,7 +176,7 @@
 
 - assert:
     that:
-      - "result.changed == false"
+      - result is not changed
 
 - name: Tear down test bridge
   command: ovs-vsctl del-br br-test

test/units/modules/network/ovs/fixtures/openvswitch_db_disable_in_band_true.cfg

@@ -13,7 +13,7 @@ mcast_snooping_enable: false
 mirrors             : []
 name                : test-br
 netflow             : []
-other_config        : {disable-in-band=True}
+other_config        : {disable-in-band=true}
 ports               : [2c9c1b35-a304-4dee-bb7a-092d656543c7]
 protocols           : []
 rstp_enable         : false

test/units/modules/network/ovs/test_openvswitch_db.py

@@ -20,11 +20,26 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-from units.compat.mock import patch
 from ansible.modules.network.ovs import openvswitch_db
+from units.compat.mock import patch, MagicMock
 from units.modules.utils import set_module_args
 from .ovs_module import TestOpenVSwitchModule, load_fixture
 
+import pytest
+
+
+@pytest.fixture
+def patched_openvswitch_db(monkeypatch):
+    mocked_ovs_db = MagicMock()
+    mocked_ovs_db.return_value = {'table': 'open_vswitch',
+                                  'record': '.',
+                                  'col': 'other_config',
+                                  'key': 'pmd-cpu-mask',
+                                  'value': '0xaaa00000000'}
+    monkeypatch.setattr(openvswitch_db, 'map_config_to_obj', mocked_ovs_db)
+    return openvswitch_db
+
+
 test_name_side_effect_matrix = {
     'test_openvswitch_db_absent_idempotent': [
         (0, 'openvswitch_db_disable_in_band_missing.cfg', None),
@@ -35,6 +50,8 @@ test_name_side_effect_matrix = {
     'test_openvswitch_db_present_idempotent': [
         (0, 'openvswitch_db_disable_in_band_true.cfg', None),
         (0, None, None)],
+    'test_openvswitch_db_present_idempotent_value': [
+        (0, None, None)],
     'test_openvswitch_db_present_adds_key': [
         (0, 'openvswitch_db_disable_in_band_missing.cfg', None),
         (0, None, None)],
@@ -86,62 +103,71 @@ class TestOpenVSwitchDBModule(TestOpenVSwitchModule):
         set_module_args(dict(state='absent',
                              table='Bridge', record='test-br',
                              col='other_config', key='disable-in-band',
-                             value='True'))
+                             value='true'))
         self.execute_module(test_name='test_openvswitch_db_absent_idempotent')
 
     def test_openvswitch_db_absent_removes_key(self):
         set_module_args(dict(state='absent',
                              table='Bridge', record='test-br',
                              col='other_config', key='disable-in-band',
-                             value='True'))
+                             value='true'))
         self.execute_module(
             changed=True,
             commands=['/usr/bin/ovs-vsctl -t 5 remove Bridge test-br other_config'
-                      ' disable-in-band=True'],
+                      ' disable-in-band=true'],
             test_name='test_openvswitch_db_absent_removes_key')
 
     def test_openvswitch_db_present_idempotent(self):
         set_module_args(dict(state='present',
                              table='Bridge', record='test-br',
                              col='other_config', key='disable-in-band',
-                             value='True'))
+                             value='true'))
         self.execute_module(test_name='test_openvswitch_db_present_idempotent')
 
+    @pytest.mark.usefixtures('patched_openvswitch_db')
+    def test_openvswitch_db_present_idempotent_value(self):
+        set_module_args({"col": "other_config",
+                         "key": "pmd-cpu-mask",
+                         "record": ".",
+                         "table": "open_vswitch",
+                         "value": "0xaaa00000000"})
+        self.execute_module(test_name='test_openvswitch_db_present_idempotent_value')
+
     def test_openvswitch_db_present_adds_key(self):
         set_module_args(dict(state='present',
                              table='Bridge', record='test-br',
                              col='other_config', key='disable-in-band',
-                             value='True'))
+                             value='true'))
         self.execute_module(
             changed=True,
             commands=['/usr/bin/ovs-vsctl -t 5 set Bridge test-br other_config'
-                      ':disable-in-band=True'],
+                      ':disable-in-band=true'],
             test_name='test_openvswitch_db_present_adds_key')
 
     def test_openvswitch_db_present_updates_key(self):
         set_module_args(dict(state='present',
                              table='Bridge', record='test-br',
                              col='other_config', key='disable-in-band',
-                             value='False'))
+                             value='false'))
         self.execute_module(
             changed=True,
             commands=['/usr/bin/ovs-vsctl -t 5 set Bridge test-br other_config'
-                      ':disable-in-band=False'],
+                      ':disable-in-band=false'],
             test_name='test_openvswitch_db_present_updates_key')
 
     def test_openvswitch_db_present_missing_key_on_map(self):
         set_module_args(dict(state='present',
                              table='Bridge', record='test-br',
                              col='other_config',
-                             value='False'))
+                             value='false'))
         self.execute_module(
             failed=True,
-            test_name='test_openvswitch_db_present_idempotent')
+            test_name='test_openvswitch_db_present_missing_key_on_map')
 
     def test_openvswitch_db_present_stp_enable(self):
         set_module_args(dict(state='present',
                              table='Bridge', record='test-br',
                              col='stp_enable',
-                             value='False'))
+                             value='true'))
         self.execute_module(changed=True,
                             test_name='test_openvswitch_db_present_stp_enable')