Skip to content

Commit

Permalink
commands/status: query routes from all the tables
Browse files Browse the repository at this point in the history
We were listing routes from only the main table. The new diff
functionality will need all routes in order to calculate the diff
properly.

The extra routes are not displayed by default.

A new parameter, --verbose, was added and the extra routes will be
displayed when netplan status is called with it.
  • Loading branch information
daniloegea committed Aug 8, 2023
1 parent 9fd9542 commit 90f0a3f
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 14 deletions.
10 changes: 9 additions & 1 deletion netplan_cli/cli/commands/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ def run(self):
help='Show only this interface')
self.parser.add_argument('-a', '--all', action='store_true',
help='Show all interface data (incl. inactive)')
self.parser.add_argument('-v', '--verbose', action='store_true',
help='Show extra information')
self.parser.add_argument('-f', '--format', default='tabular',
help='Output in machine readable `json` or `yaml` format')

Expand Down Expand Up @@ -172,6 +174,8 @@ def pretty_print(self, data: JSON, total: int, _console_width=None) -> None:
lst = data.get('routes', [])
if lst:
for i, obj in enumerate(lst):
if not self.verbose and obj.get('table') != 'main':
continue
default_start = ''
default_end = ''
if obj['to'] == 'default':
Expand All @@ -186,6 +190,9 @@ def pretty_print(self, data: JSON, total: int, _console_width=None) -> None:
metric = ''
if 'metric' in obj:
metric = ' metric ' + str(obj['metric'])
table = ''
if self.verbose and 'table' in obj:
table = ' table ' + str(obj['table'])

extra = []
if 'protocol' in obj and obj['protocol'] != 'kernel':
Expand All @@ -198,12 +205,13 @@ def pretty_print(self, data: JSON, total: int, _console_width=None) -> None:
type = obj['type']
extra.append(type)

pprint(('{title:>'+pad+'} {start}{to}{via}{src}{metric}{end}[muted]{extra}[/muted]').format(
pprint(('{title:>'+pad+'} {start}{to}{via}{src}{metric}{table}{end}[muted]{extra}[/muted]').format(
title='Routes:' if i == 0 else '',
to=obj['to'],
via=via,
src=src,
metric=metric,
table=table,
extra=' ('+', '.join(extra)+')' if extra else '',
start=default_start,
end=default_end))
Expand Down
6 changes: 4 additions & 2 deletions netplan_cli/cli/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ def __init__(self, ip: dict, nd_data: JSON = [], nm_data: JSON = [],
elem['scope'] = val
if val := obj.get('protocol'):
elem['protocol'] = val
if val := obj.get('table'):
elem['table'] = val
self.routes.append(elem)

self.addresses: list = None
Expand Down Expand Up @@ -441,10 +443,10 @@ def query_routes(cls) -> tuple:
data4 = None
data6 = None
try:
output4: str = subprocess.check_output(['ip', '-d', '-j', 'route'],
output4: str = subprocess.check_output(['ip', '-d', '-j', '-4', 'route', 'show', 'table', 'all'],
text=True)
data4: JSON = cls.process_generic(output4)
output6: str = subprocess.check_output(['ip', '-d', '-j', '-6', 'route'],
output6: str = subprocess.check_output(['ip', '-d', '-j', '-6', 'route', 'show', 'table', 'all'],
text=True)
data6: JSON = cls.process_generic(output6)
except Exception as e:
Expand Down
12 changes: 6 additions & 6 deletions tests/cli/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ def test_query_routes(self, mock):
mock.side_effect = [ROUTE4, ROUTE6]
res4, res6 = SystemConfigState.query_routes()
mock.assert_has_calls([
call(['ip', '-d', '-j', 'route'], text=True),
call(['ip', '-d', '-j', '-6', 'route'], text=True),
call(['ip', '-d', '-j', '-4', 'route', 'show', 'table', 'all'], text=True),
call(['ip', '-d', '-j', '-6', 'route', 'show', 'table', 'all'], text=True),
])
self.assertEqual(len(res4), 6)
self.assertEqual(len(res4), 7)
self.assertListEqual([route.get('dev') for route in res4],
['enp0s31f6', 'wlan0', 'wg0', 'enp0s31f6', 'wlan0', 'enp0s31f6'])
['enp0s31f6', 'wlan0', 'wg0', 'enp0s31f6', 'wlan0', 'enp0s31f6', 'enp0s31f6'])
self.assertEqual(len(res6), 10)
self.assertListEqual([route.get('dev') for route in res6],
['lo', 'enp0s31f6', 'wlan0', 'enp0s31f6', 'wlan0',
Expand All @@ -132,7 +132,7 @@ def test_query_routes_fail(self, mock):
mock.side_effect = subprocess.CalledProcessError(1, '', 'ERR')
with self.assertLogs(level='DEBUG') as cm:
res4, res6 = SystemConfigState.query_routes()
mock.assert_called_with(['ip', '-d', '-j', 'route'], text=True)
mock.assert_called_with(['ip', '-d', '-j', '-4', 'route', 'show', 'table', 'all'], text=True)
self.assertIsNone(res4)
self.assertIsNone(res6)
self.assertIn('DEBUG:root:Cannot query iproute2 route data:', cm.output[0])
Expand Down Expand Up @@ -381,7 +381,7 @@ def test_json_nd_enp0s31f6(self, networkctl_mock):
self.assertIn('dhcp', meta.get('flags'))
self.assertEqual(len(json.get('dns_addresses')), 2)
self.assertEqual(len(json.get('dns_search')), 1)
self.assertEqual(len(json.get('routes')), 7)
self.assertEqual(len(json.get('routes')), 8)

def test_json_nd_tunnel(self):
data = next((itf for itf in yaml.safe_load(IPROUTE2) if itf['ifindex'] == 41), {})
Expand Down

0 comments on commit 90f0a3f

Please sign in to comment.