Merge #1954

1954: Move get_all_ipv4() from daemon to VirtualMachine r=ricab a=luis4a0 In order to fix #1951, we need to make each backend to handle the request for IP listing. Co-authored-by: Luis Peñaranda <[email protected]>
canonical · Feb 5, 2021 · 9c63746 · 9c63746
1 parent c541aff
commit 9c63746
Show file tree

Hide file tree

Showing 16 changed files with 175 additions and 81 deletions.
diff --git a/include/multipass/utils.h b/include/multipass/utils.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2020 Canonical, Ltd.
+ * Copyright (C) 2017-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -108,6 +108,7 @@ void wait_for_cloud_init(VirtualMachine* virtual_machine, std::chrono::milliseco
                          const SSHKeyProvider& key_provider);
 void install_sshfs_for(const std::string& name, SSHSession& session,
                        const std::chrono::milliseconds timeout = std::chrono::minutes(5));
+std::string run_in_ssh_session(SSHSession& session, const std::string& cmd);
 
 // yaml helpers
 std::string emit_yaml(const YAML::Node& node);

diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2020 Canonical, Ltd.
+ * Copyright (C) 2017-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -26,6 +26,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <vector>
 
 namespace multipass
 {
@@ -64,6 +65,7 @@ class VirtualMachine
     virtual std::string ssh_hostname(std::chrono::milliseconds timeout) = 0;
     virtual std::string ssh_username() = 0;
     virtual std::string management_ipv4() = 0;
+    virtual std::vector<std::string> get_all_ipv4(const SSHKeyProvider& key_provider) = 0;
     virtual std::string ipv6() = 0;
     virtual void wait_until_ssh_up(std::chrono::milliseconds timeout) = 0;
     virtual void ensure_vm_is_running() = 0;

diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp
@@ -674,15 +674,9 @@ auto instances_running(const Instances& instances)
     return false;
 }
 
-mp::SSHProcess exec_and_log(mp::SSHSession& session, const std::string& cmd)
-{
-    mpl::log(mpl::Level::debug, category, fmt::format("Executing {}.", cmd));
-    return session.exec(cmd);
-}
-
 grpc::Status stop_accepting_ssh_connections(mp::SSHSession& session)
 {
-    auto proc = exec_and_log(session, stop_ssh_cmd);
+    auto proc = session.exec(stop_ssh_cmd);
     auto ecode = proc.exit_code();
 
     return ecode == 0 ? grpc::Status::OK
@@ -700,7 +694,7 @@ grpc::Status ssh_reboot(const std::string& hostname, int port, const std::string
     // Otherwise, there would be a race condition, and we would be unable to distinguish whether it had ever been down.
     stop_accepting_ssh_connections(session);
 
-    auto proc = exec_and_log(session, reboot_cmd);
+    auto proc = session.exec(reboot_cmd);
     try
     {
         auto ecode = proc.exit_code();
@@ -840,30 +834,6 @@ std::string generate_unused_mac_address(std::unordered_set<std::string>& s)
                     max_tries, s.size())};
 }
 
-// Executes a given command on the given session. Returns the output of the command, with spaces and feeds trimmed.
-// Caveat emptor: if the command fails, an empty string is returned.
-std::string run_in_vm(mp::SSHSession& session, const std::string& cmd)
-{
-    auto proc = exec_and_log(session, cmd);
-
-    if (proc.exit_code() != 0)
-    {
-        auto error_msg = proc.read_std_error();
-        mpl::log(mpl::Level::warning, category,
-                 fmt::format("failed to run '{}', error message: '{}'", cmd, mp::utils::trim_end(error_msg)));
-        return std::string{};
-    }
-
-    auto output = proc.read_std_output();
-    if (output.empty())
-    {
-        mpl::log(mpl::Level::warning, category, fmt::format("no output after running '{}'", cmd));
-        return std::string{};
-    }
-
-    return mp::utils::trim_end(output);
-}
-
 bool is_ipv4_valid(const std::string& ipv4)
 {
     try
@@ -878,27 +848,6 @@ bool is_ipv4_valid(const std::string& ipv4)
     return true;
 }
 
-std::vector<std::string> get_all_ipv4(mp::SSHSession& session)
-{
-    std::vector<std::string> all_ipv4;
-
-    auto ip_a_output = QString::fromStdString(run_in_vm(session, "ip -brief -family inet address show scope global"));
-
-    QRegularExpression ipv4_re{QStringLiteral("([\\d\\.]+)\\/\\d+\\s*$"), QRegularExpression::MultilineOption};
-
-    QRegularExpressionMatchIterator ip_it = ipv4_re.globalMatch(ip_a_output);
-
-    while (ip_it.hasNext())
-    {
-        auto ip_match = ip_it.next();
-        auto ip = ip_match.captured(1).toStdString();
-
-        all_ipv4.push_back(ip);
-    }
-
-    return all_ipv4;
-}
-
 } // namespace
 
 mp::Daemon::Daemon(std::unique_ptr<const DaemonConfig> the_config)
