show ports in table after deployment / inspect #1430

steiler · 2023-06-14T11:17:34Z

INFO[0000] Parsing & checking topology file: linux.clab.yml 
+---+-----------------+--------------+---------------+-------+---------+----------------+----------------------+--------------------------+
| # |      Name       | Container ID |     Image     | Kind  |  State  |  IPv4 Address  |     IPv6 Address     |          Ports           |
+---+-----------------+--------------+---------------+-------+---------+----------------+----------------------+--------------------------+
| 1 | clab-linux01-l1 | d7ddd9653556 | alpine:latest | linux | running | 172.20.20.3/24 | 2001:172:20:20::3/64 | 0.0.0.0:6010/tcp -> 8080 |
|    |                 |              |               |       |         |                |                      | :::6010/tcp -> 8080      |
| 2 | clab-linux01-l2 | 013154efda2b | alpine:latest | linux | running | 172.20.20.2/24 | 2001:172:20:20::2/64 | 0.0.0.0:6009/tcp -> 8080 |
|    |                 |              |               |       |         |                |                      | :::6009/tcp -> 8080      |
+---+-----------------+--------------+---------------+-------+---------+----------------+----------------------+--------------------------+

What is not sooo nice is, that the table gets wider and you therefore have to have a wider screen to not have any line breaks within a row.

another option might be to also introduce a port command as docker has, to figure out the ports in a seperate output.

This cmd lists port bindings:

❯ docker ps --format "table {{.Names}}\t{{.Ports}}" 
NAMES           PORTS
clab-ports-l1   0.0.0.0:32778->80/tcp, :::32778->80/tcp
clab-srl-srl

hellt · 2023-06-16T09:42:05Z

@steiler yeah, I agree having a long table is not great.
Maybe we just put it in the docs that users could use docker ports command? Or indeed clab ports list

steiler · 2023-06-19T08:49:08Z

Alright, so the port-bindings field in now part of the auto.tmpl. Hence will be outputted as part of the topology-data.json.
Further did I remove the port field in the output table again.

steiler · 2023-06-19T08:55:54Z

So we have the below now.
I'm not sure if this is maybe already sufficient or if we need another ports list command that prints tables.

sudo ./containerlab -t /home/mava/projects/containerlab/lab-examples/linux.clab.yml inspect -f json --details
INFO[0000] Parsing & checking topology file: linux.clab.yml
[
  {
    "Names": [
      "clab-linux01-l1"
    ],
    "ID": "5fd2631796d82a4caf220a14577eb4a1ed4244ca53d7ea27857ea56f4cdb0c6e",
    "ShortID": "5fd2631796d8",
    "Image": "alpine:latest",
    "State": "running",
    "Status": "Up About a minute",
    "Labels": {
      "clab-mgmt-net-bridge": "br-6fd285acdfec",
      "clab-node-group": "",
      "clab-node-kind": "linux",
      "clab-node-lab-dir": "/home/mava/projects/containerlab/clab-linux01/l1",
      "clab-node-name": "l1",
      "clab-node-type": "",
      "clab-topo-file": "/home/mava/projects/containerlab/lab-examples/linux.clab.yml",
      "containerlab": "linux01"
    },
    "Pid": 0,
    "NetworkSettings": {
      "IPv4addr": "172.20.20.2",
      "IPv4pLen": 24,
      "IPv4Gw": "172.20.20.1",
      "IPv6addr": "2001:172:20:20::2",
      "IPv6pLen": 64,
      "IPv6Gw": "2001:172:20:20::1"
    },
    "Mounts": [
      {
        "Source": "",
        "Destination": ""
      }
    ],
    "Ports": [
      {
        "host_ip": "0.0.0.0",
        "host_port": 6089,
        "port": 8080,
        "protocol": "tcp"
      },
      {
        "host_ip": "::",
        "host_port": 6089,
        "port": 8080,
        "protocol": "tcp"
      },
      {
        "host_ip": "0.0.0.0",
        "host_port": 6088,
        "port": 8081,
        "protocol": "tcp"
      },
      {
        "host_ip": "::",
        "host_port": 6088,
        "port": 8081,
        "protocol": "tcp"
      },
      {
        "host_ip": "0.0.0.0",
        "host_port": 6087,
        "port": 8082,
        "protocol": "tcp"
      },
      {
        "host_ip": "::",
        "host_port": 6087,
        "port": 8082,
        "protocol": "tcp"
      }
    ]
  },
  {
    "Names": [
      "clab-linux01-l2"
    ],
    "ID": "d06fd4ca34fcd966f249d951aefe936f3fdacc346f3bc6a88d9f8584efd72dbf",
    "ShortID": "d06fd4ca34fc",
    "Image": "alpine:latest",
    "State": "running",
    "Status": "Up About a minute",
    "Labels": {
      "clab-mgmt-net-bridge": "br-6fd285acdfec",
      "clab-node-group": "",
      "clab-node-kind": "linux",
      "clab-node-lab-dir": "/home/mava/projects/containerlab/clab-linux01/l2",
      "clab-node-name": "l2",
      "clab-node-type": "",
      "clab-topo-file": "/home/mava/projects/containerlab/lab-examples/linux.clab.yml",
      "containerlab": "linux01"
    },
    "Pid": 0,
    "NetworkSettings": {
      "IPv4addr": "172.20.20.3",
      "IPv4pLen": 24,
      "IPv4Gw": "172.20.20.1",
      "IPv6addr": "2001:172:20:20::3",
      "IPv6pLen": 64,
      "IPv6Gw": "2001:172:20:20::1"
    },
    "Mounts": [
      {
        "Source": "",
        "Destination": ""
      }
    ],
    "Ports": [
      {
        "host_ip": "0.0.0.0",
        "host_port": 6090,
        "port": 8080,
        "protocol": "tcp"
      },
      {
        "host_ip": "::",
        "host_port": 6090,
        "port": 8080,
        "protocol": "tcp"
      }
    ]
  }
]

hellt · 2023-06-23T12:53:30Z

types/types.go

@@ -129,7 +129,7 @@ type NodeConfig struct {
 	// Bind mounts strings (src:dest:options).
 	Binds []string `json:"binds,omitempty"`
 	// PortBindings define the bindings between the container ports and host ports
-	PortBindings nat.PortMap `json:"portbindings,omitempty"`
+	PortBindings nat.PortMap `json:"-"`


was this removed for testing?

or you removed it because portbindings is now substituted by port-bindings which is calculated based on runtime state?

Yes the latter was my thinking. The assumption is, that you're interested in the actual running state, which is a concret port per mapping and the possible range is pretty much of no interest...

https://github.com/srl-labs/containerlab/pull/1430/files#diff-bb9cd8f6a91720041aaf633ac5fbaf759782c31c70128e050bc1ef0d027dd755R46

hellt · 2023-06-23T13:37:47Z

clab/export.go

+		gc, err := n.GetContainers(ctx)
+		if err != nil {
+			return err
+		}


@steiler works nicely, this piece is the only thing that I am hesitant about.
Doing an API Get for all nodes just to fetch port bindings is not optimal:

not all nodes have port bindings defined.
ADD1: unless we do not call the Get API when no port bindings are defined.

and even if they have them, maybe we can just update the binding information once we created the node? I wonder if we already receive the port binding info as a result of a container create?

since you've now made it part of NodeConfig, we should be able to get this info earlier on as part of the deployment, yes.

I remembered, that we had a function designed just for this

see 9e5952e

codecov · 2023-06-23T14:50:11Z

Codecov Report

Merging #1430 (1e5fd39) into main (922259d) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1430      +/-   ##
==========================================
- Coverage   20.58%   20.51%   -0.07%     
==========================================
  Files          58       58              
  Lines        6558     6579      +21     
==========================================
  Hits         1350     1350              
- Misses       5085     5106      +21     
  Partials      123      123

Impacted Files	Coverage Δ
clab/export.go	`0.00% <0.00%> (ø)`
cmd/deploy.go	`6.83% <0.00%> (ø)`
runtime/docker/docker.go	`0.14% <0.00%> (-0.01%)`	⬇️
types/types.go	`0.00% <0.00%> (ø)`

hellt · 2023-06-23T15:07:17Z

Thanks @steiler
This looks good now. Let's ship it

steiler marked this pull request as ready for review June 21, 2023 06:54

steiler added 4 commits June 23, 2023 15:39

show ports in table after deployment / inspect

5924fae

fixing ipv6 output format

207e3ea

adjust topology-data.json output template

de39d7c

enable GenericContainer port population in podman

3fe6488

hellt reviewed Jun 23, 2023

View reviewed changes

hellt force-pushed the displayports branch from 162f814 to 3fe6488 Compare June 23, 2023 13:14

hellt added 2 commits June 23, 2023 16:16

use ResultingPortBindings as part of the NodeConfig

7d1608c

GenericPort -> GenericPortBinding

eea3771

hellt reviewed Jun 23, 2023

View reviewed changes

hellt added 3 commits June 23, 2023 17:30

remove extra call to GetContainers

9e5952e

updated schema to allow bare port e.g. "80"

1674834

rename podman GenericPort

1e5fd39

hellt merged commit be5aeac into srl-labs:main Jun 23, 2023
18 of 19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

show ports in table after deployment / inspect #1430

show ports in table after deployment / inspect #1430

steiler commented Jun 14, 2023 •

edited by hellt

hellt commented Jun 16, 2023

steiler commented Jun 19, 2023

steiler commented Jun 19, 2023

hellt Jun 23, 2023

hellt Jun 23, 2023

steiler Jun 23, 2023

steiler Jun 23, 2023

hellt Jun 23, 2023 •

edited

steiler Jun 23, 2023

hellt Jun 23, 2023

codecov bot commented Jun 23, 2023

hellt commented Jun 23, 2023

show ports in table after deployment / inspect #1430

show ports in table after deployment / inspect #1430

Conversation

steiler commented Jun 14, 2023 • edited by hellt

hellt commented Jun 16, 2023

steiler commented Jun 19, 2023

steiler commented Jun 19, 2023

hellt Jun 23, 2023

Choose a reason for hiding this comment

hellt Jun 23, 2023

Choose a reason for hiding this comment

steiler Jun 23, 2023

Choose a reason for hiding this comment

steiler Jun 23, 2023

Choose a reason for hiding this comment

hellt Jun 23, 2023 • edited

Choose a reason for hiding this comment

steiler Jun 23, 2023

Choose a reason for hiding this comment

hellt Jun 23, 2023

Choose a reason for hiding this comment

codecov bot commented Jun 23, 2023

Codecov Report

hellt commented Jun 23, 2023

steiler commented Jun 14, 2023 •

edited by hellt

hellt Jun 23, 2023 •

edited