From 86d65c41cb05babfc5291873fc71f3497e547308 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 23 May 2023 00:06:04 -0700 Subject: [PATCH 1/5] [cli] refactor common code into util class --- src/client/cli/cmd/common_cli.cpp | 19 +++++++++++++++++++ src/client/cli/cmd/common_cli.h | 1 + src/client/cli/cmd/info.cpp | 23 ----------------------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/client/cli/cmd/common_cli.cpp b/src/client/cli/cmd/common_cli.cpp index 8cf1a27de5..7c260d26f9 100644 --- a/src/client/cli/cmd/common_cli.cpp +++ b/src/client/cli/cmd/common_cli.cpp @@ -76,6 +76,25 @@ mp::InstanceNames cmd::add_instance_names(const mp::ArgParser* parser, const std return instance_names; } +std::vector cmd::add_instance_and_snapshot_names(const mp::ArgParser* parser) +{ + std::vector instance_snapshot_names; + instance_snapshot_names.reserve(parser->positionalArguments().count()); + + for (const auto& arg : parser->positionalArguments()) + { + mp::InstanceSnapshotPair inst_snap_name; + auto index = arg.indexOf('.'); + inst_snap_name.set_instance_name(arg.left(index).toStdString()); + if (index >= 0) + inst_snap_name.set_snapshot_name(arg.right(arg.length() - index - 1).toStdString()); + + instance_snapshot_names.push_back(inst_snap_name); + } + + return instance_snapshot_names; +} + mp::ParseCode cmd::handle_format_option(const mp::ArgParser* parser, mp::Formatter** chosen_formatter, std::ostream& cerr) { diff --git a/src/client/cli/cmd/common_cli.h b/src/client/cli/cmd/common_cli.h index bd5eeea68f..4d8d1a10d2 100644 --- a/src/client/cli/cmd/common_cli.h +++ b/src/client/cli/cmd/common_cli.h @@ -43,6 +43,7 @@ const QString format_option_name{"format"}; ParseCode check_for_name_and_all_option_conflict(const ArgParser* parser, std::ostream& cerr, bool allow_empty = false); InstanceNames add_instance_names(const ArgParser* parser); InstanceNames add_instance_names(const ArgParser* parser, const std::string& default_name); +std::vector add_instance_and_snapshot_names(const ArgParser* parser); ParseCode handle_format_option(const ArgParser* parser, Formatter** chosen_formatter, std::ostream& cerr); std::string instance_action_message_for(const InstanceNames& instance_names, const std::string& action_name); ReturnCode run_cmd(const QStringList& args, const ArgParser* parser, std::ostream& cout, std::ostream& cerr); diff --git a/src/client/cli/cmd/info.cpp b/src/client/cli/cmd/info.cpp index b4cb504d00..34bb7d9143 100644 --- a/src/client/cli/cmd/info.cpp +++ b/src/client/cli/cmd/info.cpp @@ -24,29 +24,6 @@ namespace mp = multipass; namespace cmd = multipass::cmd; -namespace -{ -// TODO@snapshots move this to common_cli once required by other commands -std::vector add_instance_and_snapshot_names(const mp::ArgParser* parser) -{ - std::vector instance_snapshot_names; - instance_snapshot_names.reserve(parser->positionalArguments().count()); - - for (const auto& arg : parser->positionalArguments()) - { - mp::InstanceSnapshotPair inst_snap_name; - auto index = arg.indexOf('.'); - inst_snap_name.set_instance_name(arg.left(index).toStdString()); - if (index >= 0) - inst_snap_name.set_snapshot_name(arg.right(arg.length() - index - 1).toStdString()); - - instance_snapshot_names.push_back(inst_snap_name); - } - - return instance_snapshot_names; -} -} // namespace - mp::ReturnCode cmd::Info::run(mp::ArgParser* parser) { auto ret = parse_args(parser); From 25e64458cd8fb551217180f348cf3fe43079344e Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 23 May 2023 00:26:06 -0700 Subject: [PATCH 2/5] [cli] adapt delete cli to accept snapshots --- src/client/cli/cmd/delete.cpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/client/cli/cmd/delete.cpp b/src/client/cli/cmd/delete.cpp index dd8ce3b7af..c9ffab1448 100644 --- a/src/client/cli/cmd/delete.cpp +++ b/src/client/cli/cmd/delete.cpp @@ -75,24 +75,23 @@ std::string cmd::Delete::name() const QString cmd::Delete::short_help() const { - return QStringLiteral("Delete instances"); + return QStringLiteral("Delete instances and snapshots"); } QString cmd::Delete::description() const { - return QStringLiteral("Delete instances, to be purged with the \"purge\" command,\n" - "or recovered with the \"recover\" command."); + return QStringLiteral("Delete instances and snapshots, to be purged with the \"purge\" command,\n" + "or recovered with the \"recover\" command (instances only)."); } mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser) { - parser->addPositionalArgument("name", "Names of instances to delete", " [ ...]"); + parser->addPositionalArgument("name", "Names of instances and snapshots to delete", + "[.snapshot] [[.snapshot] ...]"); - QCommandLineOption all_option(all_option_name, "Delete all instances"); - parser->addOption(all_option); - - QCommandLineOption purge_option({"p", "purge"}, "Purge instances immediately"); - parser->addOption(purge_option); + QCommandLineOption all_option(all_option_name, "Delete all instances and snapshots"); + QCommandLineOption purge_option({"p", "purge"}, "Purge deleted instances and snapshots immediately"); + parser->addOptions({all_option, purge_option}); auto status = parser->commandParse(this); if (status != ParseCode::Ok) @@ -102,11 +101,21 @@ mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser) if (parse_code != ParseCode::Ok) return parse_code; - request.mutable_instance_names()->CopyFrom(add_instance_names(parser)); + for (const auto& arg : parser->positionalArguments()) + { + if (arg.indexOf('.') >= 0 && !parser->isSet("purge")) + { + cerr << "Snapshots can only be purged (after deletion, they cannot be recovered). Please use the `--purge` " + "flag if that is what you want.\n"; + return mp::ParseCode::CommandLineError; + } + } + + for (const auto& item : add_instance_and_snapshot_names(parser)) + request.add_instances_snapshots()->CopyFrom(item); if (parser->isSet(purge_option)) - { request.set_purge(true); - } + return status; } From 2fae24fc6585b968f83fcb257075df59f448310b Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 23 May 2023 00:26:32 -0700 Subject: [PATCH 3/5] [rpc] add snapshot field to delete rpc msg --- src/rpc/multipass.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index db464aaaf8..5933e7ff8e 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -424,7 +424,7 @@ message RestartReply { } message DeleteRequest { - InstanceNames instance_names = 1; + repeated InstanceSnapshotPair instances_snapshots = 1; bool purge = 2; int32 verbosity_level = 3; } From 5d9ff28b787813bdaed2c7e2f78891c29970b685 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 23 May 2023 00:28:08 -0700 Subject: [PATCH 4/5] [daemon] process adapted delete request and add snapshot delete placeholder code --- src/daemon/daemon.cpp | 47 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 936abddd6d..f27455c37b 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -1160,6 +1160,23 @@ bool is_ipv4_valid(const std::string& ipv4) return true; } +template +std::unordered_map> map_snapshots_to_instances(const Instances& instances) +{ + std::unordered_map> instance_snapshots_map; + + for (const auto& it : instances) + { + if (it.snapshot_name().empty()) + instance_snapshots_map[it.instance_name()]; + else if (const auto& entry = instance_snapshots_map.find(it.instance_name()); + entry == instance_snapshots_map.end() || !entry->second.empty()) + instance_snapshots_map[it.instance_name()].insert(it.snapshot_name()); + } + + return instance_snapshots_map; +} + void add_aliases(google::protobuf::RepeatedPtrField* container, const std::string& remote_name, const mp::VMImageInfo& info, const std::string& default_remote) { @@ -1770,14 +1787,7 @@ try // clang-format on if (status.ok()) { - for (const auto& it : request->instances_snapshots()) - { - if (it.snapshot_name().empty()) - instance_snapshots_map[it.instance_name()]; - else if (const auto& entry = instance_snapshots_map.find(it.instance_name()); - entry == instance_snapshots_map.end() || !entry->second.empty()) - instance_snapshots_map[it.instance_name()].insert(it.snapshot_name()); - } + instance_snapshots_map = map_snapshots_to_instances(request->instances_snapshots()); // TODO@snapshots change cmd logic to include detailed snapshot info output auto cmd = @@ -2245,12 +2255,13 @@ try // clang-format on DeleteReply response; auto [instance_selection, status] = - select_instances_and_react(operative_instances, deleted_instances, request->instance_names().instance_name(), + select_instances_and_react(operative_instances, deleted_instances, request->instances_snapshots(), InstanceGroup::All, require_existing_instances_reaction); if (status.ok()) { const bool purge = request->purge(); + auto instance_snapshots_map = map_snapshots_to_instances(request->instances_snapshots()); for (const auto& vm_it : instance_selection.operative_selection) { @@ -2266,6 +2277,24 @@ try // clang-format on if (purge) { + // TODO@snapshots call method to delete snapshots + /* + if (const auto& it = instance_snapshots_map.find(name); + it == instance_snapshots_map.end() || it.second.empty()) + { + // Delete instance and snapshots + // release_resources(name); + // response.add_purged_instances(name); + } + else + { + for (const auto& snapshot_name : instance_snapshots_map[name]) + { + // Delete snapshot + } + } + */ + release_resources(name); response.add_purged_instances(name); } From 08ee002634976aa7975f24ed82ded6532e274a62 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Tue, 13 Jun 2023 14:53:29 -0700 Subject: [PATCH 5/5] [cli] edit user strings and optimize loops --- src/client/cli/cmd/delete.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/client/cli/cmd/delete.cpp b/src/client/cli/cmd/delete.cpp index c9ffab1448..92a067d9bd 100644 --- a/src/client/cli/cmd/delete.cpp +++ b/src/client/cli/cmd/delete.cpp @@ -80,8 +80,10 @@ QString cmd::Delete::short_help() const QString cmd::Delete::description() const { - return QStringLiteral("Delete instances and snapshots, to be purged with the \"purge\" command,\n" - "or recovered with the \"recover\" command (instances only)."); + return QStringLiteral( + "Delete instances and snapshots. Instances can be purged immediately or later on," + "with the \"purge\" command. Until they are purged, instances can be recovered" + "with the \"recover\" command. Snapshots cannot be recovered after deletion and must be purged at once."); } mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser) @@ -90,7 +92,7 @@ mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser) "[.snapshot] [[.snapshot] ...]"); QCommandLineOption all_option(all_option_name, "Delete all instances and snapshots"); - QCommandLineOption purge_option({"p", "purge"}, "Purge deleted instances and snapshots immediately"); + QCommandLineOption purge_option({"p", "purge"}, "Purge specified instances and snapshots immediately"); parser->addOptions({all_option, purge_option}); auto status = parser->commandParse(this); @@ -101,21 +103,18 @@ mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser) if (parse_code != ParseCode::Ok) return parse_code; - for (const auto& arg : parser->positionalArguments()) + request.set_purge(parser->isSet(purge_option)); + for (const auto& item : add_instance_and_snapshot_names(parser)) { - if (arg.indexOf('.') >= 0 && !parser->isSet("purge")) + if (item.has_snapshot_name() && !request.purge()) { cerr << "Snapshots can only be purged (after deletion, they cannot be recovered). Please use the `--purge` " "flag if that is what you want.\n"; return mp::ParseCode::CommandLineError; } - } - for (const auto& item : add_instance_and_snapshot_names(parser)) request.add_instances_snapshots()->CopyFrom(item); - - if (parser->isSet(purge_option)) - request.set_purge(true); + } return status; }