Skip to content
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

option to autoinstall/autoupdate declarative vms #94

Open
megheaiulian opened this issue Apr 21, 2023 · 11 comments
Open

option to autoinstall/autoupdate declarative vms #94

megheaiulian opened this issue Apr 21, 2023 · 11 comments

Comments

@megheaiulian
Copy link
Contributor

Currently microvm does not install updates to a vm if the vm already exists in state dir.
To push/deploy the new configuration user would need to delete the old vm or microvm -u.
Since by default the vms is using tmpfs and all its state is ephemeral I think it would make sense to add a autoupdate or autoinstall option that does the install and overwrites the vm on change.
This is prevented by ConditionPathExists

unitConfig.ConditionPathExists = "!${stateDir}/${name}";
and the new boolean option would just disable that.

@megheaiulian
Copy link
Contributor Author

@astro Let me know if you think something like that makes sense? Would work nicely for stateless vms.

megheaiulian added a commit to plumelo/microvm.nix that referenced this issue Apr 21, 2023
Implements `microvm.autoupdate` boolean option.
When enable it automatically commits changes to the vm.

Fixes astro#94
@astro
Copy link
Owner

astro commented Apr 21, 2023

Thank you for giving this some thought. The downside of this approach is that a host reboot will roll back a VM that was previously updated with microvm -u. Then again, that probably won't be used if the autoupdate option is enabled?

megheaiulian added a commit to plumelo/microvm.nix that referenced this issue Apr 21, 2023
Implements `microvm.autoupdate` boolean option.
When enable it automatically commits changes to the vm.

Fixes astro#94
@megheaiulian
Copy link
Contributor Author

When using the declarative approach it is not very pleasant to have to run microvm -u.
Ideally i'd like to do a change in the configuration, rebuild and get the latest configuration up and running.

@astro
Copy link
Owner

astro commented Apr 21, 2023

I recently added the autorestart option, and I am really unhappy about its naming because it does not convey much of its purpose. People may use it now, so renaming it would be bad. Do you have any ideas for a better name than autoupdate? To me, autoupdate sounds like true automated updates.

Actually, the "install-microvm-${name}" service can already access a MicroVM's config because in this case it is evaluated together with the host. Move the let block under script up a level, then this new option can become individually configurable per MicroVM.

I would like to add that instead of the option we could get some nice automatic behavior here if we were only able to tell which MicroVM config is newer. Everything under the store is 1970-01-01 but maybe we could export the flake.sourceInfo.lastModified into the runner package? Would that work for you?

@oddlama
Copy link
Contributor

oddlama commented May 1, 2023

Do you have any ideas for a better name than autoupdate?

Not OP, but I'd suggest [enable]DeclarativeUpgrades (or something similar) to convey that upgrades are handled automatically as opposed to imperatively. This would be a such a great fit together with impermanence - keeping all the VM's state in well defined directories makes declarative upgrades a natural choice.

I would like to add that instead of the option we could get some nice automatic behavior here if we were only able to tell which MicroVM config is newer.

I don't think this is something that should be decided automatically. People that are not using the declarative approach could have microvms with accumulated changes, where it might not be desired to restart them just because the host is deployed with a newer version.

I would even go so far as to say that opting in to the declarative approach via declarativeUpgrades (or whatever it will be called) should remove the ability to set both flake and updateFlake and instead only accept a config attribute similar to what is suggested in #92 . Including the whole flake might be undesirable, especially since it currently imposes a certain global schema requirement onto the flake.

People may use it now, so renaming it would be bad.

I agree that a different name would've been more meaningful. Currently its description also suggests unconditional restarts on rebuild, which I believe isn't the case (not all services are affected by runner version changes). Maybe restartIfChanged, similar to the option name used in the systemd service, might have been slighly better? I'm not sure.

But personally I definitely wouldn't see an issue with refactoring something like that, even if it is a breaking change. Especially since it was introduced recently and you don't guarantee any backward compatibility as far as I can tell. If you don't want to add silent breakage for users, there's always mkRemovedOptionModule and siblings.

@astro
Copy link
Owner

astro commented May 1, 2023

Thank you for elaborating on the topic.

What this really would do is kind of alwaysInstall because it causes the install-microvm service to be run on every boot and nixos-rebuild switch. Right?

@oddlama
Copy link
Contributor

oddlama commented May 1, 2023

Depends on the context. Definitely every reboot, but otherwise not strictly on every nixos-rebuild switch, since the service would only be restarted if the definition changes. (Except if you would explicitly restart it ofc, but I wouldn't see any reason to do that)

@megheaiulian
Copy link
Contributor Author

Tried to implement this but not sure how we could both mark that the install-microvm service is part of microvm-%i (so that it start when microvm-%i starts) but also when it restarts make microvm-%i restart too.

@astro
Copy link
Owner

astro commented May 3, 2023

Tried to implement this but not sure how we could both mark that the install-microvm service is part of microvm-%i (so that it start when microvm-%i starts) but also when it restarts make microvm-%i restart too.

It's already partOf = [ "microvm@${name}.service" ] -- doesn't that do the trick?

@megheaiulian
Copy link
Contributor Author

megheaiulian commented May 4, 2023

It's already partOf = [ "microvm@${name}.service" ] -- doesn't that do the trick?

That makes it start when microvm@name start. But it doesn't make microvm@name restart when it (install) has changed.

@oddlama
Copy link
Contributor

oddlama commented May 10, 2023

That makes it start when microvm@name start. But it doesn't make microvm@name restart when it (install) has changed.

Here's a very hacky idea that should get it to work:

  • For each declarative vm, use an explicit unit instead of the templated unit microvm@%i.
  • In that unit, include microvmConfig.system.build.toplevel in a comment in preStart so that
    the service changes when the toplevel changes. It will then be restarted by nixos-rebuild switch in case the service is marked as restartIfChanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants