Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: added FQDN validation to engine-setup-2
......................................................................
Patch Set 7: (6 inline comments)
....................................................
File packaging/setup/plugins/ovirt-engine-setup/config/protocols.py
Line 149: _('Could not find any configured IP address on the
host')
Line 150: )
Line 151: return set(iplist)
Line 152:
Line 153: def _dig_lookup(self, addr, reverse=False):
I suggest we split this function to forward and reverse instead of having
switch parameter.
Forward should return boolean only, with may be simplified RE.
Line 154: pattern = (self._DIG_REVLOOKUP_RE if reverse else
self._DIG_LOOKUP_RE)
Line 155: addresses = set()
Line 156: args = [self.command.get('dig')]
Line 157: if reverse:
Line 217: )
Line 218: else:
Line 219: #reverse resolved IP and compare with given fqdn
Line 220: counter = 0
Line 221: for address in resolvedAddresses:
I do not understand this logic.
Line 222: addressSet = self._dig_lookup(
Line 223: addr=address,
Line 224: reverse=True
Line 225: )
Line 224: reverse=True
Line 225: )
Line 226: reResolvedAddress = None
Line 227: revResolved = False
Line 228: if len(addressSet) > 0:
why do you check only one from the result?
Line 229: reResolvedAddress = addressSet.pop()
Line 230: if reResolvedAddress.lower() == fqdn.lower():
Line 231: counter += 1
Line 232: revResolved = True
Line 227: revResolved = False
Line 228: if len(addressSet) > 0:
Line 229: reResolvedAddress = addressSet.pop()
Line 230: if reResolvedAddress.lower() == fqdn.lower():
Line 231: counter += 1
Why is that needed?
Line 232: revResolved = True
Line 233: if not revResolved:
Line 234: self.logger.warning(
Line 235: _(
Line 229: reResolvedAddress = addressSet.pop()
Line 230: if reResolvedAddress.lower() == fqdn.lower():
Line 231: counter += 1
Line 232: revResolved = True
Line 233: if not revResolved:
So this can be out side of the look, right?
Line 234: self.logger.warning(
Line 235: _(
Line 236: '{address} did not reverse-resolve into
{fqdn}'
Line 237: ).format(
Line 238: address=address,
Line 239: fqdn=fqdn,
Line 240: )
Line 241: )
Line 242: if counter < 1:
How can counter be < 0 if we only incrementing it?
Line 243: raise RuntimeError(
Line 244: _(
Line 245: 'The following addresses: {addresses} did not
reverse'
Line 246: 'resolve into {fqdn}'
--
To view, visit http://gerrit.ovirt.org/14699
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9764f32ba5e30062532bf11c67756677333c44f
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
