Skip to content

Conversation

@mzagozen
Copy link
Contributor

Containerlab passes all strings from the node "binds" property to Docker via the HostConfig.Binds field. This API supports anonymous volumes and the container does correctly start up with an new anonymous volume attached. But the volume itself is missing the "Name" attribute for some reason (docker inspect). This makes it impossible to then access the volume from other containers, like with docker run --volumes-from <existing-container> .... An anonymous volume created with the new Mounts spec is for all intents and purposes equivalent to one created with Binds, except it has the "Name" attribute set to its random ID.

This has worked in the past, so maybe there is a regression in Docker? Anyway by fixing it here we can ensure it will work for all Docker versions while maintaining compatibility with existing containerlab topologies.

As a side note, while I was reviewing the Docker documentation, I noticed my change to add anonymous volume support to containerlab node binds in #1853 mixes up two distinct concepts: Bind mounts and Volumes. In hindsight this feels like a mistake and volumes would be better handled in a new volumes node field. This would also align the syntax with docker-compose. What is the design guideline here?

Containerlab passes all strings from the node "binds" property to Docker via
the HostConfig.Binds field. This API supports anonymous volumes and the
container does correctly start up with an new anonymous volume attached. But
the volume itself is missing the "Name" attribute for some reason (docker
inspect). This makes it impossible the then access the volume from other
containers, like with `docker run --volumes-from <existing-container> ...`.
An anonymous volume created with the new Mounts spec is for all intents and
purposes equivalent to one created with Binds, except it has the "Name"
attribute set to its random ID.

This has worked in the past, so maybe there is a regression in Docker?
Anyway by fixing it here we can ensure it will work for all versions.
@hellt
Copy link
Member

hellt commented Jun 15, 2025

Hi @mzagozen

I would agree that adding volumes as a property is the right approach here instead of overloading binds.
If you want to take this one and make a PR that would be great and it would simplify the logic and separate the concerns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants