Fix for setting values using gconftool-2 (#23713)

Miscallenous fixes added to make gconftool-2 work.

Fixes https://github.com/ansible/ansible/issues/23670

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/modules/system/gconftool2.py

@@ -1,6 +1,7 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 # (c) 2016, Kenneth D. Evensen <kevensen@redhat.com>
+# (c) 2017, Abhijeet Kasurde <akasurde@redhat.com>
 #
 # This file is part of Ansible (sort of)
 #
@@ -103,9 +104,10 @@ RETURN = '''
 ...
 '''
 
+from subprocess import Popen, PIPE
 from ansible.module_utils.basic import AnsibleModule, BOOLEANS_TRUE
 from ansible.module_utils.pycompat24 import get_exception
-import subprocess
+
 
 
 class GConf2Preference(object):
@@ -121,7 +123,8 @@ class GConf2Preference(object):
     def value_already_set(self):
         return False
 
-    def call(self, call_type):
+    def call(self, call_type, fail_onerr=True):
+        """ Helper function to perform gconftool-2 operations """
         config_source = ''
         direct = ''
         changed = False
@@ -137,34 +140,41 @@ class GConf2Preference(object):
             direct = "--direct"
 
         # Execute the call
+        cmd = "gconftool-2 "
         try:
             # If the call is "get", then we don't need as many parameters and
             # we can ignore some
             if call_type == 'get':
-                process = subprocess.Popen(["gconftool-2 --get " + self.key],
-                                           stdout=subprocess.PIPE,
-                                           stderr=subprocess.PIPE, shell=True)
+                cmd += "--get {0}".format(self.key)
             # Otherwise, we will use all relevant parameters
-            else:
-                process = subprocess.Popen(["gconftool-2 " + direct + " " +
-                                           config_source + " --type " +
-                                           self.value_type + " --" +
-                                           call_type + " " + self.key + " " +
-                                           self.value], stdout=subprocess.PIPE,
-                                           stderr=subprocess.PIPE, shell=True)
+            elif call_type == 'set':
+                cmd += "{0} {1} --type {2} --{3} {4} \"{5}\"".format(direct,
+                                                                     config_source,
+                                                                     self.value_type,
+                                                                     call_type,
+                                                                     self.key,
+                                                                     self.value)
+            elif call_type == 'unset':
+                cmd += "--unset {0}".format(self.key)
+
+            # Start external command
+            process = Popen([cmd], stdout=PIPE, stderr=PIPE, shell=True)
+
             # In either case, we will capture the output
             out = process.stdout.read()
             err = process.stderr.read()
 
             if len(err) > 0:
-                self.ansible.fail_json(msg='gconftool-2 failed with error: %s'
-                                       % (str(err)))
+                if fail_onerr:
+                    self.ansible.fail_json(msg='gconftool-2 failed with '
+                                               'error: %s' % (str(err)))
             else:
                 changed = True
 
         except OSError:
-            self.ansible.fail_json(msg='gconftool-2 failed with and exception')
-
+            exception = get_exception()
+            self.ansible.fail_json(msg='gconftool-2 failed with exception: '
+                                       '%s' % exception)
         return changed, out.rstrip()
 
 
@@ -174,18 +184,15 @@ def main():
         argument_spec=dict(
             key=dict(required=True, default=None, type='str'),
             value_type=dict(required=False,
-                                           choices=['int', 'bool',
-                                                    'float', 'string'],
-                                           type='str'),
-            value=dict(required=False, default=None,
+                            choices=['int', 'bool', 'float', 'string'],
                             type='str'),
-            state=dict(required=True, default=None,
+            value=dict(required=False, default=None, type='str'),
+            state=dict(required=True,
+                       default=None,
                        choices=['present', 'get', 'absent'],
                        type='str'),
-            direct=dict(required=False,
-                                       default=False, type='bool'),
-            config_source=dict(required=False,
-                                              default=None, type='str')
+            direct=dict(required=False, default=False, type='bool'),
+            config_source=dict(required=False, default=None, type='str')
             ),
         supports_check_mode=True
     )
@@ -230,8 +237,8 @@ def main():
     # Create a gconf2 preference
     gconf_pref = GConf2Preference(module, key, value_type,
                                   value, direct, config_source)
-    # Now we get the current value
-    _, current_value = gconf_pref.call("get")
+    # Now we get the current value, if not found don't fail
+    _, current_value = gconf_pref.call("get", fail_onerr=False)
 
     # Check if the current value equals the value we want to set.  If not, make
     # a change
@@ -250,11 +257,12 @@ def main():
     else:
         new_value = current_value
 
-    facts = {}
-    facts['gconftool2'] = {'changed': change, 'key': key,
-                           'value_type': value_type, 'new_value': new_value,
+    facts = dict(gconftool2={'changed': change,
+                             'key': key,
+                             'value_type': value_type,
+                             'new_value': new_value,
                              'previous_value': current_value,
-                           'playbook_value': module.params['value']}
+                             'playbook_value': module.params['value']})
 
     module.exit_json(changed=change, ansible_facts=facts)