Page MenuHomeVyOS Platform

dhcp server statistics/leases on latest rolling gives error,tried on fresh install and migrated install same error
Open, NormalPublicBUG

Description

vyos@vyos:~$ show dhcp server leases
Traceback (most recent call last):
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 545, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 312, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 335, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 387, in show_server_leases
    lease_data = _get_raw_server_leases(family=family, pool=pool, sorted=sorted, state=state, origin=origin)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 135, in _get_raw_server_leases
    data_lease['remaining'] = lease['expire_timestamp'] - datetime.now(timezone.utc)
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: can't subtract offset-naive and offset-aware datetimes
vyos@vyos:~$ 
show dhcp server statistics
Traceback (most recent call last):
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 545, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 312, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 335, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 360, in show_pool_statistics
    pool_data = _get_raw_pool_statistics(family=family, pool=pool)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 243, in _get_raw_pool_statistics
    leases = len(_get_raw_server_leases(family=family, pool=p))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 135, in _get_raw_server_leases
    data_lease['remaining'] = lease['expire_timestamp'] - datetime.now(timezone.utc)
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: can't subtract offset-naive and offset-aware datetimes
}

Details

Version
1.5 rolling 202412310006
Is it a breaking change?
Perfectly compatible
Issue type
Bug (incorrect behavior)
Story points
0

Event Timeline

himurae updated the task description. (Show Details)
himurae changed Version from 1.5 rolling 202411230007 to 1.5 rolling 202412310006.Sat, Jan 4, 6:34 AM

Run into the same issue, including the latest self-compiled rolling from today (2025-01-04).

Tracked it down to commit 3168305a0474573ad0f36fee399baa423cbce5e4.
I'm not a programmer at all, the following was gather searching the internet as usual:

commit 3168305a0474573ad0f36fee399baa423cbce5e4
Author: Nicolas Vandamme <[email protected]>
Date:   Mon Dec 23 10:03:04 2024 +0100

    op-mode: T5992: fix Kea DHCP server lease output

And there specifically src/op_mode/dhcp.py, line 135.

134         if lease['valid-lft'] > 0:
135             data_lease['remaining'] = lease['expire_timestamp'] - datetime.now(timezone.utc)

Before datetime.now(timezone.utc) it was datetime.utcnow(). And utcnow() is depreciated start Python 3.12. All to do with the datetime object being naive or aware (timezone related). I guess for people with knowledge of Python that is all clear.
datetime.now(timezone.utc) returns an aware object, datetime.utcnow() does return a naive one. And the lease['*_timestamp'] objects are naive.

The solution seems to be making the lease['*_timestamp'] objects aware. That seems to get rid of the need for the helper function _utc_to_local() too.

The depreciated status also applies to datetime.utcfromtimestamp() a bit further up in the code. The following solved it for me temporary, for IPv4 (unfortunately I don't know if that got wider implications). IPv6 needs changing too.

 diff -Bbus /usr/libexec/vyos/op_mode/dhcp.py{.orig,}
--- /usr/libexec/vyos/op_mode/dhcp.py.orig      2025-01-04 19:16:02.431052457 +0000
+++ /usr/libexec/vyos/op_mode/dhcp.py   2025-01-04 19:15:43.937444082 +0000
@@ -101,8 +101,8 @@
         lifetime = lease['valid-lft']
         expiry = (lease['cltt'] + lifetime)

-        lease['start_timestamp'] = datetime.utcfromtimestamp(expiry - lifetime)
-        lease['expire_timestamp'] = datetime.utcfromtimestamp(expiry) if expiry else None
+        lease['start_timestamp'] = datetime.fromtimestamp(expiry - lifetime, timezone.utc)
+        lease['expire_timestamp'] = datetime.fromtimestamp(expiry, timezone.utc) if expiry else None

         data_lease = {}
         data_lease['ip'] = lease['ip-address']
@@ -170,10 +170,8 @@
             ipaddr = lease.get('ip')
             hw_addr = lease.get('mac')
             state = lease.get('state')
-            start = lease.get('start')
-            start =  _utc_to_local(start).strftime('%Y/%m/%d %H:%M:%S')
-            end = lease.get('end')
-            end =  _utc_to_local(end).strftime('%Y/%m/%d %H:%M:%S') if end else '-'
+            start =  datetime.fromtimestamp(lease.get('start'))
+            end =  datetime.fromtimestamp(lease.get('end'))
             remain = lease.get('remaining')
             pool = lease.get('pool')
             hostname = lease.get('hostname')

Maybe a skilled programmer/maintainer can fix it correctly (and test it ;) )

