From 953e7e302ba52fdb3fae118466fea86a3ba2b4f2 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Tue, 14 Jun 2022 13:19:59 +0200 Subject: [PATCH] added bootable and active fields to disk_attachment (#439) --- docs/resources/disk_attachment.md | 7 + docs/resources/disk_attachments.md | 7 + .../ovirt_disk_attachment/resource.tf | 2 + .../ovirt_disk_attachments/resource.tf | 2 + go.mod | 2 +- go.sum | 4 +- .../ovirt/resource_ovirt_disk_attachment.go | 45 +++- .../resource_ovirt_disk_attachment_test.go | 147 +++++++---- .../ovirt/resource_ovirt_disk_attachments.go | 39 ++- .../resource_ovirt_disk_attachments_test.go | 244 +++++++++++------- 10 files changed, 352 insertions(+), 147 deletions(-) diff --git a/docs/resources/disk_attachment.md b/docs/resources/disk_attachment.md index ced0e303..f3715ecc 100644 --- a/docs/resources/disk_attachment.md +++ b/docs/resources/disk_attachment.md @@ -38,6 +38,8 @@ resource "ovirt_disk_attachment" "test" { vm_id = ovirt_vm.test.id disk_id = ovirt_disk.test.id disk_interface = "virtio_scsi" + bootable = true + active = true } ``` @@ -50,6 +52,11 @@ resource "ovirt_disk_attachment" "test" { - `disk_interface` (String) Type of interface to use for attaching disk. One of: `ide`, `sata`, `spapr_vscsi`, `virtio`, `virtio_scsi`. - `vm_id` (String) ID of the VM the disk should be attached to. +### Optional + +- `active` (Boolean) Defines whether the disk is active in the virtual machine it is attached to. +- `bootable` (Boolean) Defines whether the disk is bootable. + ### Read-Only - `id` (String) The ID of this resource. diff --git a/docs/resources/disk_attachments.md b/docs/resources/disk_attachments.md index df40a318..ec0f0acd 100644 --- a/docs/resources/disk_attachments.md +++ b/docs/resources/disk_attachments.md @@ -44,6 +44,8 @@ resource "ovirt_disk_attachments" "test" { attachment { disk_id = ovirt_disk.test.id disk_interface = "virtio_scsi" + bootable = true + active = true } } ``` @@ -75,6 +77,11 @@ Required: - `disk_id` (String) ID of the disk to attach. This disk must either be shared or not yet attached to a different VM. - `disk_interface` (String) Type of interface to use for attaching disk. One of: `ide`, `sata`, `spapr_vscsi`, `virtio`, `virtio_scsi`. +Optional: + +- `active` (Boolean) Defines whether the disk is active in the virtual machine it is attached to. +- `bootable` (Boolean) Defines whether the disk is bootable. + Read-Only: - `id` (String) The ID of this resource. diff --git a/examples/resources/ovirt_disk_attachment/resource.tf b/examples/resources/ovirt_disk_attachment/resource.tf index bb162a89..5dd5b20d 100644 --- a/examples/resources/ovirt_disk_attachment/resource.tf +++ b/examples/resources/ovirt_disk_attachment/resource.tf @@ -20,4 +20,6 @@ resource "ovirt_disk_attachment" "test" { vm_id = ovirt_vm.test.id disk_id = ovirt_disk.test.id disk_interface = "virtio_scsi" + bootable = true + active = true } \ No newline at end of file diff --git a/examples/resources/ovirt_disk_attachments/resource.tf b/examples/resources/ovirt_disk_attachments/resource.tf index b9386fd5..f81499b5 100644 --- a/examples/resources/ovirt_disk_attachments/resource.tf +++ b/examples/resources/ovirt_disk_attachments/resource.tf @@ -26,5 +26,7 @@ resource "ovirt_disk_attachments" "test" { attachment { disk_id = ovirt_disk.test.id disk_interface = "virtio_scsi" + bootable = true + active = true } } \ No newline at end of file diff --git a/go.mod b/go.mod index fe1b8996..a95e5f04 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/oklog/run v1.1.0 // indirect - github.com/ovirt/go-ovirt-client v1.0.0-beta2 + github.com/ovirt/go-ovirt-client v1.0.0-beta4 github.com/ovirt/go-ovirt-client-log/v3 v3.0.0 github.com/vmihailenco/tagparser v0.1.2 // indirect golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4 // indirect diff --git a/go.sum b/go.sum index 3283a799..0f75370d 100644 --- a/go.sum +++ b/go.sum @@ -233,8 +233,8 @@ github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA= github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU= github.com/ovirt/go-ovirt v0.0.0-20220427092237-114c47f2835c h1:jXRFpl7+W0YZj/fghoYuE4vJWW/KeQGvdrhnRwRGtAY= github.com/ovirt/go-ovirt v0.0.0-20220427092237-114c47f2835c/go.mod h1:Zkdj9/rW6eyuw0uOeEns6O3pP5G2ak+bI/tgkQ/tEZI= -github.com/ovirt/go-ovirt-client v1.0.0-beta2 h1:GszjqWO9he3h1Y4SUUAxz/5Wgev/p1A6MVIf7Z8eQXg= -github.com/ovirt/go-ovirt-client v1.0.0-beta2/go.mod h1:tv8E2pxUkggayDAgMLuQHzcNtzt8RFvnhO5V5b/5X4U= +github.com/ovirt/go-ovirt-client v1.0.0-beta4 h1:A8MFK4Y4ZZ3Ad7N6Q0obqJl8r2Pai1FmzD193WwBpoo= +github.com/ovirt/go-ovirt-client v1.0.0-beta4/go.mod h1:tv8E2pxUkggayDAgMLuQHzcNtzt8RFvnhO5V5b/5X4U= github.com/ovirt/go-ovirt-client-log/v3 v3.0.0 h1:uvACVHYhYPMkNJrrgWiABcfELB6qoFfsDDUTbpb4Jv4= github.com/ovirt/go-ovirt-client-log/v3 v3.0.0/go.mod h1:chKKxCv4lRjxezrTG+EIhkWXGhDAWByglPVXh/iYdnQ= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/internal/ovirt/resource_ovirt_disk_attachment.go b/internal/ovirt/resource_ovirt_disk_attachment.go index 3bbc2baa..c1f99426 100644 --- a/internal/ovirt/resource_ovirt_disk_attachment.go +++ b/internal/ovirt/resource_ovirt_disk_attachment.go @@ -39,6 +39,20 @@ var diskAttachmentSchema = map[string]*schema.Schema{ ForceNew: true, ValidateDiagFunc: validateDiskInterface, }, + "bootable": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + Description: "Defines whether the disk is bootable.", + }, + "active": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + Description: "Defines whether the disk is active in the virtual machine it is attached to.", + }, } func (p *provider) diskAttachmentResource() *schema.Resource { @@ -66,11 +80,34 @@ func (p *provider) diskAttachmentCreate( diskID := data.Get("disk_id").(string) diskInterface := data.Get("disk_interface").(string) + var err error + var diags diag.Diagnostics + params := ovirtclient.CreateDiskAttachmentParams() + params, err = params.WithBootable(data.Get("bootable").(bool)) + if err != nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to set bootable flag for disk attachment.", + Detail: err.Error(), + }) + } + params, err = params.WithActive(data.Get("active").(bool)) + if err != nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to set active flag for disk attachment.", + Detail: err.Error(), + }) + } + if diags.HasError() { + return diags + } + diskAttachment, err := client.CreateDiskAttachment( ovirtclient.VMID(vmID), ovirtclient.DiskID(diskID), ovirtclient.DiskInterface(diskInterface), - ovirtclient.CreateDiskAttachmentParams(), + params, ) if err != nil { return diag.Diagnostics{ @@ -164,6 +201,12 @@ func (p *provider) diskAttachmentImport( if err := data.Set("disk_interface", string(attachment.DiskInterface())); err != nil { return nil, fmt.Errorf("failed to set disk_interface to %s", attachment.DiskInterface()) } + if err := data.Set("bootable", attachment.Bootable()); err != nil { + return nil, fmt.Errorf("failed to set bootable to %v", attachment.Bootable()) + } + if err := data.Set("active", attachment.Active()); err != nil { + return nil, fmt.Errorf("failed to set active to %v", attachment.Active()) + } return []*schema.ResourceData{data}, nil } diff --git a/internal/ovirt/resource_ovirt_disk_attachment_test.go b/internal/ovirt/resource_ovirt_disk_attachment_test.go index c20b0739..fce05adc 100644 --- a/internal/ovirt/resource_ovirt_disk_attachment_test.go +++ b/internal/ovirt/resource_ovirt_disk_attachment_test.go @@ -18,52 +18,111 @@ func TestDiskAttachmentResource(t *testing.T) { clusterID := p.getTestHelper().GetClusterID() templateID := p.getTestHelper().GetBlankTemplateID() - resource.UnitTest( - t, resource.TestCase{ - ProviderFactories: p.getProviderFactories(), - Steps: []resource.TestStep{ - { - Config: fmt.Sprintf( - ` -provider "ovirt" { - mock = true -} - -resource "ovirt_disk" "test" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} - -resource "ovirt_vm" "test" { - cluster_id = "%s" - template_id = "%s" - name = "test" -} + baseConfig := fmt.Sprintf(` + provider "ovirt" { + mock = true + } + + resource "ovirt_disk" "test" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } + + resource "ovirt_vm" "test" { + cluster_id = "%s" + template_id = "%s" + name = "test" + }`, + storageDomainID, + clusterID, + templateID, + ) -resource "ovirt_disk_attachment" "test" { - vm_id = ovirt_vm.test.id - disk_id = ovirt_disk.test.id - disk_interface = "virtio_scsi" -} -`, - storageDomainID, - clusterID, - templateID, - ), - Check: resource.ComposeTestCheckFunc( - resource.TestMatchResourceAttr( - "ovirt_disk_attachment.test", - "id", - regexp.MustCompile("^.+$"), - ), - ), - }, - }, + testcases := []struct { + name string + inputBootable string + inputActive string + expectedBootable bool + expectedActive bool + }{ + { + name: "all set to true", + inputBootable: "true", + inputActive: "true", + expectedBootable: true, + expectedActive: true, }, - ) + { + name: "all set to false", + inputBootable: "false", + inputActive: "false", + expectedBootable: false, + expectedActive: false, + }, + { + name: "using defaults", + inputBootable: "null", + inputActive: "null", + expectedBootable: false, + expectedActive: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + resource.UnitTest( + t, resource.TestCase{ + ProviderFactories: p.getProviderFactories(), + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + %s + + resource "ovirt_disk_attachment" "test" { + vm_id = ovirt_vm.test.id + disk_id = ovirt_disk.test.id + disk_interface = "virtio_scsi" + bootable = %s + active = %s + }`, + baseConfig, + testcase.inputBootable, + testcase.inputActive, + ), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr( + "ovirt_disk_attachment.test", + "id", + regexp.MustCompile("^.+$"), + ), + func(s *terraform.State) error { + VMID := s.RootModule().Resources["ovirt_vm.test"].Primary.ID + diskAttachmentID := s.RootModule().Resources["ovirt_disk_attachment.test"].Primary.ID + diskAttachment, err := p.getTestHelper().GetClient().GetDiskAttachment(ovirtclient.VMID(VMID), ovirtclient.DiskAttachmentID(diskAttachmentID)) + if err != nil { + return err + } + if diskAttachment.DiskInterface() != "virtio_scsi" { + return fmt.Errorf("Expected disk_interface 'virtio_scsi', but got '%s'", diskAttachment.DiskInterface()) + } + if diskAttachment.Bootable() != testcase.expectedActive { + return fmt.Errorf("Expected bootable to be %t, but got %t", testcase.expectedBootable, diskAttachment.Bootable()) + } + if diskAttachment.Active() != testcase.expectedActive { + return fmt.Errorf("Expected active to be %t, but got %t", testcase.expectedActive, diskAttachment.Active()) + } + return nil + }, + ), + }, + }, + }, + ) + }) + } } func TestDiskAttachmentResourceImport(t *testing.T) { diff --git a/internal/ovirt/resource_ovirt_disk_attachments.go b/internal/ovirt/resource_ovirt_disk_attachments.go index 6636c299..d3bb27b9 100644 --- a/internal/ovirt/resource_ovirt_disk_attachments.go +++ b/internal/ovirt/resource_ovirt_disk_attachments.go @@ -43,6 +43,20 @@ var diskAttachmentsSchema = map[string]*schema.Schema{ ForceNew: false, ValidateDiagFunc: validateDiskInterface, }, + "bootable": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + Description: "Defines whether the disk is bootable.", + }, + "active": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + Description: "Defines whether the disk is active in the virtual machine it is attached to.", + }, }, }, }, @@ -180,6 +194,8 @@ func (p *provider) createOrUpdateDiskAttachment( id := desiredAttachment["id"].(string) diskID := desiredAttachment["disk_id"].(string) diskInterfaceName := desiredAttachment["disk_interface"].(string) + bootable := desiredAttachment["bootable"].(bool) + active := desiredAttachment["active"].(bool) var foundExisting ovirtclient.DiskAttachment if id != "" { @@ -194,7 +210,10 @@ func (p *provider) createOrUpdateDiskAttachment( if foundExisting != nil { // If we found an existing attachment, check if all parameters match. Otherwise, remove the attachment // and let it be re-created below. - if foundExisting.DiskID() == ovirtclient.DiskID(diskID) && string(foundExisting.DiskInterface()) == diskInterfaceName { + if string(foundExisting.DiskID()) == diskID && + string(foundExisting.DiskInterface()) == diskInterfaceName && + foundExisting.Bootable() == bootable && + foundExisting.Active() == active { return nil } if err := foundExisting.Remove(); err != nil && !isNotFound(err) { @@ -208,6 +227,19 @@ func (p *provider) createOrUpdateDiskAttachment( desiredAttachment["id"] = "" desiredAttachment["disk_id"] = "" desiredAttachment["disk_interface"] = "" + desiredAttachment["bootable"] = false + desiredAttachment["active"] = false + } + + var err error + params := ovirtclient.CreateDiskAttachmentParams() + params, err = params.WithBootable(bootable) + if err != nil { + return errorToDiags("set bootable flag for disk attachment.", err) + } + params, err = params.WithActive(active) + if err != nil { + return errorToDiags("set active flag for disk attachment.", err) } // Create or re-create disk attachment, then set it in the Terraform state. @@ -215,7 +247,7 @@ func (p *provider) createOrUpdateDiskAttachment( ovirtclient.VMID(vmID), ovirtclient.DiskID(diskID), ovirtclient.DiskInterface(diskInterfaceName), - nil, + params, ) if err != nil { return errorToDiags( @@ -226,6 +258,9 @@ func (p *provider) createOrUpdateDiskAttachment( desiredAttachment["id"] = attachment.ID() desiredAttachment["disk_id"] = attachment.DiskID() desiredAttachment["disk_interface"] = string(attachment.DiskInterface()) + desiredAttachment["bootable"] = attachment.Bootable() + desiredAttachment["active"] = attachment.Active() + return nil } diff --git a/internal/ovirt/resource_ovirt_disk_attachments_test.go b/internal/ovirt/resource_ovirt_disk_attachments_test.go index f0aa730c..2369b971 100644 --- a/internal/ovirt/resource_ovirt_disk_attachments_test.go +++ b/internal/ovirt/resource_ovirt_disk_attachments_test.go @@ -23,47 +23,49 @@ func TestDiskAttachmentsResource(t *testing.T) { ProviderFactories: p.getProviderFactories(), Steps: []resource.TestStep{ { - Config: fmt.Sprintf( - ` -provider "ovirt" { - mock = true -} + Config: fmt.Sprintf(` + provider "ovirt" { + mock = true + } -resource "ovirt_disk" "test1" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} + resource "ovirt_disk" "test1" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } -resource "ovirt_disk" "test2" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} + resource "ovirt_disk" "test2" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } -resource "ovirt_vm" "test" { - cluster_id = "%s" - template_id = "%s" - name = "test" -} + resource "ovirt_vm" "test" { + cluster_id = "%s" + template_id = "%s" + name = "test" + } -resource "ovirt_disk_attachments" "test" { - vm_id = ovirt_vm.test.id - - attachment { - disk_id = ovirt_disk.test1.id - disk_interface = "virtio_scsi" - } - attachment { - disk_id = ovirt_disk.test2.id - disk_interface = "virtio_scsi" - } -} -`, + resource "ovirt_disk_attachments" "test" { + vm_id = ovirt_vm.test.id + + attachment { + disk_id = ovirt_disk.test1.id + disk_interface = "virtio_scsi" + bootable = true + active = true + } + attachment { + disk_id = ovirt_disk.test2.id + disk_interface = "virtio_scsi" + bootable = null + active = null + } + }`, storageDomainID, storageDomainID, clusterID, @@ -75,46 +77,78 @@ resource "ovirt_disk_attachments" "test" { "attachment.#", regexp.MustCompile("^2$"), ), + func(s *terraform.State) error { + VMID := s.RootModule().Resources["ovirt_vm.test"].Primary.ID + disk1ID := s.RootModule().Resources["ovirt_disk.test1"].Primary.ID + disk2ID := s.RootModule().Resources["ovirt_disk.test2"].Primary.ID + + vm, err := p.getTestHelper().GetClient().GetVM(ovirtclient.VMID(VMID)) + if err != nil { + return err + } + + attachments, err := vm.ListDiskAttachments() + if err != nil { + return err + } + + for _, attachment := range attachments { + if string(attachment.DiskID()) == disk1ID { + if !attachment.Active() || !attachment.Bootable() { + return fmt.Errorf("Attachment for disk %s: Expected (bootable:%t,active:%t), but got (bootable:%t,active:%t)", + disk1ID, true, true, attachments[0].Active(), attachments[0].Bootable()) + } + continue + } + if string(attachment.DiskID()) == disk2ID { + if attachment.Active() || attachment.Bootable() { + return fmt.Errorf("Attachment for disk %s: Expected (bootable:%t,active:%t), but got (bootable:%t,active:%t)", disk2ID, false, false, attachments[1].Active(), attachments[1].Bootable()) + } + continue + } + return fmt.Errorf("Unknown attachment found: %v", attachment) + } + + return nil + }, ), }, { - Config: fmt.Sprintf( - ` -provider "ovirt" { - mock = true -} + Config: fmt.Sprintf(` + provider "ovirt" { + mock = true + } -resource "ovirt_disk" "test1" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} + resource "ovirt_disk" "test1" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } -resource "ovirt_disk" "test2" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} + resource "ovirt_disk" "test2" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } -resource "ovirt_vm" "test" { - cluster_id = "%s" - template_id = "%s" - name = "test" -} + resource "ovirt_vm" "test" { + cluster_id = "%s" + template_id = "%s" + name = "test" + } -resource "ovirt_disk_attachments" "test" { - vm_id = ovirt_vm.test.id + resource "ovirt_disk_attachments" "test" { + vm_id = ovirt_vm.test.id - attachment { - disk_id = ovirt_disk.test1.id - disk_interface = "virtio_scsi" - } -} -`, + attachment { + disk_id = ovirt_disk.test1.id + disk_interface = "virtio_scsi" + } + }`, storageDomainID, storageDomainID, clusterID, @@ -126,6 +160,25 @@ resource "ovirt_disk_attachments" "test" { "attachment.#", regexp.MustCompile("^1$"), ), + func(s *terraform.State) error { + VMID := s.RootModule().Resources["ovirt_vm.test"].Primary.ID + + vm, err := p.getTestHelper().GetClient().GetVM(ovirtclient.VMID(VMID)) + if err != nil { + return err + } + + attachments, err := vm.ListDiskAttachments() + if err != nil { + return err + } + + if attachments[0].Active() || attachments[0].Bootable() { + return fmt.Errorf("Expected (bootable:%t,active:%t), but got (bootable:%t,active:%t)", false, false, attachments[0].Active(), attachments[0].Bootable()) + } + + return nil + }, ), }, }, @@ -142,42 +195,39 @@ func TestDiskAttachmentsResourceImport(t *testing.T) { templateID := p.getTestHelper().GetBlankTemplateID() client := p.getTestHelper().GetClient() - configPart1 := fmt.Sprintf( - ` -provider "ovirt" { - mock = true -} + configPart1 := fmt.Sprintf(` + provider "ovirt" { + mock = true + } -resource "ovirt_disk" "test" { - storage_domain_id = "%s" - format = "raw" - size = 1048576 - alias = "test" - sparse = true -} + resource "ovirt_disk" "test" { + storage_domain_id = "%s" + format = "raw" + size = 1048576 + alias = "test" + sparse = true + } -resource "ovirt_vm" "test" { - cluster_id = "%s" - template_id = "%s" - name = "test" -} -`, + resource "ovirt_vm" "test" { + cluster_id = "%s" + template_id = "%s" + name = "test" + }`, storageDomainID, clusterID, templateID, ) configPart2 := fmt.Sprintf(` -%s - -resource "ovirt_disk_attachments" "test" { - vm_id = ovirt_vm.test.id - attachment { - disk_id = ovirt_disk.test.id - disk_interface = "virtio_scsi" - } -} -`, configPart1) + %s + + resource "ovirt_disk_attachments" "test" { + vm_id = ovirt_vm.test.id + attachment { + disk_id = ovirt_disk.test.id + disk_interface = "virtio_scsi" + } + }`, configPart1) resource.UnitTest( t, resource.TestCase{