Terraform-provider-libvirt: support for storage pool resource

[alxbse]

Version Reports:

Distro version of host:

Ubuntu Bionic 18.04

Terraform Version Report

v0.11.8

Libvirt version

4.7.0

terraform-provider-libvirt plugin version (git-hash)

df6aed4d9af0493e09454746549a5e95df9a2505

---

Description of Issue/Question

There doesn't seem to be any support for managing storage pools or any mentions regarding it. I've hacked together some very basic support in this provider for creating and deleting a default pool but I don't want to spend too much time on it if there is maybe a reason for it not being supported?

Would be happy to contribute the support for it, at least for basic dir and filesystem pool types.

[MalloZup]

@alxbse ciao yop, afaik we never added the pool resource because was not needed in our workflow but imho we can add it.

You can also have a look at https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/CONTRIBUTING.md

( once the feature done we need always testacc)

Also keep in mind to implement the 4 crud ops: create, read, update, delete.
( if we don't add read and update, this create annoying bugs .. :smile_cat: )

Ciao Thx, feel free to ask more for any kind of question you have! :cupid:

@dmacvicar cc

[dmacvicar]

It is certainly something that we haven't given priority as most users manage their own pools for a simple reason: data.

I would be open to add support for managing pools as long as it takes into account that we don't want then users coming back "hey, but libvirt deleted my data on destroy!".
So, we need to make the lifecycle clear or consider how to handle this in an elegant way.

Also, given that there are many types of pools, and they all have different options, and that we don't want a 1:1 mapping between XML and HCL, it is important to design the schema so that it is easy to use, powerful enough to be useful, and clever enough to be extendable to more pool types later.

So, it may be helpful that you already have some idea how the schema will look like, and may be discuss it a bit, before creating lot of code.

[rsjethani]

IMO pools are not supposed to be managed by Terraform.

[zeenix]

Just FYI, I've a WIP patch for this. Will submit soon.

[MalloZup]

@zeenix keep in mind comment https://github.com/dmacvicar/terraform-provider-libvirt/issues/435#issuecomment-427170302 as acceptance criteria of pr. For any help feel free to ask thx a lot for your help ! :sun_with_face:

[zeenix]

@MalloZup Thanks for the reminder. To be honest, I hadn't thought of the lifecycle. What I had in mind is a simple API that allows one to create and destroy a filesystem-based pool, just like volume. As for the destruction, how about we simply error out if pool is attempted to be destroyed when it has non-zero volumes in it? I'm very new to Terroform so please let me know if that doesn't work.

[MalloZup]

@zeenix np.

1) Lifecycle Resource:
So a part the operation you mentioned, create/delete/read, we need also to consider the update operation.

An update operation is like when you run terraform apply for 2 or more times, so you have an existing infrastructure, and then you want to modify only an element, in our case the pool. The update operation is the trickyiest one because it need to be idempotent.

Imagine we have default pool managed by terraform, maybe an user want to change an attriubute of the pool. Handling this things in libvirt can be sometimes tricky,

Pool are kind of global variable. If you modify a pool, it can have consequences. on all domain, so we migh need some extra attention on update some attribute of the pool in a idempotent way

this regarding the lifecycle.

2) As mentioned by @dmacvicar we had lot of problems with the 1:1 mapping between XML and HCL, so we need extra care to define the data structure of the new resource.

extra-reading:
there are some helper/hints in this page ( https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/CONTRIBUTING.md#other-learning-resources).

So resuming before we accept even a small and simple PR, we need to consider mandatory the update of the resource and some proposal of schema for avoiding to problem we had in the past, and also the architecture of the provider itself.

This is not to be picky, but is more the nature of the libvirt project itself which is complicated and beautiful project .. :grin:

[zeenix]

@MalloZup Thanks so much for such an informative reply. About update, here is the schema I went with:

            "name": {
                Type:     schema.TypeString,
                Required: true,
                ForceNew: true,
            },
            "path": {
                Type:     schema.TypeString,
                Required: true,
                ForceNew: true,
            },
            "capacity": {
                Type:     schema.TypeInt,
                Optional: true,
                Computed: true,
                ForceNew: true,
            },
            "allocation": {
                Type:     schema.TypeInt,
                Optional: true,
                Computed: true,
                ForceNew: true,
            },
            "available": {
                Type:     schema.TypeString,
                Computed: true,
                Optional: true,
                ForceNew: true,
            },
            "xml": {
                Type:     schema.TypeList,
                Optional: true,
                MaxItems: 1,
                ForceNew: true,
                Elem: &schema.Resource{
                    Schema: map[string]*schema.Schema{
                        "xslt": {
                            Type:     schema.TypeString,
                            Optional: true,
                            ForceNew: true,
                        },
                    },
                },
            },
        },

The last 3 are supposed to be read-only (that's what computed mean, right?) so it's only the name and path that we need to care about. The name should be trivial to update but path would be an issue here. Since there is no pool migration facility in libvirt, we will have to do that in the provider. That would be some work and there could be corner cases and race-conditions possible, I'm guessing. Would you have any thoughts on how can path in specific be handled in in update operation?

As for modification of 'default' pool, I don't think it was meant to be used by the whole world. Apps/services should be creating and managing their own pools if they want to modify/manage them in any way. So I am not sure if we should be worrying about consequences of updating pools to domains etc.

---

P.S. While I'm new to terraform, Go and other tech, I have years of experience with libvirt and have even added new API in there at some point (I was one of the GNOME Boxes creators and maintainer). My experience isn't very fresh though. :)

[MalloZup]

nice! @zeenix i will read it later on

[MalloZup]

@zeenix sorry for delay. to me looks good. Feel free to send a pr and we can continue discussion there.
Thx again to be on board and great that you are here in our small community. :sunflower:

[zeenix]

@MalloZup No worries about the late response and thanks so much for being so welcoming. I am currently trying to get the tests passed and in the process learning how Go tests work. :) After that, I'll add the update operation and send a PR your way. :)