fix(compute): read back computed instance fields; retry subnet delete

Live validation on a fully-featured CE lab (with a bootable image + working
Cinder backend) surfaced several real bugs in pcd_compute_instance that were
latent because the instance had never actually booted before.

instance flatten now reads back the server-computed fields it previously
ignored, so they are known after apply instead of erroring with "invalid result
object": availability_zone, security_groups (deduplicated — Nova can list the
default group once per port), and the network block's uuid/port. The network
block's inner uuid/port also gain UseStateForUnknown so an in-place flavor
change (resize) no longer re-plans them unknown and trips RequiresReplace, which
was replacing the instance instead of resizing it. metadata gains
UseStateForUnknown and its update is guarded, so an unset metadata map is no
longer pushed to Nova as `metadata: None` (a 400).

pcd_networking_subnet delete retries on 409 SubnetInUse to ride out Nova's
asynchronous release of an instance's ports after the instance is deleted.

The compute VM-boot acc tests (instance, interface_attach, volume_attach) now
resolve an existing image by name via a data source (env PCD_ACC_IMAGE_NAME) and
skip when it is unset, because PCD does not sync web-download (copy-from) images
to the hypervisor's local Glance — only library-uploaded images boot.

Validated live: instance boot + in-place resize + import, boot by image_name,
interface_attach, volume_attach (on the Synology Cinder backend), and
blockstorage volume.

Author: PF9-pushkar

CHANGELOG.md

@@ -42,9 +42,10 @@ All notable changes to this project are documented here. The format is based on
   (attach security groups to an unmanaged port, shared or exclusive via `enforce`); and
   `pcd_networking_floatingip_associate` (bind a pre-allocated floating IP to a port).
 - Compute (Nova v2): resources `pcd_compute_keypair`, `pcd_compute_flavor`,
-  `pcd_compute_servergroup` (acceptance-tested); `pcd_compute_instance` (code-complete —
-  boot verification is blocked on a lab image-library issue, see DECISIONS.md); data
+  `pcd_compute_servergroup`, `pcd_compute_instance` (boot, in-place resize, import); data
   sources `pcd_compute_flavor`, `pcd_compute_keypair`, `pcd_compute_availability_zones`.
+  The instance resource reads back all server-computed fields (availability zone,
+  security groups, network) after apply, and only pushes metadata when the user manages it.
 - Compute follow-ups: `pcd_compute_flavor` gains settable `extra_specs` (added/changed/
   removed in place; the flavor's other attributes are now correctly immutable);
   `pcd_compute_instance` supports in-place **resize** on a flavor change and booting by

DECISIONS.md

@@ -42,6 +42,28 @@ real backend fault):**
 - DNS `zone`/`recordset`: Designate returns `500 no_servers_configured` — the pool has no
   nameservers (BIND9 / `pools.yaml` from the runbook's Phase E step 7 not applied).
 
+### Follow-up run (2026-07-13): compute + Cinder unblocked
+
+After the lab operator uploaded a bootable image to the library and fixed the Cinder
+backend, the compute and block-storage suites were re-run. Block-storage volume passed.
+Compute boot passed once pointed at a library-synced image, and surfaced several real
+provider bugs in `pcd_compute_instance` (all fixed) that were latent because the instance
+had never actually booted before:
+- `flatten` did not read back `availability_zone`, `security_groups`, or the `network`
+  block, so they stayed unknown after apply ("invalid result object"). Now populated
+  (with the security-group list deduplicated, since Nova can repeat `default`).
+- The `network` block's inner `uuid`/`port` lacked `UseStateForUnknown`, so an in-place
+  flavor change re-planned them unknown and tripped `RequiresReplace` — the resize
+  replaced the instance instead of resizing. Fixed.
+- `metadata` was pushed on every update even when unset, sending Nova a nil map
+  (`metadata: None` → 400). Fixed with `UseStateForUnknown` + a guarded update.
+- Separately, `pcd_networking_subnet` delete now retries on `409 SubnetInUse` to ride out
+  Nova's asynchronous port release after an instance is deleted.
+
+The compute VM-boot acc tests now resolve an existing image by name via a data source
+(env `PCD_ACC_IMAGE_NAME`); they skip when it is unset, since PCD does not sync
+web-download images to the hypervisor's local Glance.
+
 | Family | Item | Status |
 |---|---|---|
 | Provider core | password auth + project scope, self-signed TLS (`insecure`), `pcd_identity_auth_scope` | **VALIDATED** |
@@ -61,12 +83,12 @@ real backend fault):**
 | Networking | `pcd_networking_floatingip_associate` | **VALIDATED** (2026-07-12) — associates a separately-allocated floating IP to a port; acc test builds the router path and skips unless `PCD_ACC_EXTERNAL_NETWORK` set. |
 | Compute | `pcd_compute_keypair`, `_flavor`, `_servergroup` | **VALIDATED** |
 | Compute (DS) | `pcd_compute_flavor`, `_keypair`, `_availability_zones` | **VALIDATED** |
