From b2201aa8b60b0b18da5cf1d0c43cc80f54d459e7 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 17 Jun 2022 16:13:24 +0200 Subject: [PATCH] fix: memory_ballooning for false value properly set (#442) --- internal/ovirt/resource_ovirt_disk.go | 5 +- internal/ovirt/resource_ovirt_disk_test.go | 74 +++++++++++++++++++ internal/ovirt/resource_ovirt_vm.go | 15 +++- internal/ovirt/resource_ovirt_vm_test.go | 84 ++++++++++++++++++++++ 4 files changed, 174 insertions(+), 4 deletions(-) diff --git a/internal/ovirt/resource_ovirt_disk.go b/internal/ovirt/resource_ovirt_disk.go index 7b746e7d..5cd69523 100644 --- a/internal/ovirt/resource_ovirt_disk.go +++ b/internal/ovirt/resource_ovirt_disk.go @@ -109,7 +109,10 @@ func (p *provider) diskCreate( } } } - if sparse, ok := data.GetOk("sparse"); ok { + // GetOkExists is necessary here due to GetOk check for default values (for sparse=false, ok would be false, too) + // see: https://github.com/hashicorp/terraform/pull/15723 + //nolint:staticcheck + if sparse, ok := data.GetOkExists("sparse"); ok { params, err = params.WithSparse(sparse.(bool)) if err != nil { return diag.Diagnostics{ diff --git a/internal/ovirt/resource_ovirt_disk_test.go b/internal/ovirt/resource_ovirt_disk_test.go index 01ad24a5..4f6a5de2 100644 --- a/internal/ovirt/resource_ovirt_disk_test.go +++ b/internal/ovirt/resource_ovirt_disk_test.go @@ -105,3 +105,77 @@ resource "ovirt_disk" "foo" { }, }) } + +func TestDiskResourceSparse(t *testing.T) { + t.Parallel() + + p := newProvider(newTestLogger(t)) + storageDomainID := p.getTestHelper().GetStorageDomainID() + + baseConfig := ` + provider "ovirt" { + mock = true + } + + resource "ovirt_disk" "foo" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = %s + }` + + testcases := []struct { + inputSparse string + expectedSparse bool + }{ + { + inputSparse: "null", + expectedSparse: false, + }, + { + inputSparse: "false", + expectedSparse: false, + }, + { + inputSparse: "true", + expectedSparse: true, + }, + } + + for _, tc := range testcases { + tcName := fmt.Sprintf("disk resource sparse=%s", tc.inputSparse) + t.Run(tcName, func(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + ProviderFactories: p.getProviderFactories(), + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf( + baseConfig, + storageDomainID, + tc.inputSparse, + ), + Check: resource.ComposeTestCheckFunc( + func(s *terraform.State) error { + client := p.getTestHelper().GetClient() + diskID := s.RootModule().Resources["ovirt_disk.foo"].Primary.ID + disk, err := client.GetDisk(ovirtclient.DiskID(diskID)) + if err != nil { + return err + } + + if disk.Sparse() != tc.expectedSparse { + return fmt.Errorf("Expected sparse to be %t, but got %t", + tc.expectedSparse, + disk.Sparse()) + } + + return nil + }, + ), + }, + }, + }) + }) + } +} diff --git a/internal/ovirt/resource_ovirt_vm.go b/internal/ovirt/resource_ovirt_vm.go index 422e407d..1f569ae8 100644 --- a/internal/ovirt/resource_ovirt_vm.go +++ b/internal/ovirt/resource_ovirt_vm.go @@ -326,7 +326,10 @@ func handleVMSerialConsole( params ovirtclient.BuildableVMParameters, diags diag.Diagnostics, ) diag.Diagnostics { - serialConsole, ok := data.GetOk("serial_console") + // GetOkExists is necessary here due to GetOk check for default values (for serial_console=false, ok would be false, too) + // see: https://github.com/hashicorp/terraform/pull/15723 + //nolint:staticcheck + serialConsole, ok := data.GetOkExists("serial_console") if !ok { return diags } @@ -340,7 +343,10 @@ func handleVMClone( params ovirtclient.BuildableVMParameters, diags diag.Diagnostics, ) diag.Diagnostics { - shouldClone, ok := data.GetOk("clone") + // GetOkExists is necessary here due to GetOk check for default values (for clone=false, ok would be false, too) + // see: https://github.com/hashicorp/terraform/pull/15723 + //nolint:staticcheck + shouldClone, ok := data.GetOkExists("clone") if !ok { return diags } @@ -414,7 +420,10 @@ func handleVMMemoryPolicy( addMemoryPolicy = true } } - ballooning, ok := data.GetOk("memory_ballooning") + // GetOkExists is necessary here due to GetOk check for default values (for ballooning=false, ok would be false, too) + // see: https://github.com/hashicorp/terraform/pull/15723 + //nolint:staticcheck + ballooning, ok := data.GetOkExists("memory_ballooning") if ok { var err error _, err = memoryPolicy.WithBallooning(ballooning.(bool)) diff --git a/internal/ovirt/resource_ovirt_vm_test.go b/internal/ovirt/resource_ovirt_vm_test.go index e29af241..d0e18398 100644 --- a/internal/ovirt/resource_ovirt_vm_test.go +++ b/internal/ovirt/resource_ovirt_vm_test.go @@ -876,6 +876,90 @@ resource "ovirt_vm" "test" { ) } +func TestMemoryBallooning(t *testing.T) { + t.Parallel() + + p := newProvider(newTestLogger(t)) + testHelper := p.getTestHelper() + clusterID := testHelper.GetClusterID() + + baseConfig := ` + provider "ovirt" { + mock = true + } + + data "ovirt_blank_template" "blank" { + } + + resource "ovirt_vm" "test" { + template_id = data.ovirt_blank_template.blank.id + cluster_id = "%s" + name = "%s" + memory_ballooning = %s + }` + + testcases := []struct { + inputMemoryBalloning string + expectedMemoryBallooning bool + }{ + { + inputMemoryBalloning: "null", + expectedMemoryBallooning: true, + }, + { + inputMemoryBalloning: "false", + expectedMemoryBallooning: false, + }, + { + inputMemoryBalloning: "true", + expectedMemoryBallooning: true, + }, + } + + for _, tc := range testcases { + tcName := fmt.Sprintf("memory_ballooning_for_%s", tc.inputMemoryBalloning) + t.Run(tcName, func(t *testing.T) { + config := fmt.Sprintf( + baseConfig, + clusterID, + p.getTestHelper().GenerateTestResourceName(t), + tc.inputMemoryBalloning, + ) + + 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.test"].Primary.ID + vm, err := client.GetVM(ovirtclient.VMID(vmID)) + if err != nil { + return err + } + + if vm.MemoryPolicy().Ballooning() != tc.expectedMemoryBallooning { + return fmt.Errorf("Expected memory_ballooning to be %t, but got %t", + tc.expectedMemoryBallooning, + vm.MemoryPolicy().Ballooning()) + } + + return nil + }, + }, + { + Config: config, + Destroy: true, + }, + }, + }, + ) + }) + } +} + func TestSerialConsole(t *testing.T) { t.Parallel() no := false