added null check for sparse and format (#434)

oVirt · Jun 10, 2022 · c1f57f7 · c1f57f7
1 parent ce54a35
commit c1f57f7
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 87 deletions.
diff --git 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
@@ -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
@@ -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) {