Rework tests and fix reverse peering with gluster 3.7

Reworked the test structure design for the gluster module and state so I
could create a test that mimiced the behavior difference between 3.7 and
prior versions when reverse probing by ip a server that already exists.

Added the additional data given by gluster 3.7 to the output of
glusterfs.list_peers to allow us to compare a requested peering against
a host's aliases.

Fixes #30932

salt/modules/glusterfs.py

@@ -68,9 +68,10 @@ def _gluster_xml(cmd):
 
 
 def _etree_to_dict(t):
-    if len(t.getchildren()) > 0:
+    list_t = list(t)
+    if len(list_t) > 0:
         d = {}
-        for child in t.getchildren():
+        for child in list_t:
             d[child.tag] = _etree_to_dict(child)
     else:
         d = t.text
@@ -117,7 +118,10 @@ def list_peers():
 
     '''
     root = _gluster_xml('peer status')
-    result = [x.find('hostname').text for x in _iter(root, 'peer')]
+    result = {}
+    for et_peer in _iter(root, 'peer'):
+        result.update({et_peer.find('hostname').text: [
+                      x.text for x in _iter(et_peer.find('hostnames'), 'hostname')]})
     if len(result) == 0:
         return None
     else:
@@ -161,7 +165,7 @@ def peer(name):
     cmd = 'peer probe {0}'.format(name)
 
     op_result = {
-        "exitval": _gluster_xml(cmd).find('opRet').text,
+        "exitval": _gluster_xml(cmd).find('opErrno').text,
         "output": _gluster_xml(cmd).find('output').text
     }
     return op_result
@@ -349,7 +353,7 @@ def info(name):
     for i, brick in enumerate(_iter(volume, 'brick'), start=1):
         brickkey = 'brick{0}'.format(i)
         bricks[brickkey] = {'path': brick.text}
-        for child in brick.getchildren():
+        for child in list(brick):
             if not child.tag == 'name':
                 bricks[brickkey].update({child.tag: child.text})
         for k, v in brick.items():

salt/states/glusterfs.py

@@ -11,9 +11,24 @@
 
 # Import salt libs
 import salt.utils.cloud as suc
+from salt.exceptions import SaltCloudException
 
 log = logging.getLogger(__name__)
 
+RESULT_CODES = [
+    'Peer {0} added successfully.',
+    'Probe on localhost not needed',
+    'Host {0} is already in the peer group',
+    'Host {0} is already part of another cluster',
+    'Volume on {0} conflicts with existing volumes',
+    'UUID of {0} is the same as local uuid',
+    '{0} responded with "unknown peer". This could happen if {0} doesn\'t have localhost defined',
+    'Failed to add peer. Information on {0}\'s logs',
+    'Cluster quorum is not met. Changing peers is not allowed.',
+    'Failed to update list of missed snapshots from {0}',
+    'Conflict comparing list of snapshots from {0}',
+    'Peer is already being detached from cluster.']
+
 
 def __virtual__():
     '''
@@ -48,38 +63,40 @@ def peered(name):
            'comment': '',
            'result': False}
 
+    try:
+        suc.check_name(name, 'a-zA-Z0-9._-')
+    except SaltCloudException as e:
+        ret['comment'] = 'Invalid characters in peer name.'
+        ret['result'] = False
+        return ret
+
     peers = __salt__['glusterfs.list_peers']()
 
     if peers:
-        if name in peers:
+        if name in peers or any([name in peers[x] for x in peers]):
             ret['result'] = True
             ret['comment'] = 'Host {0} already peered'.format(name)
             return ret
-        elif __opts__['test']:
-            ret['comment'] = 'Peer {0} will be added.'.format(name)
-            ret['result'] = None
-            return ret
-
-    if suc.check_name(name, 'a-zA-Z0-9._-'):
-        ret['comment'] = 'Invalid characters in peer name.'
-        ret['result'] = False
-        return ret
 
-    if 'output' in __salt__['glusterfs.peer'](name):
-        ret['comment'] = __salt__['glusterfs.peer'](name)['output']
-    else:
-        ret['comment'] = ''
+    result = __salt__['glusterfs.peer'](name)
+    ret['comment'] = ''
+    if 'exitval' in result:
+        if int(result['exitval']) <= len(RESULT_CODES):
+            ret['comment'] = RESULT_CODES[int(result['exitval'])].format(name)
+        else:
+            if 'comment' in result:
+                ret['comment'] = result['comment']
 
     newpeers = __salt__['glusterfs.list_peers']()
-    #if newpeers was null, we know something didn't work.
-    if newpeers and name in newpeers:
+    # if newpeers was null, we know something didn't work.
+    if newpeers and name in newpeers or any([name in newpeers[x] for x in newpeers]):
         ret['result'] = True
         ret['changes'] = {'new': newpeers, 'old': peers}
-    #In case the hostname doesn't have any periods in it
+    # In case the hostname doesn't have any periods in it
     elif name == socket.gethostname():
         ret['result'] = True
         return ret
-    #In case they have a hostname like "example.com"
+    # In case they have a hostname like "example.com"
     elif name == socket.gethostname().split('.')[0]:
         ret['result'] = True
         return ret
@@ -234,9 +251,9 @@ def add_volume_bricks(name, bricks):
               - host2:/srv/gluster/drive3
     '''
     ret = {'name': name,
-       'changes': {},
-       'comment': '',
-       'result': False}
+           'changes': {},
+           'comment': '',
+           'result': False}
 
     current_bricks = __salt__['glusterfs.status'](name)
 
@@ -257,7 +274,8 @@ def add_volume_bricks(name, bricks):
         old_bricks = current_bricks
         new_bricks = __salt__['glusterfs.status'](name)
         ret['result'] = True
-        ret['changes'] = {'new': list(new_bricks['bricks'].keys()), 'old': list(old_bricks['bricks'].keys())}
+        ret['changes'] = {'new': list(new_bricks['bricks'].keys()), 'old': list(
+            old_bricks['bricks'].keys())}
         return ret
 
     if 'Bricks already in volume' in add_bricks: