status: Add dual-stack mgmt address support and integration coverage#3532
status: Add dual-stack mgmt address support and integration coverage#3532moogzy wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the netlab status CLI output to improve management IP visibility by showing dual-stack (IPv4 + IPv6) management addressing, and adds platform integration tests to cover the new status output behavior.
Changes:
- Update
netlab statusto display IPv6 management addresses (when present) alongside IPv4 in the mgmt column. - Add platform integration CLI tests for IPv4-only and dual-stack mgmt status output.
- Fix
tests/check-integration-tests.shrobustness by initializing the error counter and quoting file paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
netsim/cli/status.py |
Adds IPv6 mgmt capture and renders mgmt as multi-line output in the status table. |
tests/platform-integration/cli/12-status-mgmt-dual-stack.yml |
New integration test ensuring status output contains both IPv4 and IPv6 mgmt addresses. |
tests/platform-integration/cli/13-status-mgmt-v4-only.yml |
New integration test ensuring status output contains the IPv4 mgmt address for IPv4-only nodes. |
tests/check-integration-tests.sh |
Initializes err_cnt and quotes $file to avoid validation script warnings/word-splitting issues. |
Fixes ipspace#3358 What changed: - Render node management addresses in netlab status with IPv6 on a new line under IPv4 in the mgmt column. - Add platform integration test tests/platform-integration/cli/12-status-mgmt.yml to validate dual-stack mgmt output. - Harden tests/check-integration-tests.sh by initializing err_cnt and quoting topology path arguments. Why: - The status output did not clearly present dual-stack management addresses and could be ambiguous/truncated in narrow output. - Integration validation emitted a noisy shell warning when err_cnt was unset.
521a636 to
464919e
Compare
| 'mgmt': n_data.mgmt.ipv4 | ||
| } | ||
| node_stat = ls.nodes[n_name] | ||
| if n_data.get('mgmt.ipv6'): |
There was a problem hiding this comment.
This is perfectly fine, but if you want, you could rewrite this whole thing into a loop over a dict mapping node data into status data.
ipspace
left a comment
There was a problem hiding this comment.
Nice job. The "do not set mgmt on tools" should be reverted because it changes JSON data. Other than that, it's just a few minor quirks/nits.
Thank you!
| wk_state = p_status[n_provider].get(wk_name,get_empty_box()) | ||
| ls.nodes[t_name] = { | ||
| 'device': '(tool)', | ||
| 'mgmt': '', |
| def show_lab_nodes(ls: Box, topology: Box) -> None: | ||
| rows = [] | ||
| heading = [ 'node', 'device', 'image', 'mgmt IPv4', 'connection', 'provider', 'VM/container', 'status'] | ||
| heading = [ 'node', 'device', 'image', 'mgmt', 'connection', 'provider', 'VM/container', 'status'] |
|
|
||
| for n_name,n_data in ls.nodes.items(): | ||
| row = [ n_name, n_data.device, n_data.image, n_data.get('mgmt',''), | ||
| mgmt = f'{n_data.mgmt}\n{n_data.mgmt6}' if n_data.get('mgmt6') else n_data.get('mgmt') |
There was a problem hiding this comment.
If you leave the old n_data.get('mgmt','') as the fallback expression, then you don't need to set 'mgmt' for tools.
| @@ -0,0 +1,26 @@ | |||
| --- | |||
| message: | | |||
There was a problem hiding this comment.
I love that you implemented the integration tests, but I'd put v4 only test first (just swap the sequence numbers) and use a topology that combines libvirt, clab, and a tool, so that we test whether status works over all three objects.
|
|
||
| ┏━━━━━━━━━┳━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓ | ||
| ┃ node ┃ device ┃ image ┃ mgmt IPv4 ┃ connection ┃ provider ┃ VM/container ┃ status ┃ | ||
| ┃ node ┃ device ┃ image ┃ mgmt ┃ connection ┃ provider ┃ VM/container ┃ status ┃ |
There was a problem hiding this comment.
This would have to change if we go for "mgmt IP" (below)
Summary:
Add dual-stack management visibility in
netlab statusoutput with integration coverage for that behaviour.Addresses #3358
// Previous behaviour
// New behaviour - backwards compatible with default v4 address only
// New behaviour - dual-stack support
Changes: