Skip to content

Commit

Permalink
fix: memory_ballooning for false value properly set (#442)
Browse files Browse the repository at this point in the history
  • Loading branch information
engelmi authored Jun 17, 2022
1 parent 59f4237 commit b2201aa
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 4 deletions.
5 changes: 4 additions & 1 deletion internal/ovirt/resource_ovirt_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
74 changes: 74 additions & 0 deletions internal/ovirt/resource_ovirt_disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
),
},
},
})
})
}
}
15 changes: 12 additions & 3 deletions internal/ovirt/resource_ovirt_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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))
Expand Down
84 changes: 84 additions & 0 deletions internal/ovirt/resource_ovirt_vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b2201aa

Please sign in to comment.