Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions include/multipass/exceptions/intentional_shutdown_exception.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#pragma once

#include <stdexcept>
#include <string>

namespace multipass
{

class IntentionalShutdownException : public std::runtime_error
{
public:
explicit IntentionalShutdownException(const std::string& name)
: std::runtime_error{
"Instance '" + name +
"' stopped intentionally during initialization "
"(cloud-init power_state directive)"} {}
};

}
2 changes: 2 additions & 0 deletions include/multipass/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ bool process_log_on_error(const QString& program,
multipass::logging::Level level = multipass::logging::Level::debug,
const int timeout = 30000);

bool expects_shutdown_from_cloud_init(const YAML::Node& user_data_config);

// networking helpers
void validate_server_address(const std::string& value);
bool valid_hostname(const std::string& name_string);
Expand Down
37 changes: 27 additions & 10 deletions src/daemon/daemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <multipass/exceptions/exitless_sshprocess_exceptions.h>
#include <multipass/exceptions/ghost_instance_exception.h>
#include <multipass/exceptions/image_vault_exceptions.h>
#include <multipass/exceptions/intentional_shutdown_exception.h>
#include <multipass/exceptions/invalid_memory_size_exception.h>
#include <multipass/exceptions/not_implemented_on_this_backend_exception.h>
#include <multipass/exceptions/snapshot_exceptions.h>
Expand Down Expand Up @@ -683,7 +684,7 @@ const std::string& get_instance_name(InstanceElem instance_element)
}

