Merge #619

619: Remove sshfs install from cloud-init r=ricab a=townsend2010

Also move "just in time" sshfs install logic into the daemon.

Fixes #615 

Co-authored-by: Chris Townsend <[email protected]>

Author: bors[bot]

include/multipass/utils.h

@@ -65,8 +65,6 @@ std::string timestamp();
 bool is_running(const VirtualMachine::State& state);
 void wait_until_ssh_up(VirtualMachine* virtual_machine, std::chrono::milliseconds timeout,
                        std::function<void()> const& process_vm_events = []() { });
-void wait_for_cloud_init(VirtualMachine* virtual_machine, std::chrono::milliseconds timeout,
-                         std::function<void()> const& process_vm_events = []() { });
 
 enum class TimeoutAction
 {

include/multipass/virtual_machine.h

@@ -58,7 +58,6 @@ class VirtualMachine
     virtual std::string ipv4() = 0;
     virtual std::string ipv6() = 0;
     virtual void wait_until_ssh_up(std::chrono::milliseconds timeout) = 0;
-    virtual void wait_for_cloud_init(std::chrono::milliseconds timeout) = 0;
     virtual void update_state() = 0;
 
     VirtualMachine::State state;

src/client/cmd/common_cli.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 Canonical, Ltd.
+ * Copyright (C) 2018-2019 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
@@ -102,24 +102,3 @@ mp::ReturnCode cmd::standard_failure_handler_for(const std::string& command, std
 
     return return_code_for(status.error_code());
 }
-
-void cmd::install_sshfs_for(const std::string& instance_name, int verbosity_level, grpc::Channel* rpc_channel,
-                            mp::Rpc::Stub* stub, std::ostream& cout, std::ostream& cerr)
-{
-    std::vector<Command::UPtr> command;
-    command.push_back(std::make_unique<Exec>(*rpc_channel, *stub, cout, cerr));
-
-    auto args = QStringList() << "" // This is just a dummy string for the unnecessary binary name
-                              << "exec" << QString::fromStdString(instance_name) << "--"
-                              << "sudo"
-                              << "bash"
-                              << "-c"
-                              << "apt update && apt install -y sshfs";
-    ArgParser exec_parser{args, command, cout, cerr};
-    exec_parser.setVerbosityLevel(verbosity_level);
-    exec_parser.parse();
-
-    fmt::print(cerr, "The sshfs package is missing in \"{}\". Installing...\n", instance_name);
-    if (exec_parser.chosenCommand()->run(&exec_parser) == mp::ReturnCode::Ok)
-        fmt::print(cerr, "\n***Please re-run the mount command.\n");
-}

src/client/cmd/common_cli.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 Canonical, Ltd.
+ * Copyright (C) 2018-2019 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
@@ -41,8 +41,6 @@ ParseCode handle_format_option(ArgParser* parser, Formatter** chosen_formatter,
 std::string instance_action_message_for(const InstanceNames& instance_names, const std::string& action_name);
 ReturnCode standard_failure_handler_for(const std::string& command, std::ostream& cerr, const grpc::Status& status,
                                         const std::string& error_details = std::string());
-void install_sshfs_for(const std::string& instance_name, int verbosity_level, grpc::Channel* rpc_channel,
-                       Rpc::Stub* stub, std::ostream& cout, std::ostream& cerr);
 } // namespace cmd
 } // namespace multipass
 

src/client/cmd/mount.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -17,8 +17,8 @@
 
 #include "mount.h"
 #include "common_cli.h"
-#include "exec.h"
 
+#include "animated_spinner.h"
 #include <multipass/cli/argparser.h>
 #include <multipass/cli/client_platform.h>
 #include <multipass/logging/log.h>
@@ -59,29 +59,27 @@ mp::ReturnCode cmd::Mount::run(mp::ArgParser* parser)
         return parser->returnCodeFrom(ret);
     }
 
-    auto on_success = [](mp::MountReply& reply) {
+    mp::AnimatedSpinner spinner{cout};
+
+    auto on_success = [&spinner](mp::MountReply& reply) {
+        spinner.stop();
         return ReturnCode::Ok;
     };
 
-    auto on_failure = [this, &parser](grpc::Status& status) {
-        auto ret = standard_failure_handler_for(name(), cerr, status);
-        if (!status.error_details().empty())
-        {
-            mp::MountError mount_error;
-            mount_error.ParseFromString(status.error_details());
+    auto on_failure = [this, &spinner](grpc::Status& status) {
+        spinner.stop();
 
-            if (mount_error.error_code() == mp::MountError::SSHFS_MISSING)
-            {
-                cmd::install_sshfs_for(mount_error.instance_name(), parser->verbosityLevel(), rpc_channel, stub, cout,
-                                       cerr);
-            }
-        }
+        return standard_failure_handler_for(name(), cerr, status);
+    };
 
-        return ret;
+    auto streaming_callback = [&spinner](mp::MountReply& reply) {
+        spinner.stop();
+        spinner.start(reply.mount_message());
     };
 
     request.set_verbosity_level(parser->verbosityLevel());
-    return dispatch(&RpcMethod::mount, request, on_success, on_failure);
+
+    return dispatch(&RpcMethod::mount, request, on_success, on_failure, streaming_callback);
 }
 
 std::string cmd::Mount::name() const { return "mount"; }

src/client/cmd/start.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -37,47 +37,39 @@ mp::ReturnCode cmd::Start::run(mp::ArgParser* parser)
         return parser->returnCodeFrom(ret);
     }
 
