Merge pull request #3045 from canonical/populate-snapshot-overview

[snapshots] Populate snapshot overview

include/multipass/virtual_machine.h

@@ -88,6 +88,7 @@ class VirtualMachine : private DisabledCopyMove
 
     using SnapshotVista = std::vector<std::shared_ptr<const Snapshot>>; // using vista to avoid confusion with C++ views
     virtual SnapshotVista view_snapshots() const noexcept = 0;
+    virtual int get_num_snapshots() const noexcept = 0;
     virtual std::shared_ptr<const Snapshot> get_snapshot(const std::string& name) const = 0;
     virtual std::shared_ptr<const Snapshot> take_snapshot(const QDir& snapshot_dir, const VMSpecs& specs,
                                                           const std::string& name, const std::string& comment) = 0;

src/daemon/daemon.cpp

@@ -759,10 +759,17 @@ InstanceSelectionReport select_instances(InstanceTable& operative_instances, Ins
 
         for (const auto& name : names)
         {
-            if (seen_instances.insert(name).second)
+            using T = std::decay_t<decltype(name)>;
+            const std::string* vm_name;
+            if constexpr (std::is_same_v<T, std::string>)
+                vm_name = &name;
+            else
+                vm_name = &name.instance_name();
+
+            if (seen_instances.insert(*vm_name).second)
             {
-                auto trail = find_instance(operative_instances, deleted_instances, name);
-                rank_instance(name, trail, ret);
+                auto trail = find_instance(operative_instances, deleted_instances, *vm_name);
+                rank_instance(*vm_name, trail, ret);
             }
         }
     }
@@ -1606,6 +1613,10 @@ try // clang-format on
     mpl::ClientLogger<InfoReply, InfoRequest> logger{mpl::level_from(request->verbosity_level()), *config->logger,
                                                      server};
     InfoReply response;
+
+    // Need to 'touch' a report in the response so formatters know what to do with an otherwise empty response
+    request->snapshot_overview() ? (void)response.mutable_snapshot_overview()
+                                 : (void)response.mutable_detailed_report();
     bool have_mounts = false;
     bool deleted = false;
     auto fetch_instance_info = [&](VirtualMachine& vm) {
@@ -1639,6 +1650,7 @@ try // clang-format on
             }
         }
 
+        instance_info->set_num_snapshots(vm.get_num_snapshots());
         instance_info->set_image_release(original_release);
         instance_info->set_id(vm_image.id);
 
@@ -1712,36 +1724,70 @@ try // clang-format on
         return grpc::Status::OK;
     };
 
-    // TODO@snapshots retrieve snapshot names to gather info
+    std::unordered_map<std::string, std::unordered_set<std::string>> instance_snapshots_map;
     auto fetch_snapshot_overview = [&](VirtualMachine& vm) {
+        fmt::memory_buffer errors;
         const auto& name = vm.vm_name;
-        auto overview = response.mutable_snapshot_overview()->add_overview();
-        auto fundamentals = overview->mutable_fundamentals();
 
-        overview->set_instance_name(name);
-        fundamentals->set_snapshot_name("snapshot1");
-        fundamentals->set_comment("This is a sample comment");
+        auto get_snapshot_info = [&](std::shared_ptr<const Snapshot> snapshot) {
+            auto overview = response.mutable_snapshot_overview()->add_overview();
+            auto fundamentals = overview->mutable_fundamentals();
 
-        return grpc::Status::OK;
-    };
+            overview->set_instance_name(name);
+            fundamentals->set_snapshot_name(snapshot->get_name());
+            fundamentals->set_parent(snapshot->get_parent_name());
+            fundamentals->set_comment(snapshot->get_comment());
+            // TODO@snapshots populate snapshot creation time once available
+        };
 
-    mp::InstanceNames instance_names;
-    for (const auto& n : request->instances_snapshots())
-        instance_names.add_instance_name(n.instance_name());
+        if (const auto& it = instance_snapshots_map.find(name);
+            it == instance_snapshots_map.end() || it->second.empty())
+        {
+            for (const auto& snapshot : vm.view_snapshots())
+                get_snapshot_info(snapshot);
+        }
+        else
+        {
+            for (const auto& snapshot : it->second)
+            {
+                try
+                {
+                    get_snapshot_info(vm.get_snapshot(snapshot));
+                }
+                catch (const std::out_of_range&)
+                {
+                    add_fmt_to(errors, "snapshot \"{}\" does not exist", snapshot);
+                }
+            }
+        }
+
+        return grpc_status_for(errors);
+    };
 
     auto [instance_selection, status] =
-        select_instances_and_react(operative_instances, deleted_instances, instance_names.instance_name(),
+        select_instances_and_react(operative_instances, deleted_instances, request->instances_snapshots(),
                                    InstanceGroup::All, require_existing_instances_reaction);
 
     if (status.ok())
     {
-        // TODO@snapshots change cmd logic after all info logic paths are added
+        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());
+        }
+
+        // TODO@snapshots change cmd logic to include detailed snapshot info output
         auto cmd =
             request->snapshot_overview() ? std::function(fetch_snapshot_overview) : std::function(fetch_instance_info);
 
-        cmd_vms(instance_selection.operative_selection, cmd);
-        deleted = true;
-        cmd_vms(instance_selection.deleted_selection, cmd);
+        if ((status = cmd_vms(instance_selection.operative_selection, cmd)).ok())
+        {
+            deleted = true;
+            status = cmd_vms(instance_selection.deleted_selection, cmd);
+        }
 
         if (have_mounts && !MP_SETTINGS.get_as<bool>(mp::mounts_key))
             mpl::log(mpl::Level::error, category, "Mounts have been disabled on this instance of Multipass");