-| Compute | `pcd_compute_instance` (boot) | **PENDING** — lab image-library gap: images don't reach the onboarded host's local library → nova returns HTTP 204 for image data. Create/schedule/wait/error-report verified; passes with a library-backed image. |
-| Compute | `pcd_compute_flavor` `extra_specs` | **PENDING** — code-complete; in-place add/change/remove; other flavor attrs now correctly force replacement. Acc test written (create + in-place update + import). Not yet run live (credentials unavailable this session). No lab blocker expected — flavors work on the lab. |
-| Compute | `pcd_compute_instance` resize + `image_name` | **PENDING** — code-complete; flavor change → Nova resize/confirm (revert on failure); `image_name` resolved via Glance. Boot-blocked (same as instance boot above). |
-| Compute | `pcd_compute_interface_attach` | **PENDING** — code-complete; needs a booted instance (boot-blocked). Acc test written. |
-| Compute | `pcd_compute_volume_attach` | **PENDING** — code-complete; needs a booted instance **and** a Cinder backend (both lab-blocked). Best-effort volume waiter degrades gracefully without Cinder. |
-| Block storage | `pcd_blockstorage_volume` | **PENDING** — no Cinder storage backend on the lab (`storageBackends={}`); volumes go `creating → error`. Create + waiter + error-detection verified. |
+| Compute | `pcd_compute_instance` (boot) | **VALIDATED** (2026-07-13) — boots against a library-synced image. The acc test resolves an existing image by name via a data source (env `PCD_ACC_IMAGE_NAME`), because PCD does not sync web-download (`copy-from`) images to the hypervisor's local Glance. |
+| Compute | `pcd_compute_flavor` `extra_specs` | **VALIDATED** — in-place add/change/remove; other flavor attrs force replacement. |
+| Compute | `pcd_compute_instance` resize + `image_name` | **VALIDATED** (2026-07-13) — flavor change resizes in place (verified same instance ID); `image_name` resolved via Glance. |
+| Compute | `pcd_compute_interface_attach` | **VALIDATED** (2026-07-13) — hot-attach a second NIC + import. |
+| Compute | `pcd_compute_volume_attach` | **VALIDATED** (2026-07-13) — boot + Cinder volume attach + import. |
+| Block storage | `pcd_blockstorage_volume` | **VALIDATED** (2026-07-13) — create on the `synology-iscsi` backend → available → extend → import. |
 | Block storage (DS) | `pcd_blockstorage_volume`, `_snapshot` | **PENDING** — untestable without volumes on this lab. |
 | Load balancing (Octavia) | `pcd_lb_loadbalancer`, `_listener`, `_pool`, `_member`, `_monitor` + `_loadbalancer` DS | **VALIDATED** (2026-07-12) — full-tree apply on OVN (lb + TCP listener + `SOURCE_IP_PORT` pool + member + TCP monitor + DS + rename + import). **PCD ships the OVN provider only** (`providers=[ovn]`, no amphora), which is L4 — use TCP/UDP/SCTP listeners and OVN pool algorithms; L7 policy/rule resources were removed (see backout entry). `loadbalancer_provider` was added (default `ovn`) because Octavia's server-side default `amphora` is not enabled — without it every create failed. |
 | DNS (Designate) | `pcd_dns_zone`, `pcd_dns_recordset` + `pcd_dns_zone` DS | **PENDING** — Phase 3, code-complete; async create/update/delete → wait-for-`ACTIVE`/404. Acc test (zone + recordset + import) + examples written. Designate is live on the lab (Step 0) and DNS needs no compute/storage backend, so this should pass live — not yet run this session (credentials unavailable). |

