Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions localization/strings/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -2600,6 +2600,9 @@ On first run, creates the file with all settings commented out at their defaults
<data name="WSLCCLI_VolumeArgDescription" xml:space="preserve">
<value>Bind mount a volume to the container</value>
</data>
<data name="WSLCCLI_RemoveVolumesArgDescription" xml:space="preserve">
<value>Remove anonymous volumes associated with the container</value>
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description explicitly says 'anonymous volumes', but the added E2E test expects a named volume to be removed with --volumes. Please align the description with the actual behavior (either clarify it removes only anonymous volumes, or broaden the text if named volumes are deleted as well).

Suggested change
<value>Remove anonymous volumes associated with the container</value>
<value>Remove volumes associated with the container</value>

Copilot uses AI. Check for mistakes.
</data>
<data name="WSLCCLI_WorkingDirArgDescription" xml:space="preserve">
<value>Working directory inside the container</value>
</data>
Expand Down
1 change: 1 addition & 0 deletions src/windows/wslc/arguments/ArgumentDefinitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ _(Verbose, "verbose", NO_ALIAS, Kind::Flag, L
_(Version, "version", L"v", Kind::Flag, Localization::WSLCCLI_VersionArgDescription()) \
/*_(Virtual, "virtualization", NO_ALIAS, Kind::Value, Localization::WSLCCLI_VirtualArgDescription())*/ \
_(Volume, "volume", L"v", Kind::Value, Localization::WSLCCLI_VolumeArgDescription()) \
_(Volumes, "volumes", L"v", Kind::Flag, Localization::WSLCCLI_RemoveVolumesArgDescription()) \
Comment on lines 100 to +103
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The short alias -v is now assigned to multiple arguments (Version, Volume, and Volumes). Even if parsing is command-scoped, this can become ambiguous (especially if --version is treated as a global flag). Consider removing the short alias from at least one of these or choosing distinct short aliases so -v has a single meaning in any parsing context.

Suggested change
_(Version, "version", L"v", Kind::Flag, Localization::WSLCCLI_VersionArgDescription()) \
/*_(Virtual, "virtualization", NO_ALIAS, Kind::Value, Localization::WSLCCLI_VirtualArgDescription())*/ \
_(Volume, "volume", L"v", Kind::Value, Localization::WSLCCLI_VolumeArgDescription()) \
_(Volumes, "volumes", L"v", Kind::Flag, Localization::WSLCCLI_RemoveVolumesArgDescription()) \
_(Version, "version", NO_ALIAS, Kind::Flag, Localization::WSLCCLI_VersionArgDescription()) \
/*_(Virtual, "virtualization", NO_ALIAS, Kind::Value, Localization::WSLCCLI_VirtualArgDescription())*/ \
_(Volume, "volume", L"v", Kind::Value, Localization::WSLCCLI_VolumeArgDescription()) \
_(Volumes, "volumes", NO_ALIAS, Kind::Flag, Localization::WSLCCLI_RemoveVolumesArgDescription()) \

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this comment comes up a lot by copilot I have created a PR to address this specific issue and ensure we have no collisions in the command definition and guard against one being introduced later.

#40312

_(VolumeName, "volume-name", NO_ALIAS, Kind::Positional, Localization::WSLCCLI_VolumeNameArgDescription()) \
_(WorkDir, "workdir", L"w", Kind::Value, Localization::WSLCCLI_WorkingDirArgDescription()) \
// clang-format on
1 change: 1 addition & 0 deletions src/windows/wslc/commands/ContainerRemoveCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ std::vector<Argument> ContainerRemoveCommand::GetArguments() const
return {
Argument::Create(ArgType::ContainerId, true, NO_LIMIT),
Argument::Create(ArgType::Force),
Argument::Create(ArgType::Volumes),
Argument::Create(ArgType::Session),
};
}
Expand Down
16 changes: 14 additions & 2 deletions src/windows/wslc/services/ContainerService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,11 +397,23 @@ void ContainerService::Kill(Session& session, const std::string& id, WSLCSignal
THROW_IF_FAILED(container->Kill(signal));
}

void ContainerService::Delete(Session& session, const std::string& id, bool force)
void ContainerService::Delete(Session& session, const std::string& id, bool force, bool removeVolumes)
{
wil::com_ptr<IWSLCContainer> container;
THROW_IF_FAILED(session.Get()->OpenContainer(id.c_str(), &container));
THROW_IF_FAILED(container->Delete(force ? WSLCDeleteFlagsForce : WSLCDeleteFlagsNone));

WSLCDeleteFlags flags = WSLCDeleteFlagsNone;
if (force)
{
WI_SetFlag(flags, WSLCDeleteFlagsForce);
}

if (removeVolumes)
{
WI_SetFlag(flags, WSLCDeleteFlagsDeleteVolumes);
}

THROW_IF_FAILED(container->Delete(flags));
}

