Skip to content

Commit

Permalink
openvswitch_db: Make 'key' parameter optional (ansible#42110)
Browse files Browse the repository at this point in the history
The OVSDB schema consists of typed columns. The 'key' parameter is
required only for columns with type of a 'map'. This patch makes 'key'
an optional parameter to allow setting values for other column types
like int.

Fixes ansible#42108
  • Loading branch information
cubeek authored and gdpak committed Jul 9, 2018
1 parent e9c7b51 commit 26b0908
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 11 deletions.
53 changes: 44 additions & 9 deletions lib/ansible/modules/network/ovs/openvswitch_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@
description:
- Identifies the column in the record.
key:
required: true
required: false
description:
- Identifies the key in the record column
- Identifies the key in the record column, when the column is a map
type.
value:
required: true
description:
Expand Down Expand Up @@ -87,11 +88,23 @@
record: br-int
col: other_config
key: disable-in-band
# Mark port with tag 10
- openvswitch_db:
table: Port
record: port0
col: tag
value: 10
'''
import re

from ansible.module_utils.basic import AnsibleModule

# Regular expression for map type, must not be empty
NON_EMPTY_MAP_RE = re.compile(r'{.+}')
# Regular expression for a map column type
MAP_RE = re.compile(r'{.*}')


def map_obj_to_commands(want, have, module):
""" Define ovs-vsctl command to meet desired state """
Expand All @@ -102,8 +115,16 @@ def map_obj_to_commands(want, have, module):
templatized_command = "%(ovs-vsctl)s -t %(timeout)s remove %(table)s %(record)s " \
"%(col)s %(key)s=%(value)s"
commands.append(templatized_command % module.params)
elif module.params['key'] is None:
templatized_command = "%(ovs-vsctl)s -t %(timeout)s remove %(table)s %(record)s " \
"%(col)s"
commands.append(templatized_command % module.params)
else:
if 'key' not in have.keys():
if module.params['key'] is None:
templatized_command = "%(ovs-vsctl)s -t %(timeout)s set %(table)s %(record)s " \
"%(col)s=%(value)s"
commands.append(templatized_command % module.params)
elif 'key' not in have.keys():
templatized_command = "%(ovs-vsctl)s -t %(timeout)s add %(table)s %(record)s " \
"%(col)s %(key)s=%(value)s"
commands.append(templatized_command % module.params)
Expand All @@ -125,8 +146,16 @@ def map_config_to_obj(module):
match = re.search(r'^' + module.params['col'] + r'(\s+):(\s+)(.*)$', out, re.M)

col_value = match.group(3)

# Map types require key argument
has_key = module.params['key'] is not None
is_map = MAP_RE.match(col_value)
if is_map and not has_key:
module.fail_json(
msg="missing required arguments: key for map type of column")

col_value_to_dict = {}
if col_value and col_value != '{}':
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()
Expand All @@ -137,9 +166,12 @@ def map_config_to_obj(module):
'col': module.params['col'],
}

if module.params['key'] in col_value_to_dict:
obj['key'] = module.params['key']
obj['value'] = col_value_to_dict[module.params['key']]
if has_key and is_map:
if module.params['key'] in col_value_to_dict:
obj['key'] = module.params['key']
obj['value'] = col_value_to_dict[module.params['key']]
else:
obj['value'] = col_value.strip()

return obj

Expand All @@ -149,10 +181,13 @@ def map_params_to_obj(module):
'table': module.params['table'],
'record': module.params['record'],
'col': module.params['col'],
'key': module.params['key'],
'value': module.params['value']
}

key = module.params['key']
if key is not None:
obj['key'] = key

return obj


Expand All @@ -163,7 +198,7 @@ def main():
'table': {'required': True},
'record': {'required': True},
'col': {'required': True},
'key': {'required': True},
'key': {'required': False},
'value': {'required': True},
'timeout': {'default': 5, 'type': 'int'},
}
Expand Down
41 changes: 39 additions & 2 deletions test/integration/targets/openvswitch_db/tests/basic.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
that:
- "result.changed == false"

- name: Change column value
- name: Change column value in a map
openvswitch_db:
table: Bridge
record: br-test
Expand All @@ -46,7 +46,7 @@
that:
- "result.changed == true"

- name: Change column value again (idempotent)
- name: Change column value in a map again (idempotent)
openvswitch_db:
table: Bridge
record: br-test
Expand All @@ -59,6 +59,43 @@
that:
- "result.changed == false"

- name: Change column value
openvswitch_db:
table: Bridge
record: br-test
col: stp_enable
value: true
register: result

- assert:
that:
- "result.changed == true"

- name: Change column value again (idempotent)
openvswitch_db:
table: Bridge
record: br-test
col: stp_enable
value: true
register: result

- assert:
that:
- "result.changed == false"

- name: Try to set value on a map type without a key (negative)
ignore_errors: true
openvswitch_db:
table: Bridge
record: br-test
col: other_config
value: true
register: result

- assert:
that:
- "result.failed == true"

- name: Remove bridge
openvswitch_db:
table: Bridge
Expand Down
23 changes: 23 additions & 0 deletions test/units/modules/network/ovs/test_openvswitch_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
'test_openvswitch_db_present_updates_key': [
(0, 'openvswitch_db_disable_in_band_true.cfg', None),
(0, None, None)],
'test_openvswitch_db_present_missing_key_on_map': [
(0, 'openvswitch_db_disable_in_band_true.cfg', None),
(0, None, None)],
'test_openvswitch_db_present_stp_enable': [
(0, 'openvswitch_db_disable_in_band_true.cfg', None),
(0, None, None)],
}


Expand Down Expand Up @@ -122,3 +128,20 @@ def test_openvswitch_db_present_updates_key(self):
commands=['/usr/bin/ovs-vsctl -t 5 set Bridge test-br other_config'
':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'))
self.execute_module(
failed=True,
test_name='test_openvswitch_db_present_idempotent')

def test_openvswitch_db_present_stp_enable(self):
set_module_args(dict(state='present',
table='Bridge', record='test-br',
col='stp_enable',
value='False'))
self.execute_module(changed=True,
test_name='test_openvswitch_db_present_stp_enable')

0 comments on commit 26b0908

Please sign in to comment.