internal/services/compute/instance_resource.go

@@ -24,6 +24,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/mapplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
@@ -98,7 +99,7 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
 				MarkdownDescription: "Names of security groups to associate. Changing this forces a new resource.",
 				PlanModifiers:       []planmodifier.Set{setplanmodifier.RequiresReplace(), setplanmodifier.UseStateForUnknown()},
 			},
-			"metadata":          schema.MapAttribute{Optional: true, Computed: true, ElementType: types.StringType, MarkdownDescription: "Key-value metadata attached to the instance."},
+			"metadata":          schema.MapAttribute{Optional: true, Computed: true, ElementType: types.StringType, MarkdownDescription: "Key-value metadata attached to the instance.", PlanModifiers: []planmodifier.Map{mapplanmodifier.UseStateForUnknown()}},
 			"user_data":         schema.StringAttribute{Optional: true, MarkdownDescription: "User data (cloud-init) for the instance. Changing this forces a new resource.", PlanModifiers: fn},
 			"availability_zone": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "Availability zone to launch in. Changing this forces a new resource.", PlanModifiers: fnC},
 			"config_drive":      schema.BoolAttribute{Optional: true, MarkdownDescription: "Whether to use a config drive. Changing this forces a new resource.", PlanModifiers: []planmodifier.Bool{}},
@@ -110,9 +111,9 @@ func (r *instanceResource) Schema(_ context.Context, _ resource.SchemaRequest, r
 			"network": schema.ListNestedBlock{
 				MarkdownDescription: "Networks to attach. Changing this forces a new resource.",
 				NestedObject: schema.NestedBlockObject{Attributes: map[string]schema.Attribute{
-					"uuid": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "Network UUID to attach to (required unless port is set)."},
+					"uuid": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "Network UUID to attach to (required unless port is set).", PlanModifiers: stable},
 					"name": schema.StringAttribute{Optional: true, MarkdownDescription: "Network name (informational)."},
-					"port": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "Existing port to attach (required unless uuid is set)."},
+					"port": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "Existing port to attach (required unless uuid is set).", PlanModifiers: stable},
 				}},
 				PlanModifiers: []planmodifier.List{listplanmodifier.RequiresReplace(), listplanmodifier.UseStateForUnknown()},
 			},
@@ -273,13 +274,14 @@ func (r *instanceResource) Update(ctx context.Context, req resource.UpdateReques
 		}
 	}
 