-    auto on_success = [](mp::StartReply& reply) {
+    AnimatedSpinner spinner{cout};
+
+    auto on_success = [&spinner](mp::StartReply& reply) {
+        spinner.stop();
         return ReturnCode::Ok;
     };
 
-    AnimatedSpinner spinner{cout};
-    auto on_failure = [this, &spinner, &parser](grpc::Status& status) {
+    auto on_failure = [this, &spinner](grpc::Status& status) {
         spinner.stop();
         auto ret = standard_failure_handler_for(name(), cerr, status);
-        if (!status.error_details().empty())
+        if (status.error_code() == grpc::StatusCode::ABORTED && !status.error_details().empty())
         {
-            if (status.error_code() == grpc::StatusCode::ABORTED)
-            {
-                mp::StartError start_error;
-                start_error.ParseFromString(status.error_details());
-
-                if (start_error.error_code() == mp::StartError::INSTANCE_DELETED)
-                {
-                    fmt::print(cerr,
-                               "Use 'recover' to recover the deleted instance or 'purge' to permanently delete the "
-                               "instance.\n");
-                }
-            }
-            else
+            mp::StartError start_error;
+            start_error.ParseFromString(status.error_details());
+
+            if (start_error.error_code() == mp::StartError::INSTANCE_DELETED)
             {
-                mp::MountError mount_error;
-                mount_error.ParseFromString(status.error_details());
-
-                if (mount_error.error_code() == mp::MountError::SSHFS_MISSING)
-                {
-                    cmd::install_sshfs_for(mount_error.instance_name(), parser->verbosityLevel(), rpc_channel, stub,
-                                           cout, cerr);
-                }
+                fmt::print(cerr, "Use 'recover' to recover the deleted instance or 'purge' to permanently delete the "
+                                 "instance.\n");
             }
         }
 
         return ret;
     };
 
+    auto streaming_callback = [&spinner](mp::StartReply& reply) {
+        spinner.stop();
+        spinner.start(reply.start_message());
+    };
+
     spinner.start(instance_action_message_for(request.instance_names(), "Starting "));
     request.set_verbosity_level(parser->verbosityLevel());
-    return dispatch(&RpcMethod::start, request, on_success, on_failure);
+    return dispatch(&RpcMethod::start, request, on_success, on_failure, streaming_callback);
 }
 
 std::string cmd::Start::name() const { return "start"; }

src/daemon/base_cloud_init_config.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Canonical, Ltd.
+ * Copyright (C) 2017-2019 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
@@ -13,8 +13,6 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Christopher James Halse Rogers <[email protected]>
- *
  */
 #ifndef MULTIPASS_BASE_CLOUD_INIT_CONFIG_H
 #define MULTIPASS_BASE_CLOUD_INIT_CONFIG_H
@@ -27,9 +25,7 @@ constexpr auto base_cloud_init_config = "growpart:\n"
                                         "    ignore_growroot_disabled: false\n"
                                         "users:\n"
                                         "    - default\n"
-                                        "manage_etc_hosts: true\n"
-                                        "packages:\n"
-                                        "    - sshfs\n";
+                                        "manage_etc_hosts: true\n";
 }
 
 #endif // MULTIPASS_BASE_CLOUD_INIT_CONFIG_H

src/daemon/daemon.cpp

@@ -68,6 +68,7 @@ constexpr auto metrics_opt_in_file = "multipassd-send-metrics.yaml";
 constexpr auto reboot_cmd = "sudo reboot";
 constexpr auto up_timeout = 2min; // This may be tweaked as appropriate and used in places that wait for ssh to be up
 constexpr auto stop_ssh_cmd = "sudo systemctl stop ssh";
