[compute v2] Add multiattach flag to instance block_device (#1057)

[ralgar]

Issue

Please see #1057 for detailed description.

It is not currently possible to use multiattach volumes with the block_device block of compute instances, as it requires compute microversion >=2.60. This is a major show stopper for my use case, where I need to attach such a volume to an instance before it boots.

Proposal

The provider already uses microversion 2.67 when the volume_type parameter is set in the block_device block. Rather than adding more arbitrary checks for these edge cases, I propose we stick to a KISS philosophy by removing this check altogether, and simply pin to this "BlockDevice" microversion any time the block_device block is used.

As per a maintainer's suggestion below, I have added a multiattach flag to the block_device block, which sets the compute microversion to the required 2.60. This is similar to how other resources already handle multiattach volumes, allowing for an easy way to utilize them, while maintaining backwards compatibility with older versions of OpenStack.

I've also made note of the added flag in the relevant documentation page.

[nikParasyr]

2.67 is in Openstack Stein as shown here. Basically this change will make the compute_instance_v2 resource with block_device unusable for anyone running <= Rocky.

I'm personally against this change as it goes against the "best effort to support all versions" policy of the provider mentioned here.

I would prefer to see this implemented with a "flag" on block_device which when set by the user the provider will be setting the microversion to 2.60 (which is the microversion where the multiattach has been introduced).

[ralgar]

Thanks for the feedback @nikParasyr, that makes sense. To clarify, what you'd like to see would be a boolean flag multiattach?

Also, am I correct that this option would need to be implemented in Gophercloud first? Currently I'm getting these build errors:

openstack/resource_openstack_compute_instance_v2.go:573:10: bd.Multiattach undefined (type bootfromvolume.BlockDevice has no field or method Multiattach)
openstack/resource_openstack_compute_instance_v2.go:1374:4: unknown field Multiattach in struct literal of type bootfromvolume.BlockDevice

[nikParasyr]

To clarify, what you'd like to see would be a boolean flag multiattach?

Yes. inside the block_device. This should be defaulted to false, but when enabled the compute microversion should be set correctly.

Also, am I correct that this option would need to be implemented in Gophercloud first?

No. This will not be propagated to the request.Opts. It's just a flag to allow us to understand that we need to bumb the microversion correctly.

@kayrus what is your opinion? I dont like my suggestion in all honesty, but i cant find a better way to achieve this given the microversion labyrinth...

@ralgar lets wait for some more input before you spend time implementing this and there's an easier/better way to go about it

[ralgar]

@nikParasyr 10-4.

For what it's worth, your suggestion is in line with other volume related resources. For example, both openstack_blockstorage_volume_v3, and openstack_compute_volume_attach_v2 have a multiattach flag.

I think it's a decent suggestion but, yes, let's wait for more input.

[ralgar]

Hey, @nikParasyr. This didn't get any movement over the last month, so I've gone and implemented your original suggestion. As I mentioned previously, this appears to be how other resources are already handling multiattach volumes, so it seems like a sensible change.

All works well on my end, and I've updated the docs page to reflect the change. Let me know what you think. Thanks.

[kayrus]

Thanks for the PR and suggestions. I don't have an environment that supports multiattach, but from what I read in API ref:

From v2.60, attaching a multiattach volume to multiple instances is supported for instances that are not SHELVED_OFFLOADED. The ability to actually support a multiattach volume depends on the volume type and compute hosting the instance.

2.60 is not a requirement to use multiattach. Basically the only requirement is to use a volume with a volume type having the multiattach enabled and the comment above relates to the /servers/{server_id}/os-volume_attachments API location, not to a compute instance creation.

Moreover 2.60 is already set in

terraform-provider-openstack/openstack/resource_openstack_compute_volume_attach_v2.go

Lines 123 to 126 in 03ba68a

	multiattach := d.Get("multiattach").(bool)
	if multiattach {
	computeClient.Microversion = "2.60"
	}

Have you tried it? If you want to have a bootable volume with multiattach, from my point of view you're doing a very wrong thing :\

[ralgar]

Hey, @kayrus. Let me try to clarify things as I probably didn't provide enough info here.

This PR is for the openstack_compute_instance_v2 resource, which currently does not appear to have any support for multiattach volumes. This is not a boot volume, it is a data volume which is flagged as not bootable by setting boot_index = -1.

As this compute instance is running Fedora CoreOS, the volume needs to be attached to the instance before it is started, so that it can be customized and mounted during the boot process. FCOS will fail to boot if the volume is not available at this time. The openstack_compute_volume_attach_v2 resource is unable to fulfill this requirement as the instance is started before the volume attachment even begins.

2.60 does appear to be a requirement for multiattach, as it is explicitly mentioned in the error message:

╷
│ Error: Error creating OpenStack server: Bad request with: [POST http://xx.x.x.xx:8774/v2.1/servers], error message: {"badRequest": {"code": 400, "message": "Multiattach volumes are only supported starting with compute API version 2.60."}}
│
│   with module.fcos_server.openstack_compute_instance_v2.fcos,
│   on ../../modules/openstack/fcos_server/compute.tf line 1, in resource "openstack_compute_instance_v2" "fcos":
│    1: resource "openstack_compute_instance_v2" "fcos" {
│
╵

The problem is solved by setting the microversion, as seen in this PR.

Hopefully this all makes sense now :)

[kayrus]

This PR is for the openstack_compute_instance_v2 resource

Thanks a lot for the clarification. Please don't get me wrong, I'm not against the change, I'm here to avoid overcomplicating the fragile openstack_compute_instance_v2 resource, which is already overloaded with exceptions, and unfortunately some of them are not clearly defined, so it's not obvious to see them while reading the code. As you can see there is already a 2.60 microversion assignment in another resource, which, due to legacy reasons, isn't aligned with constants definition.

What I'm afraid of is that at some point someone will add an extra microversion, which isn't compatible with API that works with a lower microversion, or another microversion setting will downgrade one defined earlier. @nikParasyr I think we need to open a new issue and develop a helper func that ensures that microversions don't override each other.

As this compute instance is running Fedora CoreOS, the volume needs to be attached to the instance before it is started, so that it can be customized and mounted during the boot process. FCOS will fail to boot if the volume is not available at this time. The openstack_compute_volume_attach_v2 resource is unable to fulfill this requirement as the instance is started before the volume attachment even begins.

I assume you're using ignition that prepares a VM with attached block devices, and it runs only during the bootstrap.
According to my experience, most of the problems can be solved by the correct use of existing tools (e.g. udev rules may be used in your case). But in your case (and according to this change, nova API reference note wasn't clear enough) it looks reasonable to set a microversion while creating a compute instance.

@ralgar have you tested this change in your env? I have some doubts about whether DevStack supports multiattach.

[ralgar]

@kayrus No problem. I completely understand your concerns, as I had the same thoughts when I was implementing this. A helper function would certainly be a good idea going forward. I would have liked to implement such a thing here but, as I am not too familiar with the nuances of the code base, I thought it best to keep things simple.

I assume you're using ignition that prepares a VM with attached block devices, and it runs only during the bootstrap.
According to my experience, most of the problems can be solved by the correct use of existing tools (e.g. udev rules may be used in your case).

That's a correct assumption. I was aware that udev has some potentially useful functionality here, however I'm not sure how that would fit into the FCOS bootstrap sequence, and I can't seem to find any documentation or examples for doing such a thing. If you happen to have more information, or a functional example, I'd certainly be interested as that could potentially nullify the need for this change.

@ralgar have you tested this change in your env? I have some doubts about whether DevStack supports multiattach.

I believe you are correct about DevStack. AFAIK, multiattach is always opt-in for any OpenStack / Cinder environment. I have tested the change in my env, and everything appears to work as expected. With the flag set to false, or unset (so defaulting to false), it behaves as before - producing the error message I posted above. With the flag set to true it successfully provisions the instance with the multiattach volume.

[nikParasyr]

@ralgar thanks for your work and sorry for the delays :)

[ralgar]

No problem at all. Thank you both for your hard work as well! It really shows in the excellent quality of the provider :)

@nikParasyr @kayrus