-	if !plan.Metadata.Equal(state.Metadata) {
-		var meta map[string]string
-		if !plan.Metadata.IsNull() && !plan.Metadata.IsUnknown() {
-			resp.Diagnostics.Append(plan.Metadata.ElementsAs(ctx, &meta, false)...)
-			if resp.Diagnostics.HasError() {
-				return
-			}
+	// Only push metadata when the user actually set it and it changed. A null or
+	// unknown plan value means "unmanaged" — Nova rejects a nil metadata map
+	// (metadata: None) with a 400.
+	if !plan.Metadata.IsNull() && !plan.Metadata.IsUnknown() && !plan.Metadata.Equal(state.Metadata) {
+		meta := map[string]string{}
+		resp.Diagnostics.Append(plan.Metadata.ElementsAs(ctx, &meta, false)...)
+		if resp.Diagnostics.HasError() {
+			return
 		}
 		if _, err := servers.UpdateMetadata(ctx, client, plan.ID.ValueString(), servers.MetadataOpts(meta)).Extract(); err != nil {
 			resp.Diagnostics.AddError("compute: updating instance metadata", err.Error())
@@ -390,6 +392,43 @@ func (r *instanceResource) flatten(ctx context.Context, server *servers.Server,
 	diags = append(diags, d...)
 	m.Metadata = metaMap
 
+	// availability_zone and security_groups are Optional+Computed: when the user
+	// omits them the server assigns values, which must be read back or they stay
+	// unknown after apply ("invalid result object").
+	m.AvailabilityZone = types.StringValue(server.AvailabilityZone)
+
+	seenSG := make(map[string]bool, len(server.SecurityGroups))
+	sgs := make([]string, 0, len(server.SecurityGroups))
+	for _, sg := range server.SecurityGroups {
+		// Nova can list the same security group once per attached port; dedupe so
+		// the Set is valid.
+		if name, ok := sg["name"].(string); ok && !seenSG[name] {
+			seenSG[name] = true
+			sgs = append(sgs, name)
+		}
+	}
+	sgSet, d := types.SetValueFrom(ctx, types.StringType, sgs)
+	diags = append(diags, d...)
+	m.SecurityGroups = sgSet
+
+	// The network block's uuid/port are Optional+Computed; resolve any that the
+	// user left unset (unknown) to a concrete value so the block is fully known.
+	if !m.Network.IsNull() && !m.Network.IsUnknown() {
+		var blocks []instanceNetworkModel
+		diags = append(diags, m.Network.ElementsAs(ctx, &blocks, false)...)
+		for i := range blocks {
+			if blocks[i].UUID.IsNull() || blocks[i].UUID.IsUnknown() {
+				blocks[i].UUID = types.StringValue("")
+			}
+			if blocks[i].Port.IsNull() || blocks[i].Port.IsUnknown() {
+				blocks[i].Port = types.StringValue("")
+			}
+		}
+		netList, d := types.ListValueFrom(ctx, m.Network.ElementType(ctx), blocks)
+		diags = append(diags, d...)
+		m.Network = netList
+	}
+
 	if m.Region.IsNull() || m.Region.IsUnknown() {
 		m.Region = types.StringValue(r.config.Region)
 	}

internal/services/compute/instance_test.go

@@ -18,18 +18,24 @@ import (
 	"github.com/platform9/terraform-provider-pcd/internal/acctest"
 )
 
-// defaultTestImageURL is a small bootable qcow2 (CirrOS) fetched by Glance via
-// web-download, which bypasses the ingress upload-size cap. Override with
-// PCD_ACC_IMAGE_URL.
-const defaultTestImageURL = "https://download.cirros-cloud.net/0.6.2/cirros-0.6.2-x86_64-disk.img"
+// testAccBootImageName returns the name of a pre-synced bootable image to boot
+// VMs from, taken from PCD_ACC_IMAGE_NAME. PCD's image library does not propagate
+// web-download (copy-from) images to the hypervisor's local Glance, so the VM
+// boot tests require an image already uploaded to the library (e.g. a CirrOS
+// image). The test skips when the variable is unset.
+func testAccBootImageName(t *testing.T) string {
+	t.Helper()
+	name := os.Getenv("PCD_ACC_IMAGE_NAME")
+	if name == "" {
+		t.Skip("PCD_ACC_IMAGE_NAME not set; skipping VM-boot test (needs a bootable image already in the image library)")
+	}
+	return name
+}
 
 // TestAccComputeInstance_basic boots a real VM through Terraform end to end:
 // network + subnet + Glance image (web-download) + keypair + instance on m1.tiny.
 func TestAccComputeInstance_basic(t *testing.T) {
-	imageURL := os.Getenv("PCD_ACC_IMAGE_URL")
-	if imageURL == "" {
-		imageURL = defaultTestImageURL
-	}
+	imageName := testAccBootImageName(t)
 	const rn = "pcd_compute_instance.test"
 	var instanceID, flavorID string
 
@@ -39,7 +45,7 @@ func TestAccComputeInstance_basic(t *testing.T) {
 		CheckDestroy:             testAccCheckInstanceDestroy(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccInstanceConfig(imageURL, "tf-acc-instance", "m1.tiny"),
+				Config: testAccInstanceConfig(imageName, "tf-acc-instance", "m1.tiny"),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckInstanceExists(t, rn),
 					testAccCaptureID(rn, &instanceID),
@@ -51,12 +57,12 @@ func TestAccComputeInstance_basic(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccInstanceConfig(imageURL, "tf-acc-instance-renamed", "m1.tiny"),
+				Config: testAccInstanceConfig(imageName, "tf-acc-instance-renamed", "m1.tiny"),
 				Check:  resource.TestCheckResourceAttr(rn, "name", "tf-acc-instance-renamed"),
 			},
 			{
 				// Resize: change flavor in place (same instance ID, new flavor_id).
-				Config: testAccInstanceConfig(imageURL, "tf-acc-instance-renamed", "m1.small"),
+				Config: testAccInstanceConfig(imageName, "tf-acc-instance-renamed", "m1.small"),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttrWith(rn, "id", func(v string) error {
 						if v != instanceID {
@@ -79,10 +85,7 @@ func TestAccComputeInstance_basic(t *testing.T) {
 // TestAccComputeInstance_imageName boots an instance referencing its image by
 // name (resolved via Glance) instead of image_id.
 func TestAccComputeInstance_imageName(t *testing.T) {
-	imageURL := os.Getenv("PCD_ACC_IMAGE_URL")
-	if imageURL == "" {
-		imageURL = defaultTestImageURL
-	}
+	imageName := testAccBootImageName(t)
 	const rn = "pcd_compute_instance.test"
 
 	resource.Test(t, resource.TestCase{
@@ -91,20 +94,24 @@ func TestAccComputeInstance_imageName(t *testing.T) {
 		CheckDestroy:             testAccCheckInstanceDestroy(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccInstanceImageNameConfig(imageURL),
+				Config: testAccInstanceImageNameConfig(imageName),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckInstanceExists(t, rn),
 					resource.TestCheckResourceAttr(rn, "status", "ACTIVE"),
 					// image_id is Computed and gets populated from the resolved image_name.
-					resource.TestCheckResourceAttrPair(rn, "image_id", "pcd_images_image.test", "id"),
+					resource.TestCheckResourceAttrPair(rn, "image_id", "data.pcd_images_image.boot", "id"),
 				),
 			},
 		},
 	})
 }
 
-func testAccInstanceConfig(imageURL, name, flavorName string) string {
+func testAccInstanceConfig(imageName, name, flavorName string) string {
 	return fmt.Sprintf(`
+data "pcd_images_image" "boot" {
+  name = %q
+}
+
 resource "pcd_networking_network" "test" {
   name = "tf-acc-inst-net"
 }
@@ -114,20 +121,13 @@ resource "pcd_networking_subnet" "test" {
   cidr       = "10.103.0.0/24"
 }
 
-resource "pcd_images_image" "test" {
-  name             = "tf-acc-inst-img"
-  container_format = "bare"
-  disk_format      = "qcow2"
-  image_source_url = %q
-}
-
 resource "pcd_compute_keypair" "test" {
   name = "tf-acc-inst-key"
 }
 
 resource "pcd_compute_instance" "test" {
   name        = %q
-  image_id    = pcd_images_image.test.id
+  image_id    = data.pcd_images_image.boot.id
   flavor_name = %q
   key_pair    = pcd_compute_keypair.test.name
 
@@ -137,11 +137,15 @@ resource "pcd_compute_instance" "test" {
 
   depends_on = [pcd_networking_subnet.test]
 }
-`, imageURL, name, flavorName)
+`, imageName, name, flavorName)
 }
 
-func testAccInstanceImageNameConfig(imageURL string) string {
+func testAccInstanceImageNameConfig(imageName string) string {
 	return fmt.Sprintf(`
+data "pcd_images_image" "boot" {
+  name = %[1]q
+}
+
 resource "pcd_networking_network" "test" {
   name = "tf-acc-instn-net"
 }
@@ -151,16 +155,9 @@ resource "pcd_networking_subnet" "test" {
   cidr       = "10.113.0.0/24"
 }
 
-resource "pcd_images_image" "test" {
-  name             = "tf-acc-instn-img"
-  container_format = "bare"
-  disk_format      = "qcow2"
-  image_source_url = %q
-}
-
 resource "pcd_compute_instance" "test" {
   name        = "tf-acc-instn"
-  image_name  = pcd_images_image.test.name
+  image_name  = %[1]q
   flavor_name = "m1.tiny"
 
   network {
@@ -169,7 +166,7 @@ resource "pcd_compute_instance" "test" {
 
   depends_on = [pcd_networking_subnet.test]
 }
-`, imageURL)
+`, imageName)
 }
 
 // testAccCaptureAttr records a resource attribute value for later comparison.