Rework mount command string generation

[alexander-smolyakov]

Description:

This PR addresses the issue when the Docker volume is named as undefined if the src argument is not specified in the mount command.

The issue related to the following code:

cli/src/spec-node/singleContainer.ts

Lines 359 to 364 in 7de4011

	const featureMounts = ([] as string[]).concat(
	...[
	...mergedConfig.mounts || [],
	...params.additionalMounts,
	].map(m => ['--mount', typeof m === 'string' ? m : `type=${m.type},src=${m.source},dst=${m.target}`])
	);

When the mount command string is generated, there is no check for the src argument. This results in the src attribute being set to undefined when cast to a string.

To fix this behavior, we need to apply null-check for the src argument and don't pass it to Docker. When the src arg is not passed, Docker generates a random name for the volume.

Repro sample:

{
	"image": "mcr.microsoft.com/devcontainers/base:latest",
	"mounts": [
        {
            "target": "/home/vscode/.vscode-server",
            "type": "volume",
        }
    ]
}

Examples:
Without fix:

node ➜ /workspaces/devcontainers-cli (main) $ docker inspect -f '{{json .Mounts }}' a85f6e52d478

[
	{
		"Type": "bind",
		"Source": "/workspaces/devcontainers-cli",
		"Destination": "/workspaces/devcontainers-cli",
		"Mode": "",
		"RW": true,
		"Propagation": "rprivate"
	},
	{
		"Type": "volume",
		"Name": "undefined",
		"Source": "/var/lib/docker/volumes/undefined/_data",
		"Destination": "/home/vscode/.vscode-server",
		"Driver": "local",
		"Mode": "z",
		"RW": true,
		"Propagation": ""
	}
]

With fix:

node ➜ /workspaces/devcontainers-cli (users/alexander-smolyakov/add_proper_handling_for_src_arg_in_mount_command) $ docker inspect -f '{{json .Mounts }}' 2e77ff5392dc

[
	{
		"Type": "bind",
		"Source": "/workspaces/devcontainers-cli",
		"Destination": "/workspaces/devcontainers-cli",
		"Mode": "",
		"RW": true,
		"Propagation": "rprivate"
	},
	{
		"Type": "volume",
		"Name": "628f29020e5d6a9514614ce0ad90411d2400161b67e07bb8744f39a83a397922",
		"Source": "/var/lib/docker/volumes/628f29020e5d6a9514614ce0ad90411d2400161b67e07bb8744f39a83a397922/_data",
		"Destination": "/home/vscode/.vscode-server",
		"Driver": "local",
		"Mode": "z",
		"RW": true,
		"Propagation": ""
	}
]

Attached related issue:

• Issue: Unnamed docker volumes #478

Changelog:

• Move the mount command generation logic to a separate function;
• Add check if the src argument is undefined;

[samruddhikhandale]

Thanks, can you help add a unit test to validate this fix?

[alexander-smolyakov]

Thanks, can you help add a unit test to validate this fix?

Added test in 7fc4270

[joshspicer]

Looks good to me - thanks @alexander-smolyakov !

[joshspicer]

Here's the spec article on anonymous volumes https://docs.docker.com/storage/volumes/#choose-the--v-or---mount-flag. Not sure why this is particularly useful, I guess so as to not clutter up a container's filesystem between starts/stops?

[chrmarti]

Thanks for looking into this!

• Could you make source optional in the interface:

  cli/src/spec-configuration/containerFeaturesConfiguration.ts

  Line 98 in 9064c70

  source: string;

• Handle the two references to source for Docker Compose:

  cli/src/spec-node/dockerCompose.ts

  Line 546 in 9064c70

  - ${m.source}:${m.target}`).join('')}` : ''}${gpuResources}${volumeMounts.length ? `

We will also need to update the schemas to make source optional: https://github.com/devcontainers/spec/tree/main/schemas

[samruddhikhandale]

Looks great, thank you!

[chrmarti]

Looks good. A few polishing suggestions. Thanks.