fixing #456

[steiler]

The filter caused the default bridge to be excluded from the network listing.
Hence the container IP wasn't found.
I'm not sure why the filter was introduced in #433 for issue #432. Topology mentioned in #456 works fine. #433 has no topo attached to test.

[hellt]

I believe the idea was to get the list of networks that containerlab created. hence the filter that has containerlab
but since bridge network is not the one that is created by containerlab, it was missing

[hellt]

I wonder if we can create a filter that will capture both networks that have a containerlab label OR network which have bridge name

That way we will avoid getting the networks that could be created by users/other_systems and which are not needed for our purposes.

It is potentially an optimization, which is not necessary, but seems like nice to have.

[karimra]

It will be an AND operation if you add name=bridge
To do an OR, 2 queries need to be performed.

[hellt]

yes, here is what I did

	if c.Mgmt.Network == "" {
		nctx, cancel := context.WithTimeout(ctx, c.timeout)
		defer cancel()
		// fetch containerlab created networks
		f := filters.NewArgs()
		f.Add("label", "containerlab")
		nr, err = c.Client.NetworkList(nctx, dockerTypes.NetworkListOptions{
			Filters: f,
		})

		if err != nil {
			return nil, err
		}

		// fetch default bridge network
		f = filters.NewArgs()
		f.Add("name", "bridge")
		bridgenet, err := c.Client.NetworkList(nctx, dockerTypes.NetworkListOptions{
			Filters: f,
		})

		if err != nil {
			return nil, err
		}

		nr = append(nr, bridgenet...)

not sure which is better though =)

[karimra]

This wouldn't work if no mgmt network was specified in the topo file, i.e use network clab

I'd say either do 2 queries for the networks (label=containerlab and name=bridge),

or make the var bridgeName default to bridge if it's still empty after going through the networks list, here: https://github.com/srl-labs/containerlab/blob/master/runtime/docker/docker.go#L421

[hellt]

This wouldn't work if no mgmt network was specified in the topo file, i.e use network clab

what wouldn't work? current version of this branch c8142b6 where filters are empty seems to work fine with the default network (i.e. omitted mgmt container in clab file)

[karimra]

it works because the containers are connected to a single bridge, so it defaults to that one.

The idea in #433 was to make sure the IPs printed come from a bridge that was created by clab, which turns out to be too restrictive in case --all is used.
The condition should be, a bridge created by clab or a bridge called bridge

[hellt]

in 38b11a3 the two-filters approach has been added