Terraform-provider-libvirt: Provider does not check image correctness after a failure downloading it

[juliogonzalez]

Version Reports:

Distro version of host:

SLE 12 SP3

Terraform Version Report

v0.11.10

Provider and libvirt versions

0.5.0+git.1542698041.34c84ded-1.1

---

Description of Issue/Question

Setup

main.tf.gz

Steps to Reproduce Issue

I am not 100% sure how to reproduce the exact error, since it happens from time to time when downloading the CentOS7 image from GitHub (S3), so it seems to be a network error that fails terrafrom (good), but then the provider does not notice the error happened.

but the sequence is as follows:

1. Run terraform apply
2. A network error happens downloading the CentOS7 images (terraform will then fail)
   14:06:31 * libvirt_volume.centos7_volume: Error while uploading source https://github.com/moio/sumaform-images/releases/download/4.3.0/centos7.qcow2: Error while copying source to volume read tcp 10.162.210.226:57720->52.216.96.35:443: read: connection reset by peer
3. Taint all resources
4. Run terrafrom apply, the CentOS7 image will not be redownloaded, and the instance such image will go maintenance mode.

If you run virsh console <instance_name:

virsh # console suma-32-min-centos7
Conectado con el dominio suma-32-min-centos7                                                                                                                                                                                                                                   
Carácter de escape es ^]                                                                                                                                                                                                                                                       
^C                                                                                                                                                                                                                                                                             
:/# exit                                                                                                                                                                                                                                                                       
logout                                                                                                                                                                                                                                                                         

Generating "/run/initramfs/rdsosreport.txt"                                                                                                                                                                                                                                    


Entering emergency mode. Exit the shell to continue.                                                                                                                                                                                                                           
Type "journalctl" to view system logs.                                                                                                                                                                                                                                         
You might want to save "/run/initramfs/rdsosreport.txt" to a USB stick or /boot                                                                                                                                                                                                
after mounting them and attach it to a bug report.                                                                                                                                                                                                                             


:/#                                                                                                                                                                                                                                                                            

https://ci.suse.de/job/manager-3.2-cucumber-NUE/5/console (terraform apply on a clean instance, and the network error)
https://ci.suse.de/job/manager-3.2-cucumber-NUE/6/console(terraform taint, terrafrom apply, and the CentOS7 image goes maintenance mode, because the image is corrupted)

---

Additional Infos:

Do you have SELinux or Apparmor/Firewall enabled? Some special configuration?
Have you tried to reproduce the issue without them enabled?

No special configuration, AFAIK.

[juliogonzalez]

Ping @Bischoff, as he initially discovered this problem a week ago.

[MalloZup]

@juliogonzalez concretely what do you mean here is e.g:

Img size = 3GB
Img downloaded = 1GB
Stop download for whatever reason, we think the download is finished.
we don't ensure that the image has the full-size right? because we have the disk there already.

I think if it this case, we might add a computed field where we store the original-size and we should retry in case of failures

[juliogonzalez]

@MalloZup the actual result is that the image size is ~700Mb but the downladed image is ~200MB.

But there was an error, and it was detected by terraform, it's on the description and the log:

Error while uploading source https://github.com/moio/sumaform-images/releases/download/4.3.0/centos7.qcow2: Error while copying source to volume read tcp 10.162.210.226:57720->52.216.96.35:443: read: connection reset by peer

So maybe the image resource should be marked just as failed from a terraform perspective (if it is not now), so when terrafrom apply runs again, the image is redownloaded.

I hope I am not saying something stupid, a I am not a terraform or provider developer :-D

[juliogonzalez]

@MalloZup not exactly.

The problem can be detected by size, of course. But the network error itself was detected by terraform. Can't that be used to mark the image as failed?

[dmacvicar]

https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/libvirt/resource_libvirt_volume.go#L16

Problem is that we only check if the volume with the same name exists. I don't think we can detect by size, as we create the volume with the target size before uploading it, and it does not help if we fully upload it but corruption happens.

I don't see any libvirt API to checksum the volumes. Given that volume upload is tied to creation (we don't support upload to existing volumes), I'd suggest that in case of a failed upload, we clean the full volume (remove it). The half uploaded volume is useless anyway.

[Bischoff]

The problem can be detected by size, of course. But the network error itself was detected by terraform.
Can't that be used to mark the image as failed?

Could we "taint" it in that case? It would force a re-download on next run.

i think we would need an additional field for that checking the size of the copied image is like
the source.. thx for issue 

That's a valid approach too. The advantage is that we would not depend on a previous failure correctly processed. And another advantage is that we would detect when the image has changed upstream.

The downside would be even more network transactions. And dmacvicar suggests it's not even possible...

I'd suggest that in case of a failed upload, we clean the full volume (remove it). The half uploaded
volume is useless anyway.

That's a third approach, yes.

[Bischoff]

I don't see any libvirt API to checksum the volumes.

Do we use libvirt to do the downloads ? IIRC the provider simply does a transfer with curl or something like that.

[MalloZup]

The provider download the files in memory with a stream, this will then download to libvirt volume.(explaining shortly) What duncan says is correct, the best and simply approach is to remove volume in case of failures. The other approchs could be more difficult and not guarantee any non corruption. It rest to be investigated if the removal has any conseq. In term of the tfstate values and logical internal resources of provider are constitent after the removal but yop , is to see when doing it

[juliogonzalez]

That approach is perfectly fine for me.

As long as the volume is redownloaded when we rerun terraform apply, I don't really care about the rest. If deleting the image on failure works, then go ahead :-D

[dmacvicar]

Could we "taint" it in that case? It would force a re-download on next run.

This is the right approach. And it is trivial IIRC. It shows that our code has a bug:

https://github.com/dmacvicar/terraform-provider-libvirt/blob/67abfbc429d5c1dd8ddc663ca01e98d733e42b6a/libvirt/resource_libvirt_volume.go#L260

We set the ID before the moment an error can occur. We should set the ID either after the resource is ready, or d.Set("id", "") in the error handling should _taint_ it.

(eg. https://github.com/terraform-providers/terraform-provider-aws/blob/a9c4547c92b51790273d46781069675206ca049d/aws/resource_aws_network_interface_sg_attachment.go#L98 and and https://www.terraform.io/docs/extend/resources.html)

[MalloZup]

@juliogonzalez is this somehow impacting you?

[MalloZup]

fixed by https://github.com/dmacvicar/terraform-provider-libvirt/pull/648

[MalloZup]

@juliogonzalez @Bischoff the only way you can profite for this is to use terraform v12 ( since the pluging is builded on tf12)