-
Notifications
You must be signed in to change notification settings - Fork 6
Add automation to catch potential issues with networks, volumes, bind mounts, and ports #269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
case 'volume': | ||
return [...curMounts, { | ||
type: 'volume', | ||
name: mount.Name, | ||
source: mount.Source, | ||
source: mount.Name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a longstanding bug. The description of source
on InspectContainersItemVolumeMount
is The source of the volume mount (volume name)
, we should have been using mount.Name
, not mount.Source
(which is an obscure path deep in the Docker data dirs and not human-friendly).
The same bug existed for the Podman client.
We would have caught this mistake ages ago if we'd used satisfies
here, so I added it to prevent possible future mistakes.
|
||
expect(testContainerId).to.be.a('string'); | ||
expect(await validateContainerExists(client, defaultRunner, { containerId: testContainerId })).to.be.ok; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just moved down into it('RunContainerCommand')
ports: [{ hostPort: 8080, containerPort: 80 }], | ||
exposePorts: [3000], // Uses the `--expose` flag to expose a port without binding it | ||
publishAllPorts: true, // Which will then get bound to a random port on the host, due to this flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importantly, this is currently broken in Docker Desktop 4.42.x, unless you disable IPv6. See microsoft/vscode-containers#155 (comment).
Fixes #267