I had a problem with this code, but I also had a hostname that was too short for the trailing '.' check.

# remove trailing dot to ensure consistency for `vyos-hostsd-client`
if len(data_lease['hostname']) > 1 and data_lease['hostname'][-1] == '.':
    data_lease['hostname'] = data_lease['hostname'][:-1]

I opened a PR with these changes and tested them by modifying the file on the image. It works now. I'm guessing there isn't a testcase for this command anyway.

https://github.com/vyos/vyos-1x/pull/4277

I opened a PR with these changes and tested them by modifying the file on the image. It works now. I'm guessing there isn't a testcase for this command anyway.

https://github.com/vyos/vyos-1x/pull/4277

Thank you @metron2 for the PR.

Thinking about it, maybe the "strftime('%Y/%m/%d %H:%M:%S')" part has to be added to the start and end variable to get the same date/time string format as before? Although I do like ISO format better as the ../../../ is confusing for me as a not-North American as to what format is it, Y/D/M or Y/M/D. But then the leases or not set for months and therefore it's clear what format it is.

And doing the same for IPv6 may be helpful in the _get_formatted_server_leases() method?

I had a problem with this code, but I also had a hostname that was too short for the trailing '.' check.

# remove trailing dot to ensure consistency for `vyos-hostsd-client`
if len(data_lease['hostname']) > 1 and data_lease['hostname'][-1] == '.':
    data_lease['hostname'] = data_lease['hostname'][:-1]

We may have to have a closer look at that. With a VyOS release from Nov 26 it does work with an empty hostname (for an expired lease).

On a current rolling right now you patch works but the 'Remaining' time shoud be empty but is not. Probably an issue with my code changes.

$ sh dhcp server leases
IP Address       MAC address        State    Lease start          Lease expiration     Remaining                Pool    Hostname         Origin
---------------  -----------------  -------  -------------------  -------------------  -----------------------  ------  ---------------  --------
192.168.101.52   xx:xx:xx:xx:ba:76  active   2025-01-05 11:44:11  2025-01-05 12:44:11  0:46:09                  LAN     thomas-air-wifi  local
192.168.101.100  xx:xx:xx:xx:af:75  active   2025-01-05 11:30:32  2025-01-05 12:30:32  0:32:30                  LAN     iphone           local
192.168.101.101  xx:xx:xx:xx:19:e9  expired  2025-01-05 10:38:43  2025-01-05 11:38:43  -1 day, 23:40:41.954740  LAN                      local
192.168.101.102  xx:xx:xx:xx:ac:51  active   2025-01-05 11:26:56  2025-01-05 12:26:56  0:28:54                  LAN     yoga-370         local
Viacheslav triaged this task as Normal priority.Mon, Jan 6, 11:17 AM

I'll edit the ipv6 part as well, but I can't test that. My ISP is terrible and does not have any ipv6 for residential customers.

If noone has an objection I like ISO or maybe %Y-%m-%d %H:%M:%S is more common in the code?

This comment was removed by himurae.

On a current rolling right now you patch works but the 'Remaining' time should be empty but is not. Probably an issue with my code changes.

I pushed another commit to fix this too. The bug was basically that you had used 'days' in the comparison, so the code would format a negative duration as long as the time remaining was less than 24h.