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

Add volume_attachment resources or equivalent #79

Closed
moio opened this issue Oct 24, 2016 · 9 comments
Closed

Add volume_attachment resources or equivalent #79

moio opened this issue Oct 24, 2016 · 9 comments

Comments

@moio
Copy link
Collaborator

moio commented Oct 24, 2016

I want to create a libvirt domain with a variable number of attached volumes (case in point is one main disk always present, and one optional data disk).

Currently this is not possible in any way I tried. Although the disk attribute looks like a map it cannot be passed parameters (the following does not work):

resource "libvirt_domain" "domain" {
disk = ["${var.disks}"]

Where var.disks is a map with a volume_id key and a correct id value. In fact not even the following works:

disk = ["${map("volume_id", "123")}"]

We get the following error on 0.7.7:

Error reading config for libvirt_domain[domain]: At 31:11: root.disk[0]: not an object type for map (*ast.LiteralType)

It would be great to have a way around this, eg:

  • having a disk_ids plain list of ids instead of a list of maps/objects
  • having a separate resource to associate disks to instances, similar to aws_volume_attachment
  • being able to use a plain map for the existing disk variable.

Thanks in advance!!!

@inercia
Copy link
Contributor

inercia commented Oct 24, 2016

Maybe we can find a solution in the context proposed in #46 ...

@moio
Copy link
Collaborator Author

moio commented Oct 24, 2016

@inercia I do not think this is related.

Besides the fact I would agree with closing #46 the RFC is about embedding parameters for the ephemeral, "must-have" OS basic volume in the domain resource, while this is about extra "data" disks which might or might be there - hence in variable quantity.

@moio moio changed the title Add support for volume_attachment resources Add volume_attachment resources or equivalent Oct 24, 2016
@moio
Copy link
Collaborator Author

moio commented Oct 24, 2016

My proposal would be to add a disk_ids list in the domain resource, which could be populated via variables or interpolation.

This could even supersede the current disk variable, unless there is a reason I cannot see for keeping one map object instead of a plain id.

@flavio, @dmacvicar?

moio added a commit to moio/terraform-provider-libvirt that referenced this issue Oct 27, 2016
moio added a commit to moio/terraform-provider-libvirt that referenced this issue Oct 28, 2016
@moio
Copy link
Collaborator Author

moio commented Oct 31, 2016

Note this comment about disk_ids not being the right way to address this: #81 (comment)

@moio
Copy link
Collaborator Author

moio commented Oct 31, 2016

Asked HashiCorp if this is a core issue or by design.

@tboerger
Copy link

I would take other providers as a template, Scaleway got a comparable approach.

@moio
Copy link
Collaborator Author

moio commented Oct 31, 2016

OK, seems it's possible to fix this on our side, accepting maps and not objects for the disk block. This would be completely backwards compatible and would allow interpolations/variables to work in the most flexible way.

https://gitter.im/hashicorp-terraform/Lobby?at=581793b5806316005dde1c80
https://github.com/hashicorp/terraform/blob/master/builtin/providers/heroku/resource_heroku_app.go#L111

I will probably attempt a pull request soon.

@moio
Copy link
Collaborator Author

moio commented Nov 2, 2016

Correct solution should be, from my point of view, #83

@moio
Copy link
Collaborator Author

moio commented Nov 2, 2016

PR merged, closing.

@moio moio closed this as completed Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants