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

[RDY] Format domain XML from a file to allow for more flexibility #903

Closed
wants to merge 1 commit into from

Conversation

teto
Copy link
Member

@teto teto commented Mar 8, 2018

Related to #883.

The PR adds a deployment.libvirtd.template setting so that users can upload their own domain.xml. They can use a few variables that will be replaced such as {interfaces} (look at domain.xml.in for an example).
I wonder if it wouldn't be best to accept a string instead of a filename ?

I need to test it a bit more.

@teto teto force-pushed the load_tpl branch 2 times, most recently from 39dda54 to 38d48db Compare March 13, 2018 04:42
@teto teto changed the title [WIP] Format domain XML from a file to allow for more flexibility [RFC] Format domain XML from a file to allow for more flexibility Mar 13, 2018
@teto
Copy link
Member Author

teto commented Mar 13, 2018

I changed the parameter to accept a string instead as it seems more "nix-like", having an extra file is a bother.
Eventually this setting could replace extraDevicesXML / extraDomainXML.

defn.vcpu,
defn.domain_type
result = defn.domain_tpl.format(
name=self._vm_id(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I tried to replace the memory_size=defn.memory_size, by **defn (more elegant) but couldn't succeed.

Allows for maximum flexibility in the definition of the libvirt domain.
Also considers the possibility of domain creation failing
@teto
Copy link
Member Author

teto commented May 24, 2018

I want to use shared folders with libvirt (http://rabexc.org/posts/p9-setup-in-libvirt), if there is any interest I can provide a proper interface deployment.libvirtd.sharedFolders but this PR would allow me to make it work inbetween.

EDIT: I guess I can use extra_devices for the shared folders, but to adjust the XML to allow debug a kernel, the PR is still needed

@teto teto changed the title [RFC] Format domain XML from a file to allow for more flexibility [RDY] Format domain XML from a file to allow for more flexibility Jul 23, 2018
@teto
Copy link
Member Author

teto commented Jul 23, 2018

bump. This should be backwards compatible.

@teto
Copy link
Member Author

teto commented Aug 16, 2018

@rbvermaa what do you think ?

@@ -53,6 +53,32 @@ in
'';
};

deployment.libvirtd.template = mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

I guess I'm confused what's the point of having this as a nixops option ?
What if someone changes this to:

deployment.libvirtd.template = ''
<other template that doesn't use the same variables>
''

That'll basically make it inconsistent with what the python code expects

@grahamc
Copy link
Member

grahamc commented Mar 26, 2020

Hello!

Thank you for this PR.

In the past several months, some major changes have taken place in
NixOps:

  1. Backends have been removed, preferring a plugin-based architecture.
    Here are some of them:

  2. NixOps Core has been updated to be Python 3 only, and at the
    same time, MyPy type hints have been added and are now strictly
    required during CI.

This is all accumulating in to what I hope will be a NixOps 2.0
release
. There is a tracking issue for that:
#1242 . It is possible that
more core changes will be made to NixOps for this release, with a
focus on simplifying NixOps core and making it easier to use and work
on.

My hope is that by adding types and more thorough automated testing,
it will be easier for contributors to make improvements, and for
contributions like this one to merge in the future.

However, because of the major changes, it has become likely that this
PR cannot merge right now as it is. The backlog of now-unmergable PRs
makes it hard to see which ones are being kept up to date.

If you would like to see this merge, please bring it up to date with
master and reopen it
. If the or mypy type checking fails, please
correct any issues and then reopen it. I will be looking primarily at
open PRs whose tests are all green.

Thank you again for the work you've done here, I am sorry to be
closing it now.

Graham

@grahamc grahamc closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants