From c1f57f77ebc90a25fcefe695127f31d8ce4e933b Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 10 Jun 2022 16:28:24 +0200 Subject: [PATCH] added null check for sparse and format (#434) --- docs/resources/vm.md | 2 +- internal/ovirt/resource_ovirt_vm.go | 23 ++- internal/ovirt/resource_ovirt_vm_test.go | 190 +++++++++++++---------- 3 files changed, 128 insertions(+), 87 deletions(-) diff --git a/docs/resources/vm.md b/docs/resources/vm.md index 7ea14b67..01523601 100644 --- a/docs/resources/vm.md +++ b/docs/resources/vm.md @@ -72,7 +72,7 @@ Required: Optional: - `format` (String) Disk format for the override. Can be 'raw' or 'cow'. -- `sparse` (Boolean) Sparse-provision the disk. +- `provisioning` (String) Provisioning the disk. Must be one of sparse,non-sparse ## Import diff --git a/internal/ovirt/resource_ovirt_vm.go b/internal/ovirt/resource_ovirt_vm.go index d25c5b05..422e407d 100644 --- a/internal/ovirt/resource_ovirt_vm.go +++ b/internal/ovirt/resource_ovirt_vm.go @@ -141,11 +141,12 @@ var vmSchema = map[string]*schema.Schema{ Description: "Disk format for the override. Can be 'raw' or 'cow'.", ValidateDiagFunc: validateFormat, }, - "sparse": { - Type: schema.TypeBool, - Optional: true, - ForceNew: true, - Description: "Sparse-provision the disk.", + "provisioning": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: fmt.Sprintf("Provisioning the disk. Must be one of %s", strings.Join(provisioningValues(), ",")), + ValidateDiagFunc: validateEnum(provisioningValues()), }, }, }, @@ -208,6 +209,10 @@ var vmSchema = map[string]*schema.Schema{ }, } +func provisioningValues() []string { + return []string{"sparse", "non-sparse"} +} + func cpuModeValues() []string { values := ovirtclient.CPUModeValues() result := make([]string, len(values)) @@ -366,15 +371,17 @@ func handleTemplateDiskAttachmentOverride( diags = append(diags, errorToDiag("add disk to VM", err)) return diags } - if formatRaw, ok := entry["format"]; ok { + + if formatRaw, ok := entry["format"]; ok && formatRaw != "" { disk, err = disk.WithFormat(ovirtclient.ImageFormat(formatRaw.(string))) if err != nil { diags = append(diags, errorToDiag("set format on disk", err)) return diags } } - if sparseRaw, ok := entry["sparse"]; ok { - disk, err = disk.WithSparse(sparseRaw.(bool)) + if provisioningRaw, ok := entry["provisioning"]; ok && provisioningRaw != "" { + isSparse := provisioningRaw.(string) == "sparse" + disk, err = disk.WithSparse(isSparse) if err != nil { diags = append(diags, errorToDiag("set sparse on disk", err)) return diags diff --git a/internal/ovirt/resource_ovirt_vm_test.go b/internal/ovirt/resource_ovirt_vm_test.go index 2db2f6e4..e29af241 100644 --- a/internal/ovirt/resource_ovirt_vm_test.go +++ b/internal/ovirt/resource_ovirt_vm_test.go @@ -683,99 +683,133 @@ func TestVMOverrideDisk(t *testing.T) { clusterID := testHelper.GetClusterID() templateID := testHelper.GetBlankTemplateID() storageDomainID := testHelper.GetStorageDomainID() - config := fmt.Sprintf( - ` -provider "ovirt" { - mock = true -} - -resource "ovirt_vm" "source" { - cluster_id = "%s" - template_id = "%s" - name = "%s" -} -resource "ovirt_disk" "source" { - storage_domain_id = "%s" - format = "cow" - size = 1048576 - alias = "test" - sparse = false -} + baseConfig := fmt.Sprintf( + ` + provider "ovirt" { + mock = true + } -resource "ovirt_disk_attachment" "source" { - vm_id = ovirt_vm.source.id - disk_id = ovirt_disk.source.id - disk_interface = "virtio_scsi" -} + resource "ovirt_vm" "source" { + cluster_id = "%s" + template_id = "%s" + name = "%s" + } -resource "ovirt_template" "source" { - vm_id = ovirt_disk_attachment.source.vm_id - name = "%s" -} + resource "ovirt_disk" "source" { + storage_domain_id = "%s" + format = "cow" + size = 1048576 + alias = "test" + sparse = false + } -data "ovirt_template_disk_attachments" "source" { - template_id = ovirt_template.source.id -} + resource "ovirt_disk_attachment" "source" { + vm_id = ovirt_vm.source.id + disk_id = ovirt_disk.source.id + disk_interface = "virtio_scsi" + } -resource "ovirt_vm" "one" { - template_id = ovirt_template.source.id - cluster_id = "%s" - name = "%s" - - dynamic "template_disk_attachment_override" { - for_each = data.ovirt_template_disk_attachments.source.disk_attachments - content { - disk_id = template_disk_attachment_override.value["disk_id"] - format = "raw" - sparse = true + resource "ovirt_template" "source" { + vm_id = ovirt_disk_attachment.source.vm_id + name = "%s" } - } -} -`, + + data "ovirt_template_disk_attachments" "source" { + template_id = ovirt_template.source.id + }`, clusterID, templateID, p.getTestHelper().GenerateTestResourceName(t), storageDomainID, p.getTestHelper().GenerateTestResourceName(t), - clusterID, - p.getTestHelper().GenerateTestResourceName(t), ) - resource.UnitTest( - t, resource.TestCase{ - ProviderFactories: p.getProviderFactories(), - Steps: []resource.TestStep{ - { - Config: config, - Check: func(state *terraform.State) error { - client := testHelper.GetClient() - vmID := state.RootModule().Resources["ovirt_vm.one"].Primary.ID - diskAttachments, err := client.ListDiskAttachments(ovirtclient.VMID(vmID)) - if err != nil { - return err - } - diskAttachment := diskAttachments[0] - disk, err := diskAttachment.Disk() - if err != nil { - return err - } - if disk.Format() != ovirtclient.ImageFormatRaw { - return fmt.Errorf("incorrect disk format: %s", disk.Format()) - } - if !disk.Sparse() { - return fmt.Errorf("disk incorrectly created as sparse") - } - return nil + rconf := + ` + resource "ovirt_vm" "one" { + template_id = ovirt_template.source.id + cluster_id = "%s" + name = "%s" + + dynamic "template_disk_attachment_override" { + for_each = data.ovirt_template_disk_attachments.source.disk_attachments + content { + disk_id = template_disk_attachment_override.value["disk_id"] + format = %s + provisioning = %s + } + } + }` + + testcases := []struct { + name string + inputFormat string + inputProvisioning string + expectedFormat ovirtclient.ImageFormat + expectedSparse bool + }{ + { + name: "override sparse", + inputFormat: "null", + inputProvisioning: "\"sparse\"", + expectedFormat: ovirtclient.ImageFormatCow, + expectedSparse: true, + }, + { + name: "override format", + inputFormat: "\"raw\"", + inputProvisioning: "null", + expectedFormat: ovirtclient.ImageFormatRaw, + expectedSparse: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + config := baseConfig + fmt.Sprintf(rconf, + clusterID, + p.getTestHelper().GenerateTestResourceName(t), + testcase.inputFormat, + testcase.inputProvisioning, + ) + + resource.UnitTest( + t, resource.TestCase{ + ProviderFactories: p.getProviderFactories(), + Steps: []resource.TestStep{ + { + Config: config, + Check: func(state *terraform.State) error { + client := testHelper.GetClient() + vmID := state.RootModule().Resources["ovirt_vm.one"].Primary.ID + diskAttachments, err := client.ListDiskAttachments(ovirtclient.VMID(vmID)) + if err != nil { + return err + } + diskAttachment := diskAttachments[0] + disk, err := diskAttachment.Disk() + if err != nil { + return err + } + if disk.Format() != testcase.expectedFormat { + return fmt.Errorf("incorrect disk format: %s", disk.Format()) + } + if disk.Sparse() != testcase.expectedSparse { + return fmt.Errorf("disk incorrectly created as sparse") + } + return nil + }, + }, + { + Config: config, + Destroy: true, + }, }, }, - { - Config: config, - Destroy: true, - }, - }, - }, - ) + ) + }) + } } func TestMemory(t *testing.T) {