@@ -1399,25 +1348,27 @@ try // clang-format on
             mp::SSHSession session{vm->ssh_hostname(), vm->ssh_port(), vm_specs.ssh_username,
                                    *config->ssh_key_provider};
 
-            info->set_load(run_in_vm(session, "cat /proc/loadavg | cut -d ' ' -f1-3"));
-            info->set_memory_usage(run_in_vm(session, "free -b | sed '1d;3d' | awk '{printf $3}'"));
-            info->set_memory_total(run_in_vm(session, "free -b | sed '1d;3d' | awk '{printf $2}'"));
-            info->set_disk_usage(
-                run_in_vm(session, "df --output=used `awk '$2 == \"/\" { print $1 }' /proc/mounts` -B1 | sed 1d"));
-            info->set_disk_total(
-                run_in_vm(session, "df --output=size `awk '$2 == \"/\" { print $1 }' /proc/mounts` -B1 | sed 1d"));
+            info->set_load(mpu::run_in_ssh_session(session, "cat /proc/loadavg | cut -d ' ' -f1-3"));
+            info->set_memory_usage(mpu::run_in_ssh_session(session, "free -b | sed '1d;3d' | awk '{printf $3}'"));
+            info->set_memory_total(mpu::run_in_ssh_session(session, "free -b | sed '1d;3d' | awk '{printf $2}'"));
+            info->set_disk_usage(mpu::run_in_ssh_session(
+                session, "df --output=used `awk '$2 == \"/\" { print $1 }' /proc/mounts` -B1 | sed 1d"));
+            info->set_disk_total(mpu::run_in_ssh_session(
+                session, "df --output=size `awk '$2 == \"/\" { print $1 }' /proc/mounts` -B1 | sed 1d"));
 
             std::string management_ip = vm->management_ipv4();
-            auto all_ipv4 = get_all_ipv4(session);
+            auto all_ipv4 = vm->get_all_ipv4(*config->ssh_key_provider);
 
             if (is_ipv4_valid(management_ip))
                 info->add_ipv4(management_ip);
+            else if (all_ipv4.empty())
+                info->add_ipv4("N/A");
 
             for (const auto& extra_ipv4 : all_ipv4)
                 if (extra_ipv4 != management_ip)
                     info->add_ipv4(extra_ipv4);
 
-            auto current_release = run_in_vm(session, "lsb_release -ds");
+            auto current_release = mpu::run_in_ssh_session(session, "lsb_release -ds");
             info->set_current_release(!current_release.empty() ? current_release : original_release);
         }
     }
@@ -1476,10 +1427,12 @@ try // clang-format on
                                    *config->ssh_key_provider};
 
             std::string management_ip = vm->management_ipv4();
-            auto all_ipv4 = get_all_ipv4(session);
+            auto all_ipv4 = vm->get_all_ipv4(*config->ssh_key_provider);
 
             if (is_ipv4_valid(management_ip))
                 entry->add_ipv4(management_ip);
+            else if (all_ipv4.empty())
+                entry->add_ipv4("N/A");
 
             for (const auto& extra_ipv4 : all_ipv4)
                 if (extra_ipv4 != management_ip)

diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2020 Canonical, Ltd.
+ * Copyright (C) 2018-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -242,7 +242,7 @@ bool domain_is_running(virDomainPtr domain, const mp::LibvirtWrapper::UPtr& libv
 mp::LibVirtVirtualMachine::LibVirtVirtualMachine(const mp::VirtualMachineDescription& desc,
                                                  const std::string& bridge_name, mp::VMStatusMonitor& monitor,
                                                  const mp::LibvirtWrapper::UPtr& libvirt_wrapper)
-    : VirtualMachine{desc.vm_name},
+    : BaseVirtualMachine{desc.vm_name},
       username{desc.ssh_username},
       desc{desc},
       monitor{&monitor},

diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.h b/src/platform/backends/libvirt/libvirt_virtual_machine.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2020 Canonical, Ltd.
+ * Copyright (C) 2018-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -20,14 +20,15 @@
 
 #include "libvirt_wrapper.h"
 
-#include <multipass/virtual_machine.h>
+#include <shared/base_virtual_machine.h>
+
 #include <multipass/virtual_machine_description.h>
 
 namespace multipass
 {
 class VMStatusMonitor;
 
-class LibVirtVirtualMachine final : public VirtualMachine
+class LibVirtVirtualMachine final : public BaseVirtualMachine
 {
 public:
     using ConnectionUPtr = std::unique_ptr<virConnect, decltype(virConnectClose)*>;

diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp
@@ -147,7 +147,7 @@ QJsonObject generate_devices_config(const multipass::VirtualMachineDescription&
 mp::LXDVirtualMachine::LXDVirtualMachine(const VirtualMachineDescription& desc, VMStatusMonitor& monitor,
                                          NetworkAccessManager* manager, const QUrl& base_url,
                                          const QString& bridge_name)
-    : VirtualMachine{desc.vm_name},
+    : BaseVirtualMachine{desc.vm_name},
       name{QString::fromStdString(desc.vm_name)},
       username{desc.ssh_username},
       monitor{&monitor},

diff --git a/src/platform/backends/lxd/lxd_virtual_machine.h b/src/platform/backends/lxd/lxd_virtual_machine.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019-2020 Canonical, Ltd.
+ * Copyright (C) 2019-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,15 +22,15 @@
 #include <QString>
 #include <QUrl>
 
-#include <multipass/virtual_machine.h>
+#include <shared/base_virtual_machine.h>
 
 namespace multipass
 {
 class NetworkAccessManager;
 class VirtualMachineDescription;
 class VMStatusMonitor;
 
-class LXDVirtualMachine final : public VirtualMachine
+class LXDVirtualMachine final : public BaseVirtualMachine
 {
 public:
     LXDVirtualMachine(const VirtualMachineDescription& desc, VMStatusMonitor& monitor, NetworkAccessManager* manager,

diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2020 Canonical, Ltd.
+ * Copyright (C) 2017-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -196,7 +196,8 @@ auto generate_metadata(const QStringList& args)
 
 mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc, const std::string& tap_device_name,
                                            DNSMasqServer& dnsmasq_server, VMStatusMonitor& monitor)
-    : VirtualMachine{instance_image_has_snapshot(desc.image.image_path) ? State::suspended : State::off, desc.vm_name},
+    : BaseVirtualMachine{instance_image_has_snapshot(desc.image.image_path) ? State::suspended : State::off,
+                         desc.vm_name},
       tap_device_name{tap_device_name},
       desc{desc},
       mac_addr{desc.default_mac_address},

diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2020 Canonical, Ltd.
+ * Copyright (C) 2017-2021 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -18,8 +18,9 @@
 #ifndef MULTIPASS_QEMU_VIRTUAL_MACHINE_H
 #define MULTIPASS_QEMU_VIRTUAL_MACHINE_H
 
+#include <shared/base_virtual_machine.h>
+
 #include <multipass/process/process.h>
-#include <multipass/virtual_machine.h>
 #include <multipass/virtual_machine_description.h>
 
 #include <QObject>
@@ -30,7 +31,7 @@ namespace multipass
 class DNSMasqServer;
 class VMStatusMonitor;
 
-class QemuVirtualMachine final : public QObject, public VirtualMachine
+class QemuVirtualMachine final : public QObject, public BaseVirtualMachine
 {
     Q_OBJECT
 public:

diff --git a/src/platform/backends/shared/CMakeLists.txt b/src/platform/backends/shared/CMakeLists.txt
@@ -15,6 +15,7 @@
 set (CMAKE_AUTOMOC ON)
 
 add_library(shared STATIC
+  base_virtual_machine.cpp
   sshfs_server_process_spec.cpp
   ${CMAKE_SOURCE_DIR}/include/multipass/process/basic_process.h)
 

diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2021 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "base_virtual_machine.h"
+
+namespace multipass
+{
+
+std::vector<std::string> BaseVirtualMachine::get_all_ipv4(const SSHKeyProvider& key_provider)
+{
+    std::vector<std::string> all_ipv4;
+
+    if (state == State::running)
+    {
+        SSHSession session{ssh_hostname(), ssh_port(), ssh_username(), key_provider};
+
+        auto ip_a_output = QString::fromStdString(
+            mpu::run_in_ssh_session(session, "ip -brief -family inet address show scope global"));
+
+        QRegularExpression ipv4_re{QStringLiteral("([\\d\\.]+)\\/\\d+\\s*$"), QRegularExpression::MultilineOption};
+
+        QRegularExpressionMatchIterator ip_it = ipv4_re.globalMatch(ip_a_output);
+
+        while (ip_it.hasNext())
+        {
+            auto ip_match = ip_it.next();
+            auto ip = ip_match.captured(1).toStdString();
+
+            all_ipv4.push_back(ip);
+        }
+    }
+
+    return all_ipv4;
+}
+
+} // namespace multipass