std::vector<ContainerInformation> ContainerService::List(Session& session)
Expand Down
2 changes: 1 addition & 1 deletion src/windows/wslc/services/ContainerService.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct ContainerService
static int Start(models::Session& session, const std::string& id, bool attach = false);
static void Stop(models::Session& session, const std::string& id, models::StopContainerOptions options);
static void Kill(models::Session& session, const std::string& id, WSLCSignal signal = WSLCSignalSIGKILL);
static void Delete(models::Session& session, const std::string& id, bool force);
static void Delete(models::Session& session, const std::string& id, bool force, bool removeVolumes = false);
static std::vector<models::ContainerInformation> List(models::Session& session);
static int Exec(models::Session& session, const std::string& id, models::ContainerOptions options);
static wsl::windows::common::wslc_schema::InspectContainer Inspect(models::Session& session, const std::string& id);
Expand Down
3 changes: 2 additions & 1 deletion src/windows/wslc/tasks/ContainerTasks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@ void RemoveContainers(CLIExecutionContext& context)
auto& session = context.Data.Get<Data::Session>();
auto containerIds = context.Args.GetAll<ArgType::ContainerId>();
bool force = context.Args.Contains(ArgType::Force);
bool removeVolumes = context.Args.Contains(ArgType::Volumes);
for (const auto& id : containerIds)
{
ContainerService::Delete(session, WideToMultiByte(id), force);
ContainerService::Delete(session, WideToMultiByte(id), force, removeVolumes);
}
}

Expand Down
55 changes: 55 additions & 0 deletions test/windows/wslc/e2e/WSLCE2EContainerRemoveTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class WSLCE2EContainerRemoveTests
{
EnsureContainerDoesNotExist(WslcContainerName);
EnsureContainerDoesNotExist(WslcContainerName2);
EnsureVolumeDoesNotExist(WslcVolumeName);
EnsureImageIsDeleted(DebianImage);
return true;
}
Expand All @@ -41,6 +42,7 @@ class WSLCE2EContainerRemoveTests
{
EnsureContainerDoesNotExist(WslcContainerName);
EnsureContainerDoesNotExist(WslcContainerName2);
EnsureVolumeDoesNotExist(WslcVolumeName);
return true;
}

Expand Down Expand Up @@ -156,9 +158,61 @@ class WSLCE2EContainerRemoveTests
VerifyContainerIsNotListed(WslcContainerName2);
}

WSLC_TEST_METHOD(WSLCE2E_Container_Remove_WithVolumes)
{
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});

auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});

auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());

VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(WslcVolumeName);

// Remove with --volumes should remove the container and its volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});

VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(WslcVolumeName);
Comment on lines +163 to +185
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses a named volume (volume create --name ...) and asserts container remove --volumes deletes it. However, the new help text/description says it removes anonymous volumes. If the intended semantics are 'anonymous only' (Docker-like), this test will be asserting the wrong behavior—switch the test to use an anonymous volume. If the intended semantics are 'delete named volumes too', then the user-facing description should be updated to match.

