diff --git a/include/multipass/cli/client_common.h b/include/multipass/cli/client_common.h index 9b515fef09..7d9b2a8203 100644 --- a/include/multipass/cli/client_common.h +++ b/include/multipass/cli/client_common.h @@ -28,6 +28,7 @@ #include #include +#include #include namespace multipass @@ -74,6 +75,8 @@ void set_logger(); void set_logger(multipass::logging::Level verbosity); // full param qualification makes sure msvc is happy void pre_setup(); void post_setup(); +const std::regex yes_answer{"y|yes", std::regex::icase | std::regex::optimize}; +const std::regex no_answer{"n|no", std::regex::icase | std::regex::optimize}; } } // namespace multipass #endif // MULTIPASS_CLIENT_COMMON_H diff --git a/include/multipass/cli/prompters.h b/include/multipass/cli/prompters.h index 3c17a30786..4723bc548f 100644 --- a/include/multipass/cli/prompters.h +++ b/include/multipass/cli/prompters.h @@ -19,6 +19,7 @@ #include #include +#include #ifndef MULTIPASS_CLI_PROMPTERS_H #define MULTIPASS_CLI_PROMPTERS_H @@ -88,6 +89,21 @@ class NewPassphrasePrompter : public PassphrasePrompter std::string prompt(const std::string& text = "Please re-enter passphrase") const override; }; + +class BridgePrompter : private DisabledCopyMove +{ +public: + explicit BridgePrompter(Terminal* term) : term(term){}; + + ~BridgePrompter() = default; + + bool bridge_prompt(const std::vector& nets_need_bridging) const; + +private: + BridgePrompter() = default; + + Terminal* term; +}; } // namespace multipass #endif // MULTIPASS_CLI_PROMPTERS_H diff --git a/include/multipass/exceptions/ssh_exception.h b/include/multipass/exceptions/ssh_exception.h index d6f5253670..5c64c9d00e 100644 --- a/include/multipass/exceptions/ssh_exception.h +++ b/include/multipass/exceptions/ssh_exception.h @@ -30,5 +30,13 @@ class SSHException : public std::runtime_error { } }; + +class SSHExecFailure : public SSHException +{ +public: + SSHExecFailure(const std::string& what_arg) : SSHException(what_arg) + { + } +}; } // namespace multipass #endif // MULTIPASS_SSH_EXCEPTION_H diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index ab08866298..18bfecd1d4 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -20,6 +20,7 @@ #include "disabled_copy_move.h" #include "ip_address.h" +#include "network_interface.h" #include "path.h" #include @@ -83,6 +84,7 @@ class VirtualMachine : private DisabledCopyMove virtual void update_cpus(int num_cores) = 0; virtual void resize_memory(const MemorySize& new_size) = 0; virtual void resize_disk(const MemorySize& new_size) = 0; + virtual void add_network_interface(int index, const NetworkInterface& net) = 0; virtual std::unique_ptr make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) = 0; diff --git a/include/multipass/vm_specs.h b/include/multipass/vm_specs.h index 67c761c990..c920207531 100644 --- a/include/multipass/vm_specs.h +++ b/include/multipass/vm_specs.h @@ -44,6 +44,7 @@ struct VMSpecs std::unordered_map mounts; bool deleted; QJsonObject metadata; + std::vector run_at_boot; }; inline bool operator==(const VMSpecs& a, const VMSpecs& b) @@ -57,16 +58,18 @@ inline bool operator==(const VMSpecs& a, const VMSpecs& b) a.state, a.mounts, a.deleted, - a.metadata) == std::tie(b.num_cores, - b.mem_size, - b.disk_space, - b.default_mac_address, - b.extra_interfaces, - b.ssh_username, - b.state, - b.mounts, - b.deleted, - b.metadata); + a.metadata, + a.run_at_boot) == std::tie(b.num_cores, + b.mem_size, + b.disk_space, + b.default_mac_address, + b.extra_interfaces, + b.ssh_username, + b.state, + b.mounts, + b.deleted, + b.metadata, + b.run_at_boot); } inline bool operator!=(const VMSpecs& a, const VMSpecs& b) // TODO drop in C++20 diff --git a/src/client/cli/cmd/common_cli.h b/src/client/cli/cmd/common_cli.h index 2376471e1c..4d8d1a10d2 100644 --- a/src/client/cli/cmd/common_cli.h +++ b/src/client/cli/cmd/common_cli.h @@ -26,8 +26,6 @@ #include -#include - using RpcMethod = multipass::Rpc::StubInterface; namespace multipass @@ -41,8 +39,6 @@ namespace cmd { const QString all_option_name{"all"}; const QString format_option_name{"format"}; -const std::regex yes_answer{"y|yes", std::regex::icase | std::regex::optimize}; -const std::regex no_answer{"n|no", std::regex::icase | std::regex::optimize}; 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); diff --git a/src/client/cli/cmd/delete.cpp b/src/client/cli/cmd/delete.cpp index 5a2c0c7d9e..9ad9b4c381 100644 --- a/src/client/cli/cmd/delete.cpp +++ b/src/client/cli/cmd/delete.cpp @@ -159,10 +159,11 @@ bool multipass::cmd::Delete::confirm_snapshot_purge() const mp::PlainPrompter prompter{term}; auto answer = prompter.prompt(fmt::format(prompt_text, snapshot_purge_notice_msg)); - while (!answer.empty() && !std::regex_match(answer, yes_answer) && !std::regex_match(answer, no_answer)) + while (!answer.empty() && !std::regex_match(answer, mp::client::yes_answer) && + !std::regex_match(answer, mp::client::no_answer)) answer = prompter.prompt(invalid_input); - return std::regex_match(answer, yes_answer); + return std::regex_match(answer, mp::client::yes_answer); } std::string multipass::cmd::Delete::generate_snapshot_purge_msg() const diff --git a/src/client/cli/cmd/launch.cpp b/src/client/cli/cmd/launch.cpp index 1b0e941367..d5f615d414 100644 --- a/src/client/cli/cmd/launch.cpp +++ b/src/client/cli/cmd/launch.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -52,16 +53,6 @@ namespace fs = std::filesystem; namespace { -constexpr bool on_windows() -{ // TODO when we have remote client-daemon communication, we need to get the daemon's platform - return -#ifdef MULTIPASS_PLATFORM_WINDOWS - true; -#else - false; -#endif -} - auto checked_mode(const std::string& mode) { if (mode == "auto") @@ -618,33 +609,10 @@ auto cmd::Launch::mount(const mp::ArgParser* parser, const QString& mount_source bool cmd::Launch::ask_bridge_permission(multipass::LaunchReply& reply) { - static constexpr auto plural = "Multipass needs to create {} to connect to {}.\nThis will temporarily disrupt " - "connectivity on those interfaces.\n\nDo you want to continue (yes/no)? "; - static constexpr auto singular = "Multipass needs to create a {} to connect to {}.\nThis will temporarily disrupt " - "connectivity on that interface.\n\nDo you want to continue (yes/no)? "; - static constexpr auto nodes = on_windows() ? "switches" : "bridges"; - static constexpr auto node = on_windows() ? "switch" : "bridge"; - - if (term->is_live()) - { - assert(reply.nets_need_bridging_size()); // precondition - if (reply.nets_need_bridging_size() != 1) - fmt::print(cout, plural, nodes, fmt::join(reply.nets_need_bridging(), ", ")); - else - fmt::print(cout, singular, node, reply.nets_need_bridging(0)); + std::vector nets; - while (true) - { - std::string answer; - std::getline(term->cin(), answer); - if (std::regex_match(answer, yes_answer)) - return true; - else if (std::regex_match(answer, no_answer)) - return false; - else - cout << "Please answer yes/no: "; - } - } + std::copy(reply.nets_need_bridging().cbegin(), reply.nets_need_bridging().cend(), std::back_inserter(nets)); - return false; + mp::BridgePrompter prompter(term); + return prompter.bridge_prompt(nets); } diff --git a/src/client/cli/cmd/restore.cpp b/src/client/cli/cmd/restore.cpp index 0a31ffc4db..8b38fa902b 100644 --- a/src/client/cli/cmd/restore.cpp +++ b/src/client/cli/cmd/restore.cpp @@ -142,8 +142,9 @@ bool cmd::Restore::confirm_destruction(const std::string& instance_name) mp::PlainPrompter prompter(term); auto answer = prompter.prompt(fmt::format(prompt_text, instance_name)); - while (!answer.empty() && !std::regex_match(answer, yes_answer) && !std::regex_match(answer, no_answer)) + while (!answer.empty() && !std::regex_match(answer, mp::client::yes_answer) && + !std::regex_match(answer, mp::client::no_answer)) answer = prompter.prompt(invalid_input); - return std::regex_match(answer, no_answer); + return std::regex_match(answer, mp::client::no_answer); } diff --git a/src/client/common/prompters.cpp b/src/client/common/prompters.cpp index d7501af314..bce8c0a577 100644 --- a/src/client/common/prompters.cpp +++ b/src/client/common/prompters.cpp @@ -15,8 +15,10 @@ * */ +#include #include #include +#include #include @@ -34,6 +36,17 @@ auto get_input(std::istream& cin) return value; } + +// TODO when we have remote client-daemon communication, we need to get the daemon's platform +constexpr bool on_windows() +{ + return +#ifdef MULTIPASS_PLATFORM_WINDOWS + true; +#else + false; +#endif +} } // namespace std::string mp::PlainPrompter::prompt(const std::string& text) const @@ -66,3 +79,37 @@ std::string mp::NewPassphrasePrompter::prompt(const std::string& text) const return passphrase; } + +bool mp::BridgePrompter::bridge_prompt(const std::vector& nets_need_bridging) const +{ + assert(nets_need_bridging.size()); // precondition + + static constexpr auto plural = "Multipass needs to create {} to connect to {}.\nThis will temporarily disrupt " + "connectivity on those interfaces.\n\nDo you want to continue (yes/no)? "; + static constexpr auto singular = "Multipass needs to create a {} to connect to {}.\nThis will temporarily disrupt " + "connectivity on that interface.\n\nDo you want to continue (yes/no)? "; + static constexpr auto nodes = on_windows() ? "switches" : "bridges"; + static constexpr auto node = on_windows() ? "switch" : "bridge"; + + if (term->is_live()) + { + if (nets_need_bridging.size() != 1) + fmt::print(term->cout(), plural, nodes, fmt::join(nets_need_bridging, ", ")); + else + fmt::print(term->cout(), singular, node, nets_need_bridging[0]); + + while (true) + { + std::string answer; + std::getline(term->cin(), answer); + if (std::regex_match(answer, mp::client::yes_answer)) + return true; + else if (std::regex_match(answer, mp::client::no_answer)) + return false; + else + term->cout() << "Please answer yes/no: "; + } + } + + return false; +} diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 1709b1a05f..1795e42b4a 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -254,7 +253,8 @@ std::vector read_extra_interfaces(const QJsonObject& recor { auto id = entry.toObject()["id"].toString().toStdString(); auto mac_address = entry.toObject()["mac_address"].toString().toStdString(); - if (!mpu::valid_mac_address(mac_address)) + // Allow empty addresses (for nonconfigured interfaces). + if (!mac_address.empty() && !mpu::valid_mac_address(mac_address)) { throw std::runtime_error(fmt::format("Invalid MAC address {}", mac_address)); } @@ -266,6 +266,22 @@ std::vector read_extra_interfaces(const QJsonObject& recor return extra_interfaces; } +std::vector read_string_vector(const std::string& key, const QJsonObject& record) +{ + std::vector ret; + QString qkey = QString::fromStdString(key); + + if (record.contains(qkey)) + { + for (const auto& entry : record[qkey].toArray()) + { + ret.push_back(entry.toString().toStdString()); + } + } + + return ret; +} + std::unordered_map load_db(const mp::Path& data_path, const mp::Path& cache_path) { QDir data_dir{data_path}; @@ -338,7 +354,8 @@ std::unordered_map load_db(const mp::Path& data_path, static_cast(state), mounts, deleted, - metadata}; + metadata, + read_string_vector("run_at_boot", record)}; } return reconstructed_records; } @@ -359,6 +376,18 @@ QJsonArray to_json_array(const std::vector& extra_interfac return json; } +QJsonArray string_vector_to_json_array(const std::vector& vec) +{ + QJsonArray string_array; + + for (const auto& element : vec) + { + string_array.push_back(QString::fromStdString(element)); + } + + return string_array; +} + QJsonObject vm_spec_to_json(const mp::VMSpecs& specs) { QJsonObject json; @@ -384,6 +413,8 @@ QJsonObject vm_spec_to_json(const mp::VMSpecs& specs) } json.insert("mounts", json_mounts); + json.insert("run_at_boot", string_vector_to_json_array(specs.run_at_boot)); + return json; } @@ -415,6 +446,19 @@ auto try_mem_size(const std::string& val) -> std::optional } } +std::string get_bridged_interface_name() +{ + const auto bridged_id = MP_SETTINGS.get(mp::bridged_interface_key); + + if (bridged_id == "") + { + throw std::runtime_error(fmt::format("You have to `multipass set {}=` to use the \"bridged\" shortcut.", + mp::bridged_interface_key)); + } + + return bridged_id.toStdString(); +} + std::vector validate_extra_interfaces(const mp::LaunchRequest* request, const mp::VirtualMachineFactory& factory, std::vector& nets_need_bridging, @@ -453,12 +497,7 @@ std::vector validate_extra_interfaces(const mp::LaunchRequ if (net_id == mp::bridged_network_name) { - const auto bridged_id = MP_SETTINGS.get(mp::bridged_interface_key); - if (bridged_id == "") - throw std::runtime_error( - fmt::format("You have to `multipass set {}=` to use the `--bridged` shortcut.", - mp::bridged_interface_key)); - net_id = bridged_id.toStdString(); + net_id = get_bridged_interface_name(); } if (!factory_networks) @@ -469,7 +508,7 @@ std::vector validate_extra_interfaces(const mp::LaunchRequ } catch (const mp::NotImplementedOnThisBackendException&) { - throw mp::NotImplementedOnThisBackendException("bridging"); + throw mp::NotImplementedOnThisBackendException("networks"); } } @@ -1058,7 +1097,8 @@ std::unordered_set mac_set_from(const mp::VMSpecs& spec) macs.insert(spec.default_mac_address); for (const auto& extra_iface : spec.extra_interfaces) - macs.insert(extra_iface.mac_address); + if (!extra_iface.mac_address.empty()) + macs.insert(extra_iface.mac_address); return macs; } @@ -1197,13 +1237,17 @@ mp::SettingsHandler* register_instance_mod(std::unordered_map& preparing_instances, - std::function instance_persister) + std::function instance_persister, + std::function bridged_interface, + std::function()> host_networks) { return MP_SETTINGS.register_handler(std::make_unique(vm_instance_specs, operative_instances, deleted_instances, preparing_instances, - std::move(instance_persister))); + std::move(instance_persister), + std::move(bridged_interface), + host_networks)); } mp::SettingsHandler* register_snapshot_mod( @@ -1328,6 +1372,24 @@ void populate_snapshot_info(mp::VirtualMachine& vm, populate_snapshot_fundamentals(snapshot, fundamentals); } +std::string generate_netplan_script(int index, const std::string& mac_address) +{ + return fmt::format("echo -e \"" + "# Generated by Multipass\n" + "network:\n" + " ethernets:\n" + " extra{0}:\n" + " dhcp4: true\n" + " dhcp4-overrides:\n" + " route-metric: 200\n" + " match:\n" + " macaddress: {1}\n" + " optional: true\n" + " version: 2" + "\" | sudo dd of=/etc/netplan/51-multipass-extra{0}.yaml oflag=append conv=notrunc", + index, + mac_address); +} } // namespace mp::Daemon::Daemon(std::unique_ptr the_config) @@ -1336,11 +1398,14 @@ mp::Daemon::Daemon(std::unique_ptr the_config) mp::utils::backend_directory_path(config->data_directory, config->factory->get_backend_directory_name()), mp::utils::backend_directory_path(config->cache_directory, config->factory->get_backend_directory_name()))}, daemon_rpc{config->server_address, *config->cert_provider, config->client_cert_store.get()}, - instance_mod_handler{register_instance_mod(vm_instance_specs, - operative_instances, - deleted_instances, - preparing_instances, - [this] { persist_instances(); })}, + instance_mod_handler{register_instance_mod( + vm_instance_specs, + operative_instances, + deleted_instances, + preparing_instances, + [this] { persist_instances(); }, + get_bridged_interface_name, + [this]() { return config->factory->networks(); })}, snapshot_mod_handler{ register_snapshot_mod(operative_instances, deleted_instances, preparing_instances, *config->factory)} { @@ -1372,7 +1437,10 @@ mp::Daemon::Daemon(std::unique_ptr the_config) // only if this instance is not invalid. auto new_macs = mac_set_from(spec); - if (new_macs.size() <= spec.extra_interfaces.size() || !merge_if_disjoint(new_macs, allocated_mac_addrs)) + if (new_macs.size() <= (size_t)count_if(spec.extra_interfaces.cbegin(), + spec.extra_interfaces.cend(), + [](const auto& i) { return !i.mac_address.empty(); }) || + !merge_if_disjoint(new_macs, allocated_mac_addrs)) { // There is at least one repeated address in new_macs. mpl::log(mpl::Level::warning, category, fmt::format("{} has repeated MAC addresses", name)); @@ -2136,6 +2204,8 @@ try // clang-format on mpl::log(mpl::Level::error, category, "Mounts have been disabled on this instance of Multipass"); } + configure_new_interfaces(name, vm, vm_instance_specs[name]); + vm.start(); } @@ -2801,7 +2871,8 @@ void mp::Daemon::create_vm(const CreateRequest* request, VirtualMachine::State::off, {}, false, - QJsonObject()}; + QJsonObject(), + std::vector{}}; operative_instances[name] = config->factory->create_virtual_machine(vm_desc, *this); preparing_instances.erase(name); @@ -3235,6 +3306,39 @@ mp::MountHandler::UPtr mp::Daemon::make_mount(VirtualMachine* vm, const std::str : vm->make_native_mount_handler(config->ssh_key_provider.get(), target, mount); } +void mp::Daemon::configure_new_interfaces(const std::string& name, mp::VirtualMachine& vm, mp::VMSpecs& specs) +{ + std::vector new_interfaces; + + for (auto i = 0u; i < specs.extra_interfaces.size(); ++i) + { + auto& interface = specs.extra_interfaces[i]; + + if (interface.mac_address.empty()) // An empty MAC address means the interface needs to be configured. + { + new_interfaces.push_back(i); + + interface.mac_address = generate_unused_mac_address(allocated_mac_addrs); + } + } + + if (!new_interfaces.empty()) + { + config->factory->prepare_networking(specs.extra_interfaces); + + auto& commands = specs.run_at_boot; + + for (const auto i : new_interfaces) + { + vm.add_network_interface(i, specs.extra_interfaces[i]); + + commands.push_back(generate_netplan_script(i, specs.extra_interfaces[i].mac_address)); + } + + commands.push_back("sudo netplan apply"); + } +} + QFutureWatcher* mp::Daemon::create_future_watcher(std::function const& finished_op) { @@ -3332,9 +3436,6 @@ mp::Daemon::async_wait_for_ready_all(grpc::ServerReaderWriterInterface& vms, const std::chrono::seconds& timeout, std::promise* status_promise, const std::string& start_errors) { - fmt::memory_buffer errors; - fmt::format_to(std::back_inserter(errors), "{}", start_errors); - QFutureSynchronizer start_synchronizer; { std::lock_guard lock{start_mutex}; @@ -3356,24 +3457,44 @@ mp::Daemon::async_wait_for_ready_all(grpc::ServerReaderWriterInterface lock{start_mutex}; for (const auto& name : vms) { async_running_futures.erase(name); + + run_commands_at_boot_on_instance(name, warnings); } } + fmt::memory_buffer errors; + fmt::format_to(std::back_inserter(errors), "{}", start_errors); + for (const auto& future : start_synchronizer.futures()) if (auto error = future.result(); !error.empty()) add_fmt_to(errors, error); if (server && std::is_same::value) { + bool write_reply{false}; + Reply reply; + if (config->update_prompt->is_time_to_show()) { - Reply reply; config->update_prompt->populate(reply.mutable_update_info()); + write_reply = true; + } + + if (warnings.size() > 0) + { + reply.set_log_line(fmt::to_string(warnings)); + write_reply = true; + } + + if (write_reply) + { server->Write(reply); } } @@ -3510,3 +3631,55 @@ void mp::Daemon::populate_instance_info(VirtualMachine& vm, instance_info->set_current_release(!current_release.empty() ? current_release : original_release); } } + +void mp::Daemon::run_commands_at_boot_on_instance(const std::string& name, fmt::memory_buffer& warnings) +{ + auto& vm_specs = vm_instance_specs[name]; + auto& commands = vm_specs.run_at_boot; + + if (!commands.empty()) + { + bool warned_exec_failure{false}; + + const auto vm = operative_instances[name]; + + try + { + mp::SSHSession session{vm->ssh_hostname(), + vm->ssh_port(), + vm_specs.ssh_username, + *config->ssh_key_provider}; + + for (const auto& command : commands) + { + mpu::run_in_ssh_session(session, command); + } + } + catch (const mp::SSHExecFailure&) // In case there is an error executing the command, report it. + { + // Currently, the only use of running commands at boot is to configure networks. For this reason, + // the warning shown here refers to that use. In the future, in case of using the feature for other + // purposes, it would be necessary to add a description or a failure message to show if the + // execution fails. + if (!warned_exec_failure) + { + add_fmt_to(warnings, + "Failure configuring network interfaces in {}: is Netplan installed?\n" + "You can still configure them manually.\n", + name); + } + + warned_exec_failure = true; + } + catch (const SSHException&) // The SSH session could not be created. + { + add_fmt_to(warnings, + "Cannot create a SSH shell to execute commands on {}, you can configure new\n" + "interfaces manually via Netplan once logged in to the instance.\n", + name); + } + + commands.clear(); + persist_instances(); + } +} diff --git a/src/daemon/daemon.h b/src/daemon/daemon.h index 790b1c87fa..b19d791ed3 100644 --- a/src/daemon/daemon.h +++ b/src/daemon/daemon.h @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -171,6 +172,7 @@ public slots: VirtualMachine* vm); MountHandler::UPtr make_mount(VirtualMachine* vm, const std::string& target, const VMMount& mount); + void configure_new_interfaces(const std::string& name, VirtualMachine& vm, VMSpecs& specs); struct AsyncOperationStatus { @@ -199,6 +201,8 @@ public slots: void populate_instance_info(VirtualMachine& vm, InfoReply& response, bool runtime_info, bool deleted, bool& have_mounts); + void run_commands_at_boot_on_instance(const std::string& name, fmt::memory_buffer& warnings); + std::unique_ptr config; std::unordered_map vm_instance_specs; InstanceTable operative_instances; diff --git a/src/daemon/instance_settings_handler.cpp b/src/daemon/instance_settings_handler.cpp index 614068f747..e5ff0e9eb3 100644 --- a/src/daemon/instance_settings_handler.cpp +++ b/src/daemon/instance_settings_handler.cpp @@ -17,8 +17,11 @@ #include "instance_settings_handler.h" +#include #include #include +#include +#include #include #include @@ -30,6 +33,7 @@ namespace constexpr auto cpus_suffix = "cpus"; constexpr auto mem_suffix = "memory"; constexpr auto disk_suffix = "disk"; +constexpr auto bridged_suffix = "bridged"; enum class Operation { @@ -46,7 +50,7 @@ QRegularExpression make_key_regex() { const auto instance_pattern = QStringLiteral("(?.+)"); const auto prop_template = QStringLiteral("(?%1)"); - const auto either_prop = QStringList{cpus_suffix, mem_suffix, disk_suffix}.join("|"); + const auto either_prop = QStringList{cpus_suffix, mem_suffix, disk_suffix, bridged_suffix}.join("|"); const auto prop_pattern = prop_template.arg(either_prop); const auto key_template = QStringLiteral(R"(%1\.%2\.%3)"); @@ -153,6 +157,56 @@ void update_disk(const QString& key, const QString& val, mp::VirtualMachine& ins } } +bool is_bridged(const mp::VMSpecs& spec, const std::string& br_interface) +{ + return std::any_of(spec.extra_interfaces.cbegin(), + spec.extra_interfaces.cend(), + [&br_interface](const auto& network) -> bool { return network.id == br_interface; }); +} + +void checked_update_bridged(const QString& key, + const QString& val, + mp::VirtualMachine& instance, + mp::VMSpecs& spec, + const std::string& br_interface) +{ + auto bridged = mp::BoolSettingSpec{key, "false"}.interpret(val) == "true"; + + if (!bridged && is_bridged(spec, br_interface)) + { + throw mp::InvalidSettingException{key, val, "Bridged interface cannot be removed"}; + } + + if (bridged) + { + // The empty string in the MAC indicates the daemon that the interface must be configured. + spec.extra_interfaces.push_back({br_interface, "", true}); + } +} + +void update_bridged(const QString& key, + const QString& val, + mp::VirtualMachine& instance, + mp::VMSpecs& spec, + std::function bridged_interface, + std::function()> host_networks) +{ + const auto& host_nets = host_networks(); // This will throw if not implemented on this backend. + const auto& br = bridged_interface(); + const auto& info = std::find_if(host_nets.cbegin(), host_nets.cend(), [br](const auto& i) { return i.id == br; }); + + if (info == host_nets.cend()) + { + throw std::runtime_error( + fmt::format("Invalid network '{}' set as bridged interface, use `multipass set {}=` to " + "correct. See `multipass networks` for valid names.", + br, + mp::bridged_interface_key)); + } + + checked_update_bridged(key, val, instance, spec, br); +} + } // namespace mp::InstanceSettingsException::InstanceSettingsException(const std::string& reason, const std::string& instance, @@ -166,12 +220,16 @@ mp::InstanceSettingsHandler::InstanceSettingsHandler( std::unordered_map& operative_instances, const std::unordered_map& deleted_instances, const std::unordered_set& preparing_instances, - std::function instance_persister) + std::function instance_persister, + std::function bridged_interface, + std::function()> host_networks) : vm_instance_specs{vm_instance_specs}, operative_instances{operative_instances}, deleted_instances{deleted_instances}, preparing_instances{preparing_instances}, - instance_persister{std::move(instance_persister)} + instance_persister{std::move(instance_persister)}, + bridged_interface{std::move(bridged_interface)}, + host_networks{std::move(host_networks)} { } @@ -181,8 +239,8 @@ std::set mp::InstanceSettingsHandler::keys() const std::set ret; for (const auto& item : vm_instance_specs) - for (const auto* suffix : {cpus_suffix, mem_suffix, disk_suffix}) - ret.insert(key_template.arg(item.first.c_str(), suffix)); + for (const auto& suffix : {cpus_suffix, mem_suffix, disk_suffix, bridged_suffix}) + ret.insert(key_template.arg(item.first.c_str()).arg(suffix)); return ret; } @@ -192,6 +250,8 @@ QString mp::InstanceSettingsHandler::get(const QString& key) const auto [instance_name, property] = parse_key(key); const auto& spec = find_spec(instance_name); + if (property == bridged_suffix) + return is_bridged(spec, bridged_interface()) ? "true" : "false"; if (property == cpus_suffix) return QString::number(spec.num_cores); if (property == mem_suffix) @@ -215,6 +275,10 @@ void mp::InstanceSettingsHandler::set(const QString& key, const QString& val) if (property == cpus_suffix) update_cpus(key, val, instance, spec); + else if (property == bridged_suffix) + { + update_bridged(key, val, instance, spec, bridged_interface, host_networks); + } else { auto size = get_memory_size(key, val); diff --git a/src/daemon/instance_settings_handler.h b/src/daemon/instance_settings_handler.h index d2236be533..48a1cdfb63 100644 --- a/src/daemon/instance_settings_handler.h +++ b/src/daemon/instance_settings_handler.h @@ -19,6 +19,7 @@ #define MULTIPASS_INSTANCE_SETTINGS_HANDLER_H #include +#include #include #include #include @@ -41,7 +42,9 @@ class InstanceSettingsHandler : public SettingsHandler std::unordered_map& operative_instances, const std::unordered_map& deleted_instances, const std::unordered_set& preparing_instances, - std::function instance_persister); + std::function instance_persister, + std::function bridged_interface, + std::function()> host_networks); std::set keys() const override; QString get(const QString& key) const override; @@ -59,6 +62,8 @@ class InstanceSettingsHandler : public SettingsHandler const std::unordered_map& deleted_instances; const std::unordered_set& preparing_instances; std::function instance_persister; + std::function bridged_interface; + std::function()> host_networks; }; class InstanceSettingsException : public SettingsException diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 1d3d3f5142..fc663374c5 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -126,29 +126,30 @@ QJsonObject generate_base_vm_config(const multipass::VirtualMachineDescription& return config; } +QJsonObject create_bridged_interface_json(const QString& name, const QString& parent, const QString& mac) +{ + return QJsonObject{{"name", name}, {"nictype", "bridged"}, {"parent", parent}, {"type", "nic"}, {"hwaddr", mac}}; +} + QJsonObject generate_devices_config(const multipass::VirtualMachineDescription& desc, const QString& default_mac_addr, const QString& storage_pool) { QJsonObject devices{{"config", QJsonObject{{"source", "cloud-init:config"}, {"type", "disk"}}}, - {"root", QJsonObject{{"path", "/"}, - {"pool", storage_pool}, - {"size", QString::number(desc.disk_space.in_bytes())}, - {"type", "disk"}}}, - {"eth0", QJsonObject{{"name", "eth0"}, - {"nictype", "bridged"}, - {"parent", "mpbr0"}, - {"type", "nic"}, - {"hwaddr", default_mac_addr}}}}; + {"root", + QJsonObject{{"path", "/"}, + {"pool", storage_pool}, + {"size", QString::number(desc.disk_space.in_bytes())}, + {"type", "disk"}}}, + {"eth0", create_bridged_interface_json("eth0", "mpbr0", default_mac_addr)}}; for (auto i = 0u; i < desc.extra_interfaces.size();) { const auto& net = desc.extra_interfaces[i]; auto net_name = QStringLiteral("eth%1").arg(++i); - devices.insert(net_name, QJsonObject{{"name", net_name}, - {"nictype", "bridged"}, - {"parent", QString::fromStdString(net.id)}, - {"type", "nic"}, - {"hwaddr", QString::fromStdString(net.mac_address)}}); + devices.insert(net_name, + create_bridged_interface_json(net_name, + QString::fromStdString(net.id), + QString::fromStdString(net.mac_address))); } return devices; @@ -475,6 +476,20 @@ void mp::LXDVirtualMachine::resize_disk(const MemorySize& new_size) lxd_request(manager, "PATCH", url(), patch_json); } +void mp::LXDVirtualMachine::add_network_interface(int index, const mp::NetworkInterface& net) +{ + assert(manager); + + auto net_name = QStringLiteral("eth%1").arg(index + 1); + auto net_config = create_bridged_interface_json(net_name, + QString::fromStdString(net.id), + QString::fromStdString(net.mac_address)); + + QJsonObject patch_json{{"devices", QJsonObject{{net_name, net_config}}}}; + + lxd_request(manager, "PATCH", url(), patch_json); +} + std::unique_ptr mp::LXDVirtualMachine::make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.h b/src/platform/backends/lxd/lxd_virtual_machine.h index 5f224a88ce..ba31b07703 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.h +++ b/src/platform/backends/lxd/lxd_virtual_machine.h @@ -57,6 +57,7 @@ class LXDVirtualMachine : public BaseVirtualMachine void update_cpus(int num_cores) override; void resize_memory(const MemorySize& new_size) override; void resize_disk(const MemorySize& new_size) override; + void add_network_interface(int index, const NetworkInterface& net) override; std::unique_ptr make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) override; diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail.h b/src/platform/backends/qemu/linux/qemu_platform_detail.h index 8c59476e06..3aa7fd0638 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail.h +++ b/src/platform/backends/qemu/linux/qemu_platform_detail.h @@ -41,6 +41,7 @@ class QemuPlatformDetail : public QemuPlatform void remove_resources_for(const std::string& name) override; void platform_health_check() override; QStringList vm_platform_args(const VirtualMachineDescription& vm_desc) override; + void add_network_interface(VirtualMachineDescription& desc, const NetworkInterface& net) override; private: const QString bridge_name; diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp index dc6fd6dc81..3eb1d67d69 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp +++ b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp @@ -195,6 +195,17 @@ QStringList mp::QemuPlatformDetail::vm_platform_args(const VirtualMachineDescrip return opts; } +void mp::QemuPlatformDetail::add_network_interface(VirtualMachineDescription& desc, const NetworkInterface& net) +{ + // TODO: Do not uncomment the following line when implementing bridging in Linux QEMU. Since implementing it would + // yield the same exact implementation than in macOS, we need to move the code out of here, and put it only once + // in src/platform/backends/qemu/qemu_virtual_machine.[h|cpp]. + // desc.extra_interfaces.push_back(net); + + // For the time being, just complain: + throw NotImplementedOnThisBackendException("add interfaces"); +} + mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform(const Path& data_dir) const { return std::make_unique(data_dir); diff --git a/src/platform/backends/qemu/qemu_platform.h b/src/platform/backends/qemu/qemu_platform.h index ae8f37d049..d4c2c2695a 100644 --- a/src/platform/backends/qemu/qemu_platform.h +++ b/src/platform/backends/qemu/qemu_platform.h @@ -58,6 +58,7 @@ class QemuPlatform : private DisabledCopyMove { throw NotImplementedOnThisBackendException("networks"); }; + virtual void add_network_interface(VirtualMachineDescription& desc, const NetworkInterface& net) = 0; protected: explicit QemuPlatform() = default; diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index b34e94918d..00a1418241 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -609,6 +609,11 @@ void mp::QemuVirtualMachine::resize_disk(const MemorySize& new_size) desc.disk_space = new_size; } +void mp::QemuVirtualMachine::add_network_interface(int /* not used on this backend */, const NetworkInterface& net) +{ + return qemu_platform->add_network_interface(desc, net); +} + mp::MountHandler::UPtr mp::QemuVirtualMachine::make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index a795ba241a..30b8240830 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.h +++ b/src/platform/backends/qemu/qemu_virtual_machine.h @@ -22,6 +22,7 @@ #include +#include #include #include @@ -63,6 +64,7 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine void update_cpus(int num_cores) override; void resize_memory(const MemorySize& new_size) override; void resize_disk(const MemorySize& new_size) override; + virtual void add_network_interface(int index, const NetworkInterface& net) override; virtual MountArgs& modifiable_mount_args(); std::unique_ptr make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) override; diff --git a/src/platform/backends/shared/base_virtual_machine.h b/src/platform/backends/shared/base_virtual_machine.h index 46696db4b6..7ccf5deb92 100644 --- a/src/platform/backends/shared/base_virtual_machine.h +++ b/src/platform/backends/shared/base_virtual_machine.h @@ -43,6 +43,10 @@ class BaseVirtualMachine : public VirtualMachine BaseVirtualMachine(const std::string& vm_name, const Path& instance_dir); std::vector get_all_ipv4(const SSHKeyProvider& key_provider) override; + void add_network_interface(int index, const NetworkInterface& net) override + { + throw NotImplementedOnThisBackendException("networks"); + } std::unique_ptr make_native_mount_handler(const SSHKeyProvider* ssh_key_provider, const std::string& target, const VMMount& mount) override diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index 0eb1a158ae..dfe415dd41 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -371,10 +371,9 @@ std::string mp::utils::run_in_ssh_session(mp::SSHSession& session, const std::st 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))); - throw std::runtime_error(mp::utils::trim_end(error_msg)); + auto error_msg = mp::utils::trim_end(proc.read_std_error()); + mpl::log(mpl::Level::warning, category, fmt::format("failed to run '{}', error message: '{}'", cmd, error_msg)); + throw mp::SSHExecFailure(error_msg); } auto output = proc.read_std_output(); diff --git a/tests/daemon_test_fixture.cpp b/tests/daemon_test_fixture.cpp index 3a1e742162..81cce02338 100644 --- a/tests/daemon_test_fixture.cpp +++ b/tests/daemon_test_fixture.cpp @@ -15,7 +15,9 @@ * */ +// This header contains premock code so it must be included first. #include "daemon_test_fixture.h" + #include "common.h" #include "file_operations.h" #include "mock_cert_provider.h" diff --git a/tests/daemon_test_fixture.h b/tests/daemon_test_fixture.h index c6122c81f3..18328370eb 100644 --- a/tests/daemon_test_fixture.h +++ b/tests/daemon_test_fixture.h @@ -18,6 +18,9 @@ #ifndef MULTIPASS_DAEMON_TEST_FIXTURE_H #define MULTIPASS_DAEMON_TEST_FIXTURE_H +// This include must go first because it comes from premock. +#include "mock_ssh_test_fixture.h" + #include "mock_virtual_machine_factory.h" #include "temp_dir.h" @@ -90,6 +93,7 @@ struct DaemonTestFixture : public ::Test template grpc::Status call_daemon_slot(Daemon& daemon, DaemonSlotPtr slot, const Request& request, Server&& server); + MockSSHTestFixture mock_ssh_test_fixture; #ifdef MULTIPASS_PLATFORM_WINDOWS std::string server_address{"localhost:50051"}; #else diff --git a/tests/lxd/test_lxd_backend.cpp b/tests/lxd/test_lxd_backend.cpp index 77641ed4b9..ecae440869 100644 --- a/tests/lxd/test_lxd_backend.cpp +++ b/tests/lxd/test_lxd_backend.cpp @@ -2309,3 +2309,49 @@ TEST_F(LXDBackend, createsBridgesViaBackendUtils) EXPECT_CALL(*mock_backend, create_bridge_with(net.id)).WillOnce(Return(bridge)); EXPECT_EQ(factory.create_bridge_with(net), bridge); } + +TEST_F(LXDBackend, addsNetworkInterface) +{ + mpt::StubVMStatusMonitor stub_monitor; + unsigned times_called = 0; + + EXPECT_CALL(*mock_network_access_manager, createRequest(_, _, _)) + .WillRepeatedly([×_called](auto, auto request, auto outgoingData) { + outgoingData->open(QIODevice::ReadOnly); + auto data = outgoingData->readAll(); + auto op = request.attribute(QNetworkRequest::CustomVerbAttribute).toString(); + auto url = request.url().toString(); + + if (op == "GET" && url.contains("1.0/virtual-machines/pied-piper-valley/state")) + { + return new mpt::MockLocalSocketReply(mpt::vm_state_fully_running_data); + } + else if (op == "PUT" && url.contains("1.0/virtual-machines/pied-piper-valley/state") && + data.contains("stop")) + { + return new mpt::MockLocalSocketReply(mpt::stop_vm_data); + } + else if (op == "PATCH") + { + ++times_called; + + EXPECT_EQ(data.toStdString(), + "{\"devices\":{\"eth2\":{\"hwaddr\":\"52:54:00:56:78:90\",\"name\":" + "\"eth2\",\"nictype\":\"bridged\",\"parent\":\"id\",\"type\":\"nic\"}}}"); + + return new mpt::MockLocalSocketReply(mpt::stop_vm_data); + } + + return new mpt::MockLocalSocketReply(mpt::not_found_data, QNetworkReply::ContentNotFoundError); + }); + + mp::LXDVirtualMachineFactory backend{std::move(mock_network_access_manager), data_dir.path(), base_url}; + + auto machine = backend.create_virtual_machine(default_description, stub_monitor); + + machine->stop(); + + machine->add_network_interface(1, {"id", "52:54:00:56:78:90", true}); + + EXPECT_EQ(times_called, 1u); +} diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index c124000710..eb596d6f69 100644 --- a/tests/mock_virtual_machine.h +++ b/tests/mock_virtual_machine.h @@ -71,6 +71,7 @@ struct MockVirtualMachineT : public T MOCK_METHOD(void, update_cpus, (int), (override)); MOCK_METHOD(void, resize_memory, (const MemorySize&), (override)); MOCK_METHOD(void, resize_disk, (const MemorySize&), (override)); + MOCK_METHOD(void, add_network_interface, (int, const NetworkInterface&), (override)); MOCK_METHOD(std::unique_ptr, make_native_mount_handler, (const SSHKeyProvider*, const std::string&, const VMMount&), diff --git a/tests/qemu/linux/test_qemu_platform_detail.cpp b/tests/qemu/linux/test_qemu_platform_detail.cpp index 113e77cf3f..7854160103 100644 --- a/tests/qemu/linux/test_qemu_platform_detail.cpp +++ b/tests/qemu/linux/test_qemu_platform_detail.cpp @@ -208,3 +208,12 @@ TEST_F(QemuPlatformDetail, writing_ipforward_file_failure_logs_expected_message) mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; } + +TEST_F(QemuPlatformDetail, add_network_interface_throws) +{ + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + + mp::VirtualMachineDescription desc; + mp::NetworkInterface net{"id", "52:54:00:98:76:54", true}; + EXPECT_THROW(qemu_platform_detail.add_network_interface(desc, net), mp::NotImplementedOnThisBackendException); +} diff --git a/tests/qemu/mock_qemu_platform.h b/tests/qemu/mock_qemu_platform.h index 8a84e6b5fe..5e39615e1c 100644 --- a/tests/qemu/mock_qemu_platform.h +++ b/tests/qemu/mock_qemu_platform.h @@ -42,6 +42,7 @@ struct MockQemuPlatform : public QemuPlatform MOCK_METHOD(QStringList, vmstate_platform_args, (), (override)); MOCK_METHOD(QStringList, vm_platform_args, (const VirtualMachineDescription&), (override)); MOCK_METHOD(QString, get_directory_name, (), (const, override)); + MOCK_METHOD(void, add_network_interface, (VirtualMachineDescription&, const NetworkInterface&), (override)); }; struct MockQemuPlatformFactory : public QemuPlatformFactory diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index 1cc193780b..4c316b06b7 100644 --- a/tests/stub_virtual_machine.h +++ b/tests/stub_virtual_machine.h @@ -119,6 +119,10 @@ struct StubVirtualMachine final : public multipass::VirtualMachine { } + void add_network_interface(int, const NetworkInterface&) override + { + } + std::unique_ptr make_native_mount_handler(const SSHKeyProvider*, const std::string&, const VMMount&) override diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 06e5907911..917834ab10 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -169,6 +169,15 @@ TEST_F(BaseVM, get_all_ipv4_works_when_instance_is_off) EXPECT_EQ(base_vm.get_all_ipv4(key_provider).size(), 0u); } +TEST_F(BaseVM, add_network_interface_throws) +{ + StubBaseVirtualMachine base_vm(mp::VirtualMachine::State::off); + + MP_EXPECT_THROW_THAT(base_vm.add_network_interface(1, {"eth1", "52:54:00:00:00:00", true}), + mp::NotImplementedOnThisBackendException, + mpt::match_what(HasSubstr("networks"))); +} + struct IpTestParams { int exit_status; diff --git a/tests/test_cli_prompters.cpp b/tests/test_cli_prompters.cpp index 1e8143a255..411e3e3a5f 100644 --- a/tests/test_cli_prompters.cpp +++ b/tests/test_cli_prompters.cpp @@ -164,3 +164,75 @@ TEST_P(CLIPromptersBadCinState, PlainThrows) INSTANTIATE_TEST_SUITE_P(CLIPrompters, CLIPromptersBadCinState, Values(std::ios::eofbit, std::ios::failbit, std::ios::badbit)); + +class BridgePrompterTests : public CLIPrompters, + public WithParamInterface, std::string, bool>> +{ +}; + +TEST_F(CLIPrompters, failsIfNoNetworks) +{ + std::vector nets{}; + + mpt::MockTerminal mock_terminal; + + mp::BridgePrompter prompter{&mock_terminal}; + + EXPECT_CALL(mock_terminal, cin_is_live()).WillRepeatedly(Return(false)); + EXPECT_CALL(mock_terminal, cout_is_live()).WillRepeatedly(Return(false)); + + ASSERT_DEBUG_DEATH(prompter.bridge_prompt(nets), "[Aa]ssert"); +} + +TEST_P(BridgePrompterTests, correctlyReturns) +{ + auto [nets, answer, ret] = GetParam(); + + mpt::MockTerminal mock_terminal; + EXPECT_CALL(mock_terminal, cout()).WillRepeatedly(ReturnRef(cout)); + EXPECT_CALL(mock_terminal, cout_is_live()).WillOnce(Return(true)); + EXPECT_CALL(mock_terminal, cin()).WillOnce(ReturnRef(cin)); + EXPECT_CALL(mock_terminal, cin_is_live()).WillOnce(Return(true)); + + cin.str(answer + "\n"); + + mp::BridgePrompter prompter{&mock_terminal}; + + EXPECT_EQ(prompter.bridge_prompt(nets), ret); +} + +INSTANTIATE_TEST_SUITE_P(CLIPrompters, + BridgePrompterTests, + Values(std::make_tuple(std::vector{"eth1"}, "yes", true), + std::make_tuple(std::vector{"eth1", "eth3"}, "y", true), + std::make_tuple(std::vector{"eth1", "eth3"}, "no", false), + std::make_tuple(std::vector{"eth1"}, "n", false))); + +TEST_F(CLIPrompters, handlesWrongAnswer) +{ + mpt::MockTerminal mock_terminal; + EXPECT_CALL(mock_terminal, cout()).WillRepeatedly(ReturnRef(cout)); + EXPECT_CALL(mock_terminal, cout_is_live()).WillOnce(Return(true)); + EXPECT_CALL(mock_terminal, cin()).WillRepeatedly(ReturnRef(cin)); + EXPECT_CALL(mock_terminal, cin_is_live()).WillOnce(Return(true)); + + cin.str("qqq\nyes\n"); + + mp::BridgePrompter prompter{&mock_terminal}; + + std::vector nets{"eth2"}; + + EXPECT_EQ(prompter.bridge_prompt(nets), true); +} + +TEST_F(CLIPrompters, falseOnNonLiveTerminal) +{ + mpt::MockTerminal mock_terminal; + EXPECT_CALL(mock_terminal, cin_is_live()).WillOnce(Return(false)); + + mp::BridgePrompter prompter{&mock_terminal}; + + std::vector nets{"eth2"}; + + EXPECT_EQ(prompter.bridge_prompt(nets), false); +} diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 996a192e55..5552c93731 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -1571,7 +1571,7 @@ TEST_F(Daemon, refuses_launch_because_bridging_is_not_implemented) std::stringstream err_stream; send_command({"launch", "--network", "eth0"}, trash_stream, err_stream); - EXPECT_THAT(err_stream.str(), HasSubstr("The bridging feature is not implemented on this backend")); + EXPECT_THAT(err_stream.str(), HasSubstr("The networks feature is not implemented on this backend")); } TEST_P(RefuseBridging, old_image) @@ -1963,7 +1963,7 @@ TEST_F(Daemon, refuses_launch_bridged_without_setting) std::stringstream err_stream; send_command({"launch", "--network", "bridged"}, trash_stream, err_stream); EXPECT_THAT(err_stream.str(), - HasSubstr("You have to `multipass set local.bridged-network=` to use the `--bridged` shortcut.")); + HasSubstr("You have to `multipass set local.bridged-network=` to use the \"bridged\" shortcut.")); } TEST_F(Daemon, refuses_launch_with_invalid_bridged_interface) diff --git a/tests/test_daemon_start.cpp b/tests/test_daemon_start.cpp index 9cb152864a..4eab191862 100644 --- a/tests/test_daemon_start.cpp +++ b/tests/test_daemon_start.cpp @@ -15,8 +15,10 @@ * */ -#include "common.h" +// The daemon test fixture contains premock code so it must be included first. #include "daemon_test_fixture.h" + +#include "common.h" #include "mock_image_host.h" #include "mock_mount_handler.h" #include "mock_platform.h" @@ -81,6 +83,120 @@ TEST_F(TestDaemonStart, successfulStartOkStatus) EXPECT_TRUE(status.ok()); } +TEST_F(TestDaemonStart, messageOnSSHError) +{ + REPLACE(ssh_new, []() { return nullptr; }); // This makes creating the SSH session throw. + + std::vector unconfigured{{"eth7", "", true}}; + + auto mock_factory = use_a_mock_vm_factory(); + const auto [temp_dir, filename] = plant_instance_json(fake_json_contents(mac_addr, unconfigured)); + + auto instance_ptr = std::make_unique>(mock_instance_name); + EXPECT_CALL(*mock_factory, create_virtual_machine(_, _)).WillOnce([&instance_ptr](const auto&, auto&) { + return std::move(instance_ptr); + }); + + EXPECT_CALL(*instance_ptr, wait_until_ssh_up).WillRepeatedly(Return()); + EXPECT_CALL(*instance_ptr, current_state()).WillRepeatedly(Return(mp::VirtualMachine::State::off)); + EXPECT_CALL(*instance_ptr, start()).Times(1); + EXPECT_CALL(*instance_ptr, add_network_interface(_, _)).Times(1); + + config_builder.data_directory = temp_dir->path(); + config_builder.vault = std::make_unique>(); + + mp::Daemon daemon{config_builder.build()}; + + mp::StartRequest request; + request.mutable_instance_names()->add_instance_name(mock_instance_name); + + StrictMock> server; + + EXPECT_CALL(server, Write(Property(&mp::StartReply::log_line, HasSubstr("Cannot create a SSH shell")), _)).Times(1); + + auto status = call_daemon_slot(daemon, &mp::Daemon::start, request, std::move(server)); + + EXPECT_THAT(status.error_message(), StrEq("")); + EXPECT_TRUE(status.ok()); +} + +struct WithSSH : public TestDaemonStart, WithParamInterface +{ +}; + +TEST_P(WithSSH, startConfiguresInterfaces) +{ + ssh_channel_callbacks callbacks{nullptr}; + auto add_channel_cbs = [&callbacks](ssh_channel, ssh_channel_callbacks cb) { + callbacks = cb; + return SSH_OK; + }; + REPLACE(ssh_add_channel_callbacks, add_channel_cbs); + + int expected_status{GetParam()}; // The exit status given by SSH polling. + auto event_dopoll = [&callbacks, &expected_status](auto...) { + if (!callbacks) + return SSH_ERROR; + callbacks->channel_exit_status_function(nullptr, nullptr, expected_status, callbacks->userdata); + return SSH_OK; + }; + REPLACE(ssh_event_dopoll, event_dopoll); + + std::string fake_output{"some output"}; + auto remaining = fake_output.size(); + auto channel_read = [&fake_output, &remaining](ssh_channel, void* dest, uint32_t count, int is_stderr, int) { + const auto num_to_copy = std::min(count, static_cast(remaining)); + const auto begin = fake_output.begin() + fake_output.size() - remaining; + std::copy_n(begin, num_to_copy, reinterpret_cast(dest)); + remaining -= num_to_copy; + return num_to_copy; + }; + REPLACE(ssh_channel_read_timeout, channel_read); + + std::vector unconfigured{{"eth7", "", true}}; + + auto mock_factory = use_a_mock_vm_factory(); + const auto [temp_dir, filename] = plant_instance_json(fake_json_contents(mac_addr, unconfigured)); + + auto instance_ptr = std::make_unique>(mock_instance_name); + EXPECT_CALL(*mock_factory, create_virtual_machine(_, _)).WillOnce([&instance_ptr](const auto&, auto&) { + return std::move(instance_ptr); + }); + + EXPECT_CALL(*instance_ptr, wait_until_ssh_up).WillRepeatedly(Return()); + EXPECT_CALL(*instance_ptr, current_state()).WillRepeatedly(Return(mp::VirtualMachine::State::off)); + EXPECT_CALL(*instance_ptr, start()).Times(1); + EXPECT_CALL(*instance_ptr, add_network_interface(_, _)).Times(1); + + config_builder.data_directory = temp_dir->path(); + config_builder.vault = std::make_unique>(); + + mp::Daemon daemon{config_builder.build()}; + + mp::StartRequest request; + request.mutable_instance_names()->add_instance_name(mock_instance_name); + + StrictMock> server; + + if (expected_status) + { + EXPECT_CALL(server, + Write(Property(&mp::StartReply::log_line, HasSubstr("Failure configuring network interfaces")), _)) + .Times(1); + } + else + { + EXPECT_CALL(server, Write(_, _)).Times(0); + } + + auto status = call_daemon_slot(daemon, &mp::Daemon::start, request, std::move(server)); + + EXPECT_THAT(status.error_message(), StrEq("")); + EXPECT_TRUE(status.ok()); +} + +INSTANTIATE_TEST_SUITE_P(TestDaemonStart, WithSSH, Values(0, 1, -1)); + TEST_F(TestDaemonStart, unknownStateDoesNotStart) { auto mock_factory = use_a_mock_vm_factory(); diff --git a/tests/test_instance_settings_handler.cpp b/tests/test_instance_settings_handler.cpp index 9ec37e87ad..af84f4406b 100644 --- a/tests/test_instance_settings_handler.cpp +++ b/tests/test_instance_settings_handler.cpp @@ -49,7 +49,13 @@ struct TestInstanceSettingsHandler : public Test { mp::InstanceSettingsHandler make_handler() { - return mp::InstanceSettingsHandler{specs, vms, deleted_vms, preparing_vms, make_fake_persister()}; + return mp::InstanceSettingsHandler{specs, + vms, + deleted_vms, + preparing_vms, + make_fake_persister(), + make_fake_bridged_interface(), + make_fake_host_networks()}; } void fake_instance_state(const char* name, SpecialInstanceState special_state) @@ -65,6 +71,20 @@ struct TestInstanceSettingsHandler : public Test return [this] { fake_persister_called = true; }; } + std::function make_fake_bridged_interface() + { + return [] { return "eth8"; }; + } + + std::function()> make_fake_host_networks() + { + std::vector ret; + ret.push_back(mp::NetworkInterfaceInfo{"eth8", "ethernet", "Ethernet device", {}, true}); + ret.push_back(mp::NetworkInterfaceInfo{"virbr0", "bridge", "Network bridge", {}, false}); + + return [ret] { return ret; }; + } + template