template <typename... Ts>
auto add_fmt_to(fmt::memory_buffer& buffer, fmt::format_string<Ts...> fmt, Ts&&... fmt_params)
auto add_fmt_to(fmt::memory_buffer& buffer,fmt::format_string<Ts...> fmt, Ts&&... fmt_params)
-> std::back_insert_iterator<fmt::memory_buffer>
{
if (buffer.size())
Expand Down Expand Up @@ -3400,7 +3401,6 @@ void mp::Daemon::create_vm(const CreateRequest* request,
vm_desc.meta_data_config = mpu::make_cloud_init_meta_config(name);
vm_desc.user_data_config = YAML::Load(request->cloud_init_user_data());
prepare_user_data(vm_desc.user_data_config, vm_desc.vendor_data_config);

if (vm_desc.num_cores < std::stoi(mp::min_cpu_cores))
vm_desc.num_cores = std::stoi(mp::default_cpu_cores);

Expand Down Expand Up @@ -3670,18 +3670,35 @@ error_string mp::Daemon::async_wait_for_ssh_and_start_mounts_for(
return fmt::to_string(errors);
}
const auto vm = it->second;
vm->wait_until_ssh_up(timeout);

if (std::is_same<Reply, LaunchReply>::value)
try
{
if (server)
vm->wait_until_ssh_up(timeout);

if (std::is_same<Reply, LaunchReply>::value)
{
Reply reply;
reply.set_reply_message("Waiting for initialization to complete");
server->Write(reply);
}
if (server)
{
Reply reply;
reply.set_reply_message("Waiting for initialization to complete");
server->Write(reply);
}

vm->wait_for_cloud_init(timeout);
vm->wait_for_cloud_init(timeout);
}
}
catch (const mp::IntentionalShutdownException& e)
{
if constexpr (std::is_same_v<Request, LaunchRequest>)
{
mpl::log(mpl::Level::info, name,
"Instance powered off intentionally during initialization");
return {}; // Success - intentional shutdown
}
else
{
throw StartException(name, "Unexpected shutdown during start");
}
}

if (MP_SETTINGS.get_as<bool>(mp::mounts_key))
Expand Down
5 changes: 5 additions & 0 deletions src/platform/backends/applevz/applevz_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
#include <multipass/top_catch_all.h>
#include <multipass/utils/qemu_img_utils.h>
#include <multipass/vm_status_monitor.h>
#include <multipass/utils.h>

#include <qemu/qemu_img_utils.h>
#include <shared/macos/backend_utils.h>

namespace mp = multipass;
Expand All @@ -43,6 +46,8 @@ AppleVZVirtualMachine::AppleVZVirtualMachine(const VirtualMachineDescription& de
desc{desc},
monitor{&monitor}
{

expected_shutdown = utils::expects_shutdown_from_cloud_init(desc.user_data_config);
initialize_vm_handle();
}

Expand Down
1 change: 1 addition & 0 deletions src/platform/backends/hyperv/hyperv_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ mp::HyperVVirtualMachine::HyperVVirtualMachine(const VirtualMachineDescription&
power_shell{std::make_unique<PowerShell>(vm_name)},
monitor{&monitor}
{
expected_shutdown = mp::utils::expects_shutdown_from_cloud_init(desc.user_data_config);
}

void mp::HyperVVirtualMachine::setup_network_interfaces()
Expand Down
2 changes: 2 additions & 0 deletions src/platform/backends/qemu/qemu_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc
{
remove_snapshots_from_backend();
}

expected_shutdown = mp::utils::expects_shutdown_from_cloud_init(desc.user_data_config);
}

mp::QemuVirtualMachine::~QemuVirtualMachine()
Expand Down
39 changes: 36 additions & 3 deletions src/platform/backends/shared/base_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <multipass/cloud_init_iso.h>
#include <multipass/constants.h>
#include <multipass/exceptions/file_open_failed_exception.h>
#include <multipass/exceptions/intentional_shutdown_exception.h>
#include <multipass/exceptions/internal_timeout_exception.h>
#include <multipass/exceptions/ip_unavailable_exception.h>
#include <multipass/exceptions/snapshot_exceptions.h>
Expand Down Expand Up @@ -289,7 +290,8 @@ void mp::BaseVirtualMachine::detect_aborted_start()
{
shutdown_while_starting = true;
state_wait.notify_all();

if (expected_shutdown)
throw IntentionalShutdownException(vm_name);
std::string msg{"Instance shutdown during start"};
if (!saved_error_msg.empty())
msg += ": " + saved_error_msg;
Expand All @@ -304,7 +306,23 @@ void mp::BaseVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout
drop_ssh_session();
mpl::debug(vm_name, "Waiting for SSH to be up");

auto action = std::bind_front(&BaseVirtualMachine::try_to_ssh, this);
auto action = [this]() -> utils::TimeoutAction {
auto vm_state = current_state();

if (vm_state == State::stopped || vm_state == State::off)
{
if (expected_shutdown) {
mpl::log(mpl::Level::info, vm_name, "VM powered off as configured in cloud-init");
throw IntentionalShutdownException(vm_name);
} else {
mpl::log(mpl::Level::error, vm_name, "VM stopped unexpectedly");
throw StartException(vm_name, "VM stopped unexpectedly");
}
}
Comment on lines +312 to +321
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the exception could be thrown here instead of after the try_action_for.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tobe2098, I tried throwing the exception as you suggested and removed the final state check, but after that I am not able to see the intentional exception or the start exception. I am not sure why this behavior changed. I will dig deeper into it and add logging to investigate.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that if the shutdown happens during the cloud-init after the ssh_up you are not throwing in that function yet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you are right. But in my updated code, I am throwing the exception from that function. After some debugging, I think there may be a race condition issue—my vm_state is not updating inside the lambda.

So, can we do the same as in the old code where I added a final check? It introduces some delay, but it gives the expected results.

I wanted your feedback—does my observation about a race condition make sense?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense, but if you check the vm_state=current_state(); the state is supposed to be properly updated. I can do more testing on that later on.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tobe2098, You're right! I was testing the crash scenario incorrectly. I was stopping the VM before it reached the running state, which is why the state wasn't updating as expected in the lambda. That's why I thought there was a race condition.


return try_to_ssh();
};

auto timeout_action = std::bind_front(&BaseVirtualMachine::timeout_ssh, this);
mpu::try_action_for(timeout_action, timeout, action);

Expand All @@ -313,8 +331,23 @@ void mp::BaseVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout

void mp::BaseVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeout)
{
auto action = [this] {
auto action = [this]() -> mpu::TimeoutAction {
detect_aborted_start();

auto vm_state = current_state();
if(vm_state == State::stopped || vm_state == State::off)
{
if(expected_shutdown)
{
mpl::log(mpl::Level::error, vm_name, "VM powered off as configured in cloud-init");
throw IntentionalShutdownException(vm_name);
}
else
{
mpl::log(mpl::Level::error, vm_name, "VM stopped unexpectedly during cloud-init");
throw StartException(vm_name, "VM Stopped unexpectedly during cloud-init");
}
}
try
{
ssh_exec("[ -e /var/lib/cloud/instance/boot-finished ]");
Expand Down
1 change: 1 addition & 0 deletions src/platform/backends/shared/base_virtual_machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ class BaseVirtualMachine : public VirtualMachine
const QDir instance_dir;
std::optional<IPAddress> management_ip;
bool shutdown_while_starting = false;
bool expected_shutdown = false;

private:
std::string saved_error_msg = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ mp::VirtualBoxVirtualMachine::VirtualBoxVirtualMachine(const VirtualMachineDescr
name{QString::fromStdString(desc.vm_name)},
monitor{&monitor}
{
expected_shutdown = mp::utils::expects_shutdown_from_cloud_init(desc.user_data_config);
}

mp::VirtualBoxVirtualMachine::~VirtualBoxVirtualMachine()
Expand Down Expand Up @@ -450,9 +451,7 @@ void mp::VirtualBoxVirtualMachine::suspend()
mp::VirtualMachine::State mp::VirtualBoxVirtualMachine::current_state()
{
auto present_state = instance_state_for(name);

if ((state == State::delayed_shutdown && present_state == State::running) ||
state == State::starting)
if ((state == State::delayed_shutdown && present_state == State::running) || state == State::starting)
return state;

state = present_state;
Expand Down
14 changes: 14 additions & 0 deletions src/utils/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -700,3 +700,17 @@ auto mp::utils::find_bridge_with(const std::vector<mp::NetworkInterfaceInfo>& ne
});
return it == std::cend(networks) ? std::nullopt : std::make_optional(*it);
}

bool mp::utils::expects_shutdown_from_cloud_init(const YAML::Node& user_data_config)
{
if(user_data_config["power_state"])
{
auto ps = user_data_config["power_state"];
if(ps["mode"])
{
std::string mode = ps["mode"].as<std::string>();
return (mode == "poweroff" || mode == "halt");
}
}
return false;
}