Suggested change
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});
auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(WslcVolumeName);
// Remove with --volumes should remove the container and its volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(WslcVolumeName);
// Capture the current volume list so we can verify an anonymous volume is created and then removed.
auto result = RunWslc(L"volume list");
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto volumesBeforeCreate = result.Stdout;
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
});
result = RunWslc(
std::format(L"container create --name {} -v /data {}", WslcContainerName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
result = RunWslc(L"volume list");
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto volumesAfterCreate = result.Stdout;
VERIFY_ARE_NOT_EQUAL(volumesBeforeCreate, volumesAfterCreate);
// Remove with --volumes should remove the container and its anonymous volume.
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
result = RunWslc(L"volume list");
result.Verify({.Stderr = L"", .ExitCode = 0});
VERIFY_ARE_EQUAL(volumesBeforeCreate, result.Stdout);

Copilot uses AI. Check for mistakes.
Comment on lines +163 to +185
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses a named volume (volume create --name ...) and asserts container remove --volumes deletes it. However, the new help text/description says it removes anonymous volumes. If the intended semantics are 'anonymous only' (Docker-like), this test will be asserting the wrong behavior—switch the test to use an anonymous volume. If the intended semantics are 'delete named volumes too', then the user-facing description should be updated to match.

Suggested change
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});
auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(WslcVolumeName);
// Remove with --volumes should remove the container and its volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(WslcVolumeName);
// Create a container that uses an anonymous volume
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
});
auto result =
RunWslc(std::format(L"container create --name {} -v /data {}", WslcContainerName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
// Remove with --volumes should remove the container and its anonymous volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);

Copilot uses AI. Check for mistakes.
Comment on lines +163 to +185
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses a named volume (volume create --name ...) and asserts container remove --volumes deletes it. However, the new help text/description says it removes anonymous volumes. If the intended semantics are 'anonymous only' (Docker-like), this test will be asserting the wrong behavior—switch the test to use an anonymous volume. If the intended semantics are 'delete named volumes too', then the user-facing description should be updated to match.

Suggested change
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});
auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(WslcVolumeName);
// Remove with --volumes should remove the container and its volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(WslcVolumeName);
std::wstring anonymousVolumeName;
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
if (!anonymousVolumeName.empty())
{
EnsureVolumeDoesNotExist(anonymousVolumeName);
}
});
// Create a container with an anonymous volume mounted at /data
auto result = RunWslc(std::format(L"container create --name {} -v /data {}", WslcContainerName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
result = RunWslc(std::format(
L"container inspect --format \"{{range .Mounts}}{{if eq .Destination \\\"/data\\\"}}{{.Name}}{{end}}{{end}}\" {}",
containerId));
result.Verify({.Stderr = L"", .ExitCode = 0});
anonymousVolumeName = result.GetStdoutOneLine();
VERIFY_IS_FALSE(anonymousVolumeName.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(anonymousVolumeName);
// Remove with --volumes should remove the container and its anonymous volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(anonymousVolumeName);

Copilot uses AI. Check for mistakes.
Comment on lines +163 to +185
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses a named volume (volume create --name ...) and asserts container remove --volumes deletes it. However, the new help text/description says it removes anonymous volumes. If the intended semantics are 'anonymous only' (Docker-like), this test will be asserting the wrong behavior—switch the test to use an anonymous volume. If the intended semantics are 'delete named volumes too', then the user-facing description should be updated to match.

Suggested change
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});
auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(WslcVolumeName);
// Remove with --volumes should remove the container and its volumes
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(WslcVolumeName);
auto getVolumes = [&]() {
auto listResult = RunWslc(L"volume list --quiet");
listResult.Verify({.Stderr = L"", .ExitCode = 0});
std::vector<std::wstring> volumes;
std::wistringstream output(listResult.Stdout);
std::wstring volumeName;
while (std::getline(output, volumeName))
{
if (!volumeName.empty())
{
volumes.push_back(volumeName);
}
}
return volumes;
};
const auto volumesBeforeCreate = getVolumes();
auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
});
auto result =
RunWslc(std::format(L"container create --name {} -v /data {}", WslcContainerName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());
const auto volumesAfterCreate = getVolumes();
std::wstring anonymousVolumeName;
for (const auto& volumeName : volumesAfterCreate)
{
if (std::find(volumesBeforeCreate.begin(), volumesBeforeCreate.end(), volumeName) == volumesBeforeCreate.end())
{
anonymousVolumeName = volumeName;
break;
}
}
VERIFY_IS_FALSE(anonymousVolumeName.empty());
VerifyContainerIsListed(containerId, L"created");
VerifyVolumeIsListed(anonymousVolumeName);
// Remove with --volumes should remove the container and its anonymous volume
result = RunWslc(std::format(L"container remove --volumes {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
VerifyContainerIsNotListed(containerId);
VerifyVolumeIsNotListed(anonymousVolumeName);

Copilot uses AI. Check for mistakes.
}

WSLC_TEST_METHOD(WSLCE2E_Container_Remove_WithoutVolumesFlag_KeepsVolumes)
{
// Create a named volume and a container that uses it
RunWslc(std::format(L"volume create --name {}", WslcVolumeName)).Verify({.Stderr = L"", .ExitCode = 0});

auto cleanup = wil::scope_exit([&]() {
EnsureContainerDoesNotExist(WslcContainerName);
EnsureVolumeDoesNotExist(WslcVolumeName);
});

auto result = RunWslc(
std::format(L"container create --name {} -v {}:/data {}", WslcContainerName, WslcVolumeName, DebianImage.NameAndTag()));
result.Verify({.Stderr = L"", .ExitCode = 0});
const auto containerId = result.GetStdoutOneLine();
VERIFY_IS_FALSE(containerId.empty());

// Remove without --volumes should keep the volume
result = RunWslc(std::format(L"container remove {}", containerId));
result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});

VerifyContainerIsNotListed(containerId);
VerifyVolumeIsListed(WslcVolumeName);
}

private:
const std::wstring WslcContainerName = L"wslc-test-container";
const std::wstring WslcContainerName2 = L"wslc-test-container-2";
const std::wstring WslcVolumeName = L"wslc-test-rm-volume";
const TestImage& DebianImage = DebianTestImage();

std::wstring GetHelpMessage() const
Expand Down Expand Up @@ -200,6 +254,7 @@ class WSLCE2EContainerRemoveTests
std::wstringstream options;
options << L"The following options are available:\r\n" //
<< L" -f,--force Delete containers even if they are running\r\n"
<< L" -v,--volumes " << Localization::WSLCCLI_RemoveVolumesArgDescription() << L"\r\n"
<< L" --session Specify the session to use\r\n"
<< L" -?,--help Shows help about the selected command\r\n"
<< L"\r\n";
Expand Down
Loading