Skip to content

Commit

Permalink
Merge pull request #3193 from canonical/add-bridged-setting
Browse files Browse the repository at this point in the history 
Add `local.instance.bridged` setting.
  • Loading branch information
@luis4a0
luis4a0 committed Oct 16, 2023
2 parents b8f0795 + 6e21818 commit 63e0881
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 18 deletions.
23 changes: 19 additions & 4 deletions src/daemon/daemon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,19 @@ auto try_mem_size(const std::string& val) -> std::optional<mp::MemorySize>
}
}

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 {}=<name>` to use the \"bridged\" shortcut.",
mp::bridged_interface_key));
}

return bridged_id.toStdString();
}

std::vector<mp::NetworkInterface> validate_extra_interfaces(const mp::LaunchRequest* request,
const mp::VirtualMachineFactory& factory,
std::vector<std::string>& nets_need_bridging,
Expand Down Expand Up @@ -1170,10 +1183,11 @@ register_instance_mod(std::unordered_map<std::string, mp::VMSpecs>& vm_instance_
std::unordered_map<std::string, mp::VirtualMachine::ShPtr>& vm_instances,
const std::unordered_map<std::string, mp::VirtualMachine::ShPtr>& deleted_instances,
const std::unordered_set<std::string>& preparing_instances,
std::function<void()> instance_persister)
std::function<void()> instance_persister, std::function<std::string()> bridged_interface)
{
return MP_SETTINGS.register_handler(std::make_unique<mp::InstanceSettingsHandler>(
vm_instance_specs, vm_instances, deleted_instances, preparing_instances, std::move(instance_persister)));
vm_instance_specs, vm_instances, deleted_instances, preparing_instances, std::move(instance_persister),
std::move(bridged_interface)));
}

} // namespace
Expand All @@ -1184,8 +1198,9 @@ mp::Daemon::Daemon(std::unique_ptr<const DaemonConfig> 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)}
{
connect_rpc(daemon_rpc, *this);
std::vector<std::string> invalid_specs;
Expand Down
41 changes: 37 additions & 4 deletions src/daemon/instance_settings_handler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <multipass/constants.h>
#include <multipass/exceptions/invalid_memory_size_exception.h>
#include <multipass/settings/bool_setting_spec.h>

#include <QRegularExpression>
#include <QStringList>
Expand All @@ -30,6 +31,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
{
Expand All @@ -46,7 +48,7 @@ QRegularExpression make_key_regex()
{
const auto instance_pattern = QStringLiteral("(?<instance>.+)");
const auto prop_template = QStringLiteral("(?<property>%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)");
Expand Down Expand Up @@ -153,6 +155,28 @@ 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 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)
{
// TODO: add the interface
}
}

} // namespace

mp::InstanceSettingsException::InstanceSettingsException(const std::string& reason, const std::string& instance,
Expand All @@ -165,12 +189,15 @@ mp::InstanceSettingsHandler::InstanceSettingsHandler(
std::unordered_map<std::string, VMSpecs>& vm_instance_specs,
std::unordered_map<std::string, VirtualMachine::ShPtr>& vm_instances,
const std::unordered_map<std::string, VirtualMachine::ShPtr>& deleted_instances,
const std::unordered_set<std::string>& preparing_instances, std::function<void()> instance_persister)
const std::unordered_set<std::string>& preparing_instances, std::function<void()> instance_persister,
std::function<std::string()> bridged_interface)
: vm_instance_specs{vm_instance_specs},
vm_instances{vm_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)}

{
}

Expand All @@ -180,7 +207,7 @@ std::set<QString> mp::InstanceSettingsHandler::keys() const

std::set<QString> ret;
for (const auto& item : vm_instance_specs)
for (const auto& suffix : {cpus_suffix, mem_suffix, disk_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;
Expand All @@ -191,6 +218,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)
Expand All @@ -214,6 +243,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());
}
else
{
auto size = get_memory_size(key, val);
Expand Down
3 changes: 2 additions & 1 deletion src/daemon/instance_settings_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class InstanceSettingsHandler : public SettingsHandler
std::unordered_map<std::string, VirtualMachine::ShPtr>& vm_instances,
const std::unordered_map<std::string, VirtualMachine::ShPtr>& deleted_instances,
const std::unordered_set<std::string>& preparing_instances,
std::function<void()> instance_persister);
std::function<void()> instance_persister, std::function<std::string()> bridged_interface);

std::set<QString> keys() const override;
QString get(const QString& key) const override;
Expand All @@ -59,6 +59,7 @@ class InstanceSettingsHandler : public SettingsHandler
const std::unordered_map<std::string, VirtualMachine::ShPtr>& deleted_instances;
const std::unordered_set<std::string>& preparing_instances;
std::function<void()> instance_persister;
std::function<std::string()> bridged_interface;
};

class InstanceSettingsException : public SettingsException
Expand Down
85 changes: 76 additions & 9 deletions tests/test_instance_settings_handler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ 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()};
}

void fake_instance_state(const char* name, SpecialInstanceState special_state)
Expand All @@ -65,6 +66,11 @@ struct TestInstanceSettingsHandler : public Test
return [this] { fake_persister_called = true; };
}

std::function<std::string()> make_fake_bridged_interface()
{
return [] { return "eth8"; };
}

template <template <typename /*MockClass*/> typename MockCharacter = ::testing::NiceMock>
mpt::MockVirtualMachine& mock_vm(const std::string& name, bool deleted = false)
{
Expand All @@ -82,7 +88,9 @@ struct TestInstanceSettingsHandler : public Test
std::unordered_map<std::string, mp::VirtualMachine::ShPtr> deleted_vms;
std::unordered_set<std::string> preparing_vms;
bool fake_persister_called = false;
inline static constexpr auto properties = std::array{"cpus", "disk", "memory"};
inline static constexpr std::array numeric_properties{"cpus", "disk", "memory"};
inline static constexpr std::array boolean_properties{"bridged"};
inline static constexpr std::array properties{"cpus", "disk", "memory", "bridged"};
};

QString make_key(const QString& instance_name, const QString& property)
Expand Down Expand Up @@ -193,6 +201,27 @@ TEST_F(TestInstanceSettingsHandler, getReturnsMemorySizesInHumanReadableFormat)
EXPECT_EQ(handler.get(make_key(target_instance_name, "memory")), "337.6KiB");
}

struct TestBridgedInstanceSettings : public TestInstanceSettingsHandler,
public WithParamInterface<std::pair<std::string, bool>>
{
};

TEST_P(TestBridgedInstanceSettings, getFetchesBridged)
{
const auto [br_interface, bridged] = GetParam();

constexpr auto target_instance_name = "lemmy";
specs.insert({{"mikkey", {}}, {"phil", {}}, {target_instance_name, {}}});

specs[target_instance_name].extra_interfaces = {{br_interface, "52:54:00:12:34:56", true}};

const auto got = make_handler().get(make_key(target_instance_name, "bridged"));
EXPECT_EQ(got, bridged ? "true" : "false");
}

INSTANTIATE_TEST_SUITE_P(getFetchesBridged, TestBridgedInstanceSettings,
Values(std::make_pair("eth8", true), std::make_pair("eth9", false)));

TEST_F(TestInstanceSettingsHandler, getFetchesPropertiesOfInstanceInSpecialState)
{
constexpr auto preparing_instance = "nouvelle", deleted_instance = "vague";
Expand Down Expand Up @@ -424,6 +453,19 @@ TEST_F(TestInstanceSettingsHandler, setRefusesWrongProperty)
EXPECT_EQ(original_specs, specs[target_instance_name]);
}

TEST_F(TestInstanceSettingsHandler, setRefusesToUnbridge)
{
constexpr auto target_instance_name = "hendrix";
specs.insert({{"voodoo", {}}, {"chile", {}}, {target_instance_name, {}}});
specs[target_instance_name].extra_interfaces = {{"eth8", "52:54:00:78:90:12", true}};

mock_vm(target_instance_name); // TODO: make this an expectation.

MP_EXPECT_THROW_THAT(make_handler().set(make_key(target_instance_name, "bridged"), "false"),
mp::InvalidSettingException,
mpt::match_what(HasSubstr("Bridged interface cannot be removed")));
}

using VMSt = mp::VirtualMachine::State;
using Property = const char*;
using PropertyAndState = std::tuple<Property, VMSt>; // no subliminal political msg intended :)
Expand Down Expand Up @@ -476,7 +518,8 @@ TEST_P(TestInstanceModOnStoppedInstance, setWorksOnOtherStates)
}

INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceModOnStoppedInstance,
Combine(ValuesIn(TestInstanceSettingsHandler::properties), Values(VMSt::off, VMSt::stopped)));
Combine(ValuesIn(TestInstanceSettingsHandler::numeric_properties),
Values(VMSt::off, VMSt::stopped)));

struct TestInstanceModPersists : public TestInstanceSettingsHandler, public WithParamInterface<Property>
{
Expand All @@ -496,7 +539,7 @@ TEST_P(TestInstanceModPersists, setPersistsInstances)
}

INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceModPersists,
ValuesIn(TestInstanceSettingsHandler::properties));
ValuesIn(TestInstanceSettingsHandler::numeric_properties));

TEST_F(TestInstanceSettingsHandler, setRefusesToModifyInstancesInSpecialState)
{
Expand Down Expand Up @@ -554,12 +597,12 @@ TEST_F(TestInstanceSettingsHandler, getAndSetThrowOnBadKey)

using PlainValue = const char*;
using PropertyValue = std::tuple<Property, PlainValue>;
struct TestInstanceSettingsHandlerBadValues : public TestInstanceSettingsHandler,
public WithParamInterface<PropertyValue>
struct TestInstanceSettingsHandlerBadNumericValues : public TestInstanceSettingsHandler,
public WithParamInterface<PropertyValue>
{
};

TEST_P(TestInstanceSettingsHandlerBadValues, setRefusesBadValues)
TEST_P(TestInstanceSettingsHandlerBadNumericValues, setRefusesBadNumericValues)
{
constexpr auto target_instance_name = "xanana";
const auto& [property, bad_val] = GetParam();
Expand All @@ -574,8 +617,32 @@ TEST_P(TestInstanceSettingsHandlerBadValues, setRefusesBadValues)
EXPECT_EQ(original_specs, specs[target_instance_name]);
}

INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceSettingsHandlerBadValues,
Combine(ValuesIn(TestInstanceSettingsHandler::properties),
INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceSettingsHandlerBadNumericValues,
Combine(ValuesIn(TestInstanceSettingsHandler::numeric_properties),
Values("0", "2u", "1.5f", "2.0", "0xa", "0x8", "-4", "-1", "rubbish", " 123nonsense ",
"¤9", "\n", "\t", "^", "")));

struct TestInstanceSettingsHandlerBadBooleanValues : public TestInstanceSettingsHandler,
public WithParamInterface<PropertyValue>
{
};

TEST_P(TestInstanceSettingsHandlerBadBooleanValues, setRefusesBadBooleanValues)
{
constexpr auto target_instance_name = "zappa";
const auto& [property, bad_val] = GetParam();

const auto original_specs = specs[target_instance_name];
mock_vm(target_instance_name); // TODO: make this an expectation.

MP_EXPECT_THROW_THAT(make_handler().set(make_key(target_instance_name, property), bad_val),
mp::InvalidSettingException,
mpt::match_what(AllOf(HasSubstr(bad_val), HasSubstr("try \"true\" or \"false\""))));

EXPECT_EQ(original_specs, specs[target_instance_name]);
}

INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceSettingsHandlerBadBooleanValues,
Combine(ValuesIn(TestInstanceSettingsHandler::boolean_properties),
Values("apostrophe", "(')", "1974")));
} // namespace

0 comments on commit 63e0881

Please sign in to comment.