-
Notifications
You must be signed in to change notification settings - Fork 736
Az implementation #4061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: availability-zones
Are you sure you want to change the base?
Az implementation #4061
Changes from all commits
a444f51
94648e1
4cdd38d
5ec148f
aeab9a5
bc3e81c
59dc732
48d6ec7
acc2e04
f000acf
6739f0b
f00a058
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1034,6 +1034,8 @@ mp::InstanceStatus::Status grpc_instance_status_for(const mp::VirtualMachine::St | |
| return mp::InstanceStatus::SUSPENDING; | ||
| case mp::VirtualMachine::State::suspended: | ||
| return mp::InstanceStatus::SUSPENDED; | ||
| case mp::VirtualMachine::State::unavailable: | ||
| return mp::InstanceStatus::UNAVAILABLE; | ||
| case mp::VirtualMachine::State::unknown: | ||
| default: | ||
| return mp::InstanceStatus::UNKNOWN; | ||
|
|
@@ -2083,6 +2085,12 @@ try | |
| continue; | ||
| } | ||
|
|
||
| if (vm->current_state() == VirtualMachine::State::unavailable) | ||
| { | ||
| add_fmt_to(errors, "instance '{}' is not available", name); | ||
| continue; | ||
| } | ||
|
|
||
| auto& vm_mounts = mounts[name]; | ||
| if (vm_mounts.find(target_path) != vm_mounts.end()) | ||
| { | ||
|
|
@@ -2259,9 +2267,12 @@ try | |
| continue; | ||
| } | ||
| case VirtualMachine::State::suspending: | ||
| case VirtualMachine::State::unavailable: | ||
| fmt::format_to(std::back_inserter(start_errors), | ||
| "Cannot start the instance '{}' while suspending.", | ||
| name); | ||
| "Cannot start the instance '{}' while {}.", | ||
| name, | ||
| vm.current_state() == VirtualMachine::State::suspending ? "suspending" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be easier once the Hyper-V branch is in: https://github.com/canonical/multipass/pull/4080/files#diff-d847b18fbc46d755306da3eb6cac52de71043c1cc87ff345cdf8b034ad141ffaR158 You could add a TODO here, or convince @xmkg to split that off 😉 |
||
| : "unavailable"); | ||
| continue; | ||
| case VirtualMachine::State::delayed_shutdown: | ||
| delayed_shutdown_instances.erase(name); | ||
|
|
@@ -2364,6 +2375,14 @@ try | |
| if (status.ok()) | ||
| { | ||
| status = cmd_vms(instance_selection.operative_selection, [this](auto& vm) { | ||
| if (vm.current_state() == VirtualMachine::State::unavailable) | ||
| { | ||
| mpl::log(mpl::Level::info, | ||
| vm.vm_name, | ||
| "Ignoring suspend since instance is unavailable."); | ||
| return grpc::Status::OK; | ||
| } | ||
|
Comment on lines
+2378
to
+2384
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idem: shouldn't this be handled by the VM? It would go in the direction of relieving the daemon from such concerns. |
||
|
|
||
| stop_mounts(vm.vm_name); | ||
|
|
||
| vm.suspend(); | ||
|
|
@@ -2485,6 +2504,14 @@ try | |
| { | ||
| const auto& instance_name = vm_it->first; | ||
|
|
||
| if (vm_it->second->current_state() == VirtualMachine::State::unavailable) | ||
| { | ||
| mpl::log(mpl::Level::info, | ||
| instance_name, | ||
| "Ignoring delete since instance is unavailable."); | ||
| continue; | ||
| } | ||
|
|
||
| auto snapshot_pick_it = instance_snapshots_map.find(instance_name); | ||
| const auto& [pick, all] = snapshot_pick_it == instance_snapshots_map.end() | ||
| ? SnapshotPick{{}, true} | ||
|
|
@@ -2533,12 +2560,19 @@ try | |
| const auto target_path = | ||
| QDir::cleanPath(QString::fromStdString(path_entry.target_path())).toStdString(); | ||
|
|
||
| if (operative_instances.find(name) == operative_instances.end()) | ||
| auto vm = operative_instances.find(name); | ||
| if (vm == operative_instances.end()) | ||
| { | ||
| add_fmt_to(errors, "instance '{}' does not exist", name); | ||
| continue; | ||
| } | ||
|
|
||
| if (vm->second->current_state() == VirtualMachine::State::unavailable) | ||
| { | ||
| mpl::log(mpl::Level::info, name, "Ignoring umount since instance unavailable."); | ||
| continue; | ||
| } | ||
|
|
||
| auto& vm_spec_mounts = vm_instance_specs[name].mounts; | ||
| auto& vm_mounts = mounts[name]; | ||
|
|
||
|
|
@@ -3730,6 +3764,7 @@ 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) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -412,6 +412,10 @@ void mp::HyperVVirtualMachine::suspend() | |
| { | ||
| mpl::info(vm_name, "Ignoring suspend issued while stopped"); | ||
| } | ||
| else if (present_state == State::unavailable) | ||
| { | ||
| mpl::log(mpl::Level::info, vm_name, "Ignoring suspend since instance is unavailable"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem: merge with the above? Otherwise, use |
||
| } | ||
|
|
||
| monitor->on_suspend(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -426,9 +426,11 @@ void mp::QemuVirtualMachine::suspend() | |
|
|
||
| vm_process.reset(nullptr); | ||
| } | ||
| else if (state == State::off || state == State::suspended) | ||
| else if (state == State::off || state == State::suspended || state == State::unavailable) | ||
| { | ||
| mpl::info(vm_name, "Ignoring suspend issued while stopped/suspended"); | ||
| mpl::log(mpl::Level::info, | ||
| vm_name, | ||
| "Ignoring suspend issued while stopped/suspended/unavailable"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem: state formatting would help |
||
| monitor->on_suspend(); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,8 +117,32 @@ void BaseAvailabilityZone::set_available(const bool new_available) | |
| m.available = new_available; | ||
| serialize(); | ||
|
|
||
| for (auto& vm : m.vms) | ||
| vm.get().set_available(m.available); | ||
| try | ||
| { | ||
| for (auto& vm : m.vms) | ||
| vm.get().set_available(new_available); | ||
| } | ||
| catch (...) | ||
| { | ||
| // if an error occurs fallback to available. | ||
| m.available = true; | ||
| serialize(); | ||
|
Comment on lines
+128
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't we just do this at the end? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since the catch is rethrowing, control flow would never reach the end of the function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. You could use a scope guard, but no need really. |
||
|
|
||
| // make sure nothing is still unavailable. | ||
| for (auto& vm : m.vms) | ||
| { | ||
| // setting the state here breaks encapsulation, but it's already broken. | ||
| std::unique_lock vm_lock{vm.get().state_mutex}; | ||
| if (vm.get().current_state() == VirtualMachine::State::unavailable) | ||
| { | ||
| vm.get().state = VirtualMachine::State::off; | ||
| vm.get().update_state(); | ||
| } | ||
|
Comment on lines
+134
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered force-stop instead? That would avoid breaking encapsulation yet again and make sure there were no leftover processes. |
||
| } | ||
|
|
||
| // rethrow the error so something else can deal with it. | ||
| throw; | ||
| } | ||
| } | ||
|
|
||
| void BaseAvailabilityZone::add_vm(VirtualMachine& vm) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,6 +204,10 @@ void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_po | |
| { | ||
| throw VMStateIdempotentException{"Ignoring shutdown since instance is already stopped."}; | ||
| } | ||
| if (state == State::unavailable) | ||
| { | ||
| throw VMStateIdempotentException{"Ignoring shutdown since instance is unavailable."}; | ||
| } | ||
|
Comment on lines
+207
to
+210
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about merging with the above branch here as well. Again, the formatter would help. If you want to simplify, I wouldn't be shocked if the message read "Ignoring shutdown since instance is already unavailable". |
||
|
|
||
| if (shutdown_policy == ShutdownPolicy::Poweroff) | ||
| { | ||
|
|
@@ -238,6 +242,29 @@ void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_po | |
| } | ||
| } | ||
|
|
||
| void mp::BaseVirtualMachine::set_available(bool available) | ||
| { | ||
| if (available) | ||
| { | ||
| state = State::off; | ||
| update_state(); | ||
| if (was_running) | ||
| { | ||
| start(); | ||
|
|
||
| // normally the daemon sets the state to running... | ||
ricab marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| state = State::running; | ||
| update_state(); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| was_running = state == State::running || state == State::starting || state == State::restarting; | ||
| shutdown(ShutdownPolicy::Poweroff); | ||
| state = State::unavailable; | ||
| update_state(); | ||
| } | ||
|
|
||
| std::string mp::BaseVirtualMachine::ssh_exec(const std::string& cmd, bool whisper) | ||
| { | ||
| std::unique_lock lock{state_mutex}; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.