+constexpr auto max_install_sshfs_retries = 3;
 
 mp::Query query_from(const mp::LaunchRequest* request, const std::string& name)
 {
@@ -144,10 +145,6 @@ void prepare_user_data(YAML::Node& user_data_config, YAML::Node& vendor_config)
     if (users.IsSequence())
         users.push_back("default");
 
-    auto packages = user_data_config["packages"];
-    if (packages.IsSequence())
-        packages.push_back("sshfs");
-
     auto keys = user_data_config["ssh_authorized_keys"];
     if (keys.IsSequence())
         keys.push_back(vendor_config["ssh_authorized_keys"][0]);
@@ -304,11 +301,8 @@ auto validate_create_arguments(const mp::LaunchRequest* request)
 
 auto grpc_status_for_mount_error(const std::string& instance_name)
 {
-    mp::MountError mount_error;
-    mount_error.set_error_code(mp::MountError::SSHFS_MISSING);
-    mount_error.set_instance_name(instance_name);
-
-    return grpc::Status(grpc::StatusCode::FAILED_PRECONDITION, "Mount failed", mount_error.SerializeAsString());
+    return grpc::Status(grpc::StatusCode::FAILED_PRECONDITION,
+                        fmt::format("Error enabling mount support in '{}'", instance_name));
 }
 
 auto grpc_status_for(fmt::memory_buffer& errors)
@@ -757,10 +751,6 @@ try // clang-format on
     vm->start();
     vm->wait_until_ssh_up(std::chrono::minutes(5));
 
-    reply.set_create_message("Waiting for initialization to complete");
-    server->Write(reply);
-    vm->wait_for_cloud_init(std::chrono::minutes(5));
-
     reply.set_vm_instance_name(name);
     server->Write(reply);
 
@@ -1265,7 +1255,18 @@ try // clang-format on
             }
             catch (const mp::SSHFSMissingError&)
             {
-                return grpc_status_for_mount_error(name);
+                try
+                {
+                    MountReply mount_reply;
+                    mount_reply.set_mount_message("Enabling support for mounting");
+                    server->Write(mount_reply);
+                    install_sshfs(vm, name);
+                    start_mount(vm, name, request->source_path(), target_path, gid_map, uid_map);
+                }
+                catch (const mp::SSHFSMissingError&)
+                {
+                    return grpc_status_for_mount_error(name);
+                }
             }
             catch (const std::exception& e)
             {
@@ -1474,7 +1475,18 @@ try // clang-format on
             }
             catch (const mp::SSHFSMissingError&)
             {
-                return grpc_status_for_mount_error(name);
+                try
+                {
+                    StartReply start_reply;
+                    start_reply.set_start_message("Enabling support for mounting");
+                    server->Write(start_reply);
+                    install_sshfs(vm, name);
+                    start_mount(vm, name, source_path, target_path, gid_map, uid_map);
+                }
+                catch (const mp::SSHFSMissingError&)
+                {
+                    return grpc_status_for_mount_error(name);
+                }
             }
             catch (const std::exception& e)
             {
@@ -2038,3 +2050,38 @@ grpc::Status mp::Daemon::cmd_vms(const std::vector<std::string>& tgts, std::func
 
     return grpc::Status::OK;
 }
+
+void mp::Daemon::install_sshfs(const VirtualMachine::UPtr& vm, const std::string& name)
+{
+    auto& key_provider = *config->ssh_key_provider;
+
+    SSHSession session{vm->ssh_hostname(), vm->ssh_port(), vm->ssh_username(), key_provider};
+
+    mpl::log(mpl::Level::info, category, fmt::format("Installing sshfs in \'{}\'", name));
+
+    int retries{0};
+    while (++retries <= max_install_sshfs_retries)
+    {
+        try
+        {
+            auto proc = session.exec("sudo apt update && sudo apt install -y sshfs");
+            if (proc.exit_code(std::chrono::minutes(5)) != 0)
+            {
+                auto error_msg = proc.read_std_error();
+                mpl::log(mpl::Level::warning, category,
+                         fmt::format("Failed to install 'sshfs', error message: '{}'", mp::utils::trim_end(error_msg)));
+            }
+            else
+            {
+                break;
+            }
+        }
+        catch (const mp::ExitlessSSHProcessException&)
+        {
+            mpl::log(mpl::Level::info, category, fmt::format("Timeout while installing 'sshfs' in '{}'", name));
+        }
+    }
+
+    if (retries > max_install_sshfs_retries)
+        throw mp::SSHFSMissingError();
+}

src/daemon/daemon.h

@@ -139,6 +139,7 @@ public slots:
     grpc::Status shutdown_vm(VirtualMachine& vm, const std::chrono::milliseconds delay);
     grpc::Status cancel_vm_shutdown(const VirtualMachine& vm);
     grpc::Status cmd_vms(const std::vector<std::string>& tgts, std::function<grpc::Status(VirtualMachine&)> cmd);
+    void install_sshfs(const VirtualMachine::UPtr& vm, const std::string& name);
 
     std::unique_ptr<const DaemonConfig> config;
     std::unordered_map<std::string, VMSpecs> vm_instance_specs;

src/platform/backends/libvirt/libvirt_virtual_machine.cpp

@@ -384,11 +384,6 @@ void mp::LibVirtVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds time
     mp::utils::wait_until_ssh_up(this, timeout);
 }
 
-void mp::LibVirtVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeout)
-{
-    mp::utils::wait_for_cloud_init(this, timeout);
-}
-
 void mp::LibVirtVirtualMachine::update_state()
 {
     monitor->persist_state_for(vm_name);