Bug 1223530 - vdsm fails to read dhclient lease config "expire never"
Summary: vdsm fails to read dhclient lease config "expire never"
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: vdsm
Version: 3.5.1
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ovirt-3.6.0-rc
: 3.6.0
Assignee: Ondřej Svoboda
QA Contact: Meni Yakove
URL:
Whiteboard:
Depends On:
Blocks: 1224632
TreeView+ depends on / blocked
 
Reported: 2015-05-20 19:11 UTC by Marina Kalinin
Modified: 2019-09-12 08:29 UTC (History)
13 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1224632 (view as bug list)
Environment:
Last Closed: 2016-03-09 19:40:15 UTC
oVirt Team: Network
Target Upstream Version:
Embargoed:
ylavi: Triaged+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Knowledge Base (Solution) 1464153 0 None None None Never
Red Hat Product Errata RHBA-2016:0362 0 normal SHIPPED_LIVE vdsm 3.6.0 bug fix and enhancement update 2016-03-09 23:49:32 UTC
oVirt gerrit 41266 0 master MERGED netinfo: handle never-expiring leases Never
oVirt gerrit 41354 0 ovirt-3.5 MERGED netinfo: handle never-expiring leases Never

Description Marina Kalinin 2015-05-20 19:11:26 UTC
Description of problem:
When dhclient leases file contain "never" for expire rather then a date, vdsm throws this exception:

supervdsm.log:
~~~
MainThread::DEBUG::2015-05-13 14:45:19,991::libvirtconnection::150::root::(get) trying to connect libvirt
MainThread::ERROR::2015-05-13 14:45:20,021::upgrade::95::upgrade::(apply_upgrade) Failed to run upgrade-unified-persistence
Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/vdsm/tool/upgrade.py", line 93, in apply_upgrade
    upgrade.run(ns, args)
  File "/usr/lib/python2.6/site-packages/vdsm/tool/unified_persistence.py", line 169, in run
    run()
  File "/usr/lib/python2.6/site-packages/vdsm/tool/unified_persistence.py", line 43, in run
    networks, bondings = _getNetInfo()
  File "/usr/lib/python2.6/site-packages/vdsm/tool/unified_persistence.py", line 129, in _getNetInfo
    netinfo = NetInfo()
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 820, in __init__
    _netinfo = get()
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 730, in get
    d['networks'] = _libvirtNets2vdsm(nets, gateways, ipv6routes, ipaddrs)
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 687, in _libvirtNets2vdsm
    dhcp4 = getDhclientIfaces(_DHCLIENT_LEASES_GLOBS)
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 673, in getDhclientIfaces
    interfaces.update(_parseLeaseFile(leaseFile, ipv6))
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 639, in _parseLeaseFile
    expiryTime = _parseExpiryTime(line[len(EXPIRE):end])
  File "/usr/lib/python2.6/site-packages/vdsm/netinfo.py", line 617, in _parseExpiryTime
    return datetime.strptime(expiryTime, '%w %Y/%m/%d %H:%M:%S')
  File "/usr/lib64/python2.6/_strptime.py", line 325, in _strptime
    (data_string, format))
ValueError: time data 'never' does not match format '%w %Y/%m/%d %H:%M:%S'
~~~

Version-Release number of selected component (if applicable):
vdsm-4.16.13.1-1.el6ev

Reading dhclient.conf man pages, this setting is valid:
"If the time is infinite in duration, then the date is never instead of an actual date."
Thus vdsm should be able handling it.

Comment 3 Marina Kalinin 2015-05-21 04:17:54 UTC
Looking into netinfo.py, I see that parsing the "expire" does not do anything actually - it does not assign the expiration time to anywhere outside this function. So I suggest to completely get rid of that piece of code.
And this is also what I am going to suggest to the customer as a test package for now.

diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 2949a6f..8b9b196 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -631,15 +631,15 @@ def _parseLeaseFile(leaseFile, ipv6):
             if line.startswith(IFACE) and line.endswith(IFACE_END):
                 name = line[len(IFACE):-len(IFACE_END)]
 
-            elif line.startswith(EXPIRE):
-                end = line.find(';')
-                if end == -1:
-                    continue  # the line should always contain a ;
-
-                expiryTime = _parseExpiryTime(line[len(EXPIRE):end])
-                if datetime.utcnow() > expiryTime:
-                    insideLease = False
-                    continue
+#            elif line.startswith(EXPIRE):
+#                end = line.find(';')
+#                if end == -1:
+#                    continue  # the line should always contain a ;
+#
+#                expiryTime = _parseExpiryTime(line[len(EXPIRE):end])
+#                if datetime.utcnow() > expiryTime:
+#                    insideLease = False
+#                    continue

Comment 4 Ondřej Svoboda 2015-05-21 17:26:18 UTC
May I ask you for your dhclient version?

Currently (as I am travelling) I have limited access to my EL6 host so I only tried to reproduce a bug on Fedora 21, and I failed (probably due to a bug in dhclient). I will try to reproduce on EL6 later.

===

I was not able to obtain a lease containing "expire never" using dhclient 4.3.1-12.fc21 and dnsmasq 2.72-3.fc21 (with option --dhcp-range=240.0.0.10,240.0.0.100,infinite).

The lease contained "expire 0 2151/06/27 13:03:58;" instead, although there is a constant MAX_TIME in dhclient's source that translates to "never". dnsmasq was aware of the option (dnsmasq-dhcp: DHCP, IP range 240.0.0.10 -- 240.0.0.100, lease time infinite).

I recorded DHCP communication between dnsmasq and dhclient and in DHCP Offer there was "IP Address Lease Time: (4294967295s) infinity" (interpreted by Wireshark). So it looks like a bug in dhclient (which in turn prevents a bug in VDSM) in Fedora 21.

Comment 6 Ondřej Svoboda 2015-05-22 12:34:05 UTC
I tried to reproduce on EL6 but again, failed to obtain the lease with "expire never".

Assuming that the lease that exposed VDSM's bug was indeed written by dhclient and not altered manually (both of which is unlikely) it seems that I'll have to dig in dhclient sources to understand how to really trigger "expire never".

Nevertheless, Dan's quick fix includes and passes a unit test with a lease containing "expire never".

Comment 9 Marina Kalinin 2015-05-25 15:35:48 UTC
Hi Ondrej,
Is there still anything required from my side?
Simon has provided the dhcp info earlier.

How I reproduced it - just by editing all the fails to "never" instead of actual date.

Comment 10 Marina Kalinin 2015-05-25 16:47:11 UTC
And, according to the man pages, never is a valid option.

Comment 11 Ondřej Svoboda 2015-05-26 08:36:02 UTC
Hi Marina,

thanks for the clarification of how you reproduced the bug. "never" is indeed a valid value.

All I was after was a way to make dhclient write such a file. When inspecting its code (which uses integer overflow more than I would like) I found no other caveat ("expire" may only contain "never", or a datetime in two formats we already handle).

Comment 12 Michael Burman 2015-06-14 06:24:23 UTC
In the future please provide 'Fixed in Version' when moving bugs to ON_QA, thanks.

Verified on - 3.6.0-0.0.master.20150519172219.git9a2e2b3.el6.noarch
with vdsm-4.17.0-912.git25a063d.el7.noarch and dhclient-4.2.5-36.el7.x86_64

Comment 13 Ondřej Svoboda 2015-08-07 13:25:54 UTC
Gil Klein, this bug is covered by the testGetDhclientIfaces unit test (in tests/netinfoTests.py), is this helpful to resolve qe_test_coverage?

Comment 15 errata-xmlrpc 2016-03-09 19:40:15 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-0362.html


Note You need to log in before you can comment on or make changes to this bug.