Change in vdsm[ovirt-3.6]: network.api: EL7's dhclient doesn't support -df so stop usin...

Ondřej Svoboda has uploaded a new change for review.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it
there
......................................................................

network.api: EL7's dhclient doesn't support -df so stop using it there

This is a quick fix only. A proper solution for the affected bug will
have to fall back to -lf (a copy of a lease file).

Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Bug-Url: https://bugzilla.redhat.com/1219429
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/45713
Reviewed-by: Francesco Romani <fromani(a)redhat.com&gt;
Tested-by: Martin Sivák <msivak(a)redhat.com&gt;
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm/network/api.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
3 files changed, 26 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/45754/1

diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 5b3a905..5bd675a 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -36,7 +36,7 @@
 from vdsm import utils
 from vdsm import ipwrapper
 
-from .configurators import libvirt
+from .configurators import dhclient, libvirt
 from .errors import ConfigNetworkError
 from . import errors as ne
 from .models import Bond, Bridge, IPv4, IPv6, Nic, Vlan
@@ -608,6 +608,14 @@
     If there is dhclient already running on a bridge's port we have to use the
     same DHCP unique identifier (DUID) in order to get the same IP address.
     """
+    # On EL7 dhclient doesn't have a -df option (to read DUID from the port's
+    # lease file). We must detect if the option is available, by running
+    # dhclient manually. To unbreak a beta4 release we just won't use the -df
+    # option in that case. A proper fix is probably to fall back to -lf,
+    # passing to it a modified NIC lease file.
+    if not dhclient.supports_duid_file():
+        return
+
     for devices in (_netinfo.nics, _netinfo.bondings, _netinfo.vlans):
         port = devices.get(bridge.port.name)
         if port and port['dhcpv4']:
diff --git a/vdsm/network/configurators/__init__.py
b/vdsm/network/configurators/__init__.py
index bca3eba..86e17a1 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -176,7 +176,7 @@
 
 def runDhclient(iface, family=4, default_route=False):
     dhclient = DhcpClient(iface.name, family, default_route, iface.duid_source)
-    rc = dhclient.start(iface.blockingdhcp)
+    rc, _, _ = dhclient.start(iface.blockingdhcp)
     if iface.blockingdhcp and rc:
         raise ConfigNetworkError(ERR_FAILED_IFUP, 'dhclient%s failed' % family)
 
diff --git a/vdsm/network/configurators/dhclient.py
b/vdsm/network/configurators/dhclient.py
index 181c302..783126b 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -28,10 +28,7 @@
 from vdsm import cmdutils
 from vdsm import ipwrapper
 from vdsm import netinfo
-from vdsm.utils import CommandPath
-from vdsm.utils import execCmd
-from vdsm.utils import pgrep
-from vdsm.utils import rmFile
+from vdsm.utils import CommandPath, execCmd, memoized, pgrep, rmFile
 
 DHCLIENT_CGROUP = 'vdsm-dhclient'
 LEASE_DIR = '/var/lib/dhclient'
@@ -68,13 +65,11 @@
         if self.duid_source_file:
             cmd += ['-df', self.duid_source_file]
         cmd = cmdutils.systemd_run(cmd, scope=True, slice=self._cgroup)
-        rc, out, err = execCmd(cmd)
-        return rc, out, err
+        return execCmd(cmd)
 
     def start(self, blocking):
         if blocking:
-            rc, _, _ = self._dhclient()
-            return rc
+            return self._dhclient()
         else:
             t = threading.Thread(target=self._dhclient, name='vdsm-dhclient-%s'
                                  % self.iface)
@@ -137,3 +132,16 @@
             raise
     if pid_file is not None:
         rmFile(pid_file)
+
+
+@memoized
+def supports_duid_file():
+    probe = DhcpClient('-invalid-option')  # dhclient doesn't have -h/--help
+    rc, out, err = probe.start(blocking=True)
+    if rc:  # -invalid-option should always fail, unlike --help if implemented
+        for line in err:
+            if '-df' in line:
+                return True
+        return False
+    else:
+        raise AssertionError("dhclient shouldn't succeed with invalid
options")


-- 
To view, visit https://gerrit.ovirt.org/45754
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it
there
......................................................................


Patch Set 1:

* Update tracker::#1219429::OK
* Check Bug-Url::OK
* Check Public Bug::#1219429::OK, public bug
* Check Product::#1219429::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/45754
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it
there
......................................................................


Patch Set 1: Code-Review+2 Verified+1

testDhcpReplaceNicWithBridge now passes on el7.

-- 
To view, visit https://gerrit.ovirt.org/45754
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it
there
......................................................................


network.api: EL7's dhclient doesn't support -df so stop using it there

This is a quick fix only. A proper solution for the affected bug will
have to fall back to -lf (a copy of a lease file).

Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Bug-Url: https://bugzilla.redhat.com/1219429
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/45713
Reviewed-by: Francesco Romani <fromani(a)redhat.com&gt;
Tested-by: Martin Sivák <msivak(a)redhat.com&gt;
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/45754
Tested-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm/network/api.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
3 files changed, 26 insertions(+), 10 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified; Looks good to me, approved



-- 
To view, visit https://gerrit.ovirt.org/45754
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org

automation(a)ovirt.org has posted comments on this change.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it
there
......................................................................


Patch Set 2:

* Update tracker::#1219429::OK
* Check TR::#1219429::OK
* Set MODIFIED::bug 1219429::::#1219429::::OK

-- 
To view, visit https://gerrit.ovirt.org/45754
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No