src/platform/backends/shared/base_virtual_machine.cpp

@@ -43,8 +43,8 @@ namespace
 constexpr auto snapshot_extension = "snapshot.json";
 constexpr auto head_filename = "snapshot-head";
 constexpr auto count_filename = "snapshot-count";
-constexpr auto index_digits = 4;        // these two go together
-constexpr auto max_snapshots = 1000ull; // replace suffix with uz for size_t in C++23
+constexpr auto index_digits = 4; // these two go together
+constexpr auto max_snapshots = 1000;
 constexpr auto yes_overwrite = true;
 } // namespace
 
@@ -93,7 +93,7 @@ std::vector<std::string> BaseVirtualMachine::get_all_ipv4(const SSHKeyProvider&
     return all_ipv4;
 }
 
-auto multipass::BaseVirtualMachine::view_snapshots() const noexcept -> SnapshotVista
+auto BaseVirtualMachine::view_snapshots() const noexcept -> SnapshotVista
 {
     SnapshotVista ret;
 
@@ -112,7 +112,7 @@ std::shared_ptr<const Snapshot> BaseVirtualMachine::get_snapshot(const std::stri
 }
 
 void BaseVirtualMachine::take_snapshot_rollback_helper(SnapshotMap::iterator it, std::shared_ptr<Snapshot>& old_head,
-                                                       size_t old_count)
+                                                       int old_count)
 {
     if (old_head != head_snapshot)
     {
@@ -188,7 +188,7 @@ void BaseVirtualMachine::load_generic_snapshot_info(const QDir& snapshot_dir)
 {
     try
     {
-        snapshot_count = std::stoull(mpu::contents_of(snapshot_dir.filePath(count_filename)));
+        snapshot_count = std::stoi(mpu::contents_of(snapshot_dir.filePath(count_filename)));
         head_snapshot = snapshots.at(mpu::contents_of(snapshot_dir.filePath(head_filename)));
     }
     catch (FileOpenFailedException&)
@@ -201,7 +201,7 @@ void BaseVirtualMachine::load_generic_snapshot_info(const QDir& snapshot_dir)
 template <typename LockT>
 void BaseVirtualMachine::log_latest_snapshot(LockT lock) const
 {
-    auto num_snapshots = snapshots.size();
+    auto num_snapshots = static_cast<int>(snapshots.size());
     auto parent_name = head_snapshot->get_parent_name();
 
     assert(num_snapshots <= snapshot_count && "can't have more snapshots than were ever taken");

src/platform/backends/shared/base_virtual_machine.h

@@ -54,6 +54,10 @@ class BaseVirtualMachine : public VirtualMachine
     };
 
     SnapshotVista view_snapshots() const noexcept override;
+    inline int get_num_snapshots() const noexcept override
+    {
+        return static_cast<int>(snapshots.size());
+    }
     std::shared_ptr<const Snapshot> get_snapshot(const std::string& name) const override;
 
     // TODO: the VM should know its directory, but that is true of everything in its VMDescription; pulling that from
@@ -80,7 +84,7 @@ class BaseVirtualMachine : public VirtualMachine
     void load_snapshot(const QJsonObject& json);
 
     auto make_take_snapshot_rollback(SnapshotMap::iterator it);
-    void take_snapshot_rollback_helper(SnapshotMap::iterator it, std::shared_ptr<Snapshot>& old_head, size_t old_count);
+    void take_snapshot_rollback_helper(SnapshotMap::iterator it, std::shared_ptr<Snapshot>& old_head, int old_count);
 
     auto make_head_file_rollback(const QString& head_path, QFile& head_file) const;
     void head_file_rollback_helper(const QString& head_path, QFile& head_file, const std::string& old_head,
@@ -99,7 +103,7 @@ class BaseVirtualMachine : public VirtualMachine
 private:
     SnapshotMap snapshots;
     std::shared_ptr<Snapshot> head_snapshot = nullptr;
-    size_t snapshot_count = 0; // tracks the number of snapshots ever taken (regardless or deletes)
+    int snapshot_count = 0; // tracks the number of snapshots ever taken (regardless or deletes)
     mutable std::recursive_mutex snapshot_mutex;
 };
 

tests/mock_virtual_machine.h

@@ -66,6 +66,7 @@ struct MockVirtualMachineT : public T
     MOCK_METHOD(std::unique_ptr<MountHandler>, make_native_mount_handler,
                 (const SSHKeyProvider*, const std::string&, const VMMount&), (override));
     MOCK_METHOD(VirtualMachine::SnapshotVista, view_snapshots, (), (const, override, noexcept));
+    MOCK_METHOD(int, get_num_snapshots, (), (const, override, noexcept));
     MOCK_METHOD(std::shared_ptr<const Snapshot>, get_snapshot, (const std::string&), (const, override));
     MOCK_METHOD(std::shared_ptr<const Snapshot>, take_snapshot,
                 (const QDir&, const VMSpecs&, const std::string&, const std::string&), (override));

tests/stub_virtual_machine.h

@@ -123,6 +123,11 @@ struct StubVirtualMachine final : public multipass::VirtualMachine
         return {};
     }
 
+    int get_num_snapshots() const noexcept override
+    {
+        return 0;
+    }
+
     std::shared_ptr<const Snapshot> get_snapshot(const std::string&) const override
     {
         return {};