Merge pull request #46 from platform9/pushkar/hostconfig-strand-guard

Refuse to delete a host config a host is still assigned to

Author: PF9-pushkar

CHANGELOG.md

@@ -6,6 +6,21 @@ All notable changes to this project are documented here. The format is based on
 
 ## [Unreleased]
 
+## [0.1.8] - 2026-08-18
+
+### Changed
+
+- `pcd_host_config`: destroying one is now refused while any host is still assigned to it, and the
+  check is fail-closed — if resmgr will not answer, the delete does not proceed. PCD accepts that
+  delete, keeps the assignment, and from then on refuses to remove it (404) and refuses every
+  re-assignment (409 `HostToHostconfigConflict`); deleting the host record does not clear it and the
+  id cannot be re-created, so the host can never be assigned a host configuration — never onboarded —
+  again. Remove the `pcd_host_config_assignment` first. The guard will be lifted once resmgr refuses
+  the unsafe delete itself.
+- `pcd_host_config_assignment`: destroying one now confirms the host has actually stopped reporting
+  the host configuration instead of trusting the `204`, which resmgr returns whether or not it
+  unbound anything. A binding Terraform believed was gone is what left a host strandable.
+
 ## [0.1.7] - 2026-08-18
 
 ### Fixed

docs/resources/host_config.md

@@ -3,12 +3,12 @@
 page_title: "pcd_host_config Resource - PCD"
 subcategory: "Cluster Blueprint"
 description: |-
-  Manages a PCD host configuration: the mapping of traffic types (management, VM console, tunnels, image library, live migration, host liveness) to network interfaces, plus physical-network labels.
+  Manages a PCD host configuration: the mapping of traffic types (management, VM console,  Destroying one is refused while any host is still assigned to it: PCD leaves such a host unable to be assigned a host configuration ever again, so remove the pcd_host_config_assignment first.tunnels, image library, live migration, host liveness) to network interfaces, plus physical-network labels.
 ---
 
 # pcd_host_config (Resource)
 
-Manages a PCD host configuration: the mapping of traffic types (management, VM console, tunnels, image library, live migration, host liveness) to network interfaces, plus physical-network labels.
+Manages a PCD host configuration: the mapping of traffic types (management, VM console,  Destroying one is refused while any host is still assigned to it: PCD leaves such a host unable to be assigned a host configuration ever again, so remove the `pcd_host_config_assignment` first.tunnels, image library, live migration, host liveness) to network interfaces, plus physical-network labels.
 
 ## Example Usage
 

internal/services/resmgr/absence_internal_test.go

@@ -7,7 +7,9 @@ import (
 	"context"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/gophercloud/gophercloud/v2"
 )
@@ -87,3 +89,90 @@ func TestIsNullJSON(t *testing.T) {
 		}
 	}
 }
+
+// hostList serves a resmgr host list, switching to `then` after the first request so a
+// test can watch a binding clear.
+func hostList(t *testing.T, first, then string) *gophercloud.ServiceClient {
+	t.Helper()
+	var n int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		body := first
+		if n > 0 && then != "" {
+			body = then
+		}
+		n++
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(body))
+	}))
+	t.Cleanup(srv.Close)
+	return &gophercloud.ServiceClient{ProviderClient: &gophercloud.ProviderClient{}, Endpoint: srv.URL + "/"}
+}
+
+const twoHosts = `[{"id":"host-a","hostconfig_id":"hc-1"},{"id":"host-b","hostconfig_id":"hc-2"}]`
+
+func TestHostsAssignedTo(t *testing.T) {
+	for _, tc := range []struct {
+		name, body, hostConfig string
+		want                   []string
+	}{
+		{name: "one host carries it", body: twoHosts, hostConfig: "hc-1", want: []string{"host-a"}},
+		{name: "nobody carries it", body: twoHosts, hostConfig: "hc-9"},
+		{name: "an empty region", body: `[]`, hostConfig: "hc-1"},
+		// getJSON reads a null body as absence; for a collection it is simply empty, and
+		// mistaking the two here would report a bound host config as safe to delete.
+		{name: "a null list is empty, not an error", body: `null`, hostConfig: "hc-1"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := hostsAssignedTo(context.Background(), hostList(t, tc.body, ""), tc.hostConfig)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if len(got) != len(tc.want) {
+				t.Fatalf("got %v, want %v", got, tc.want)
+			}
+			for i := range got {
+				if got[i] != tc.want[i] {
+					t.Fatalf("got %v, want %v", got, tc.want)
+				}
+			}
+		})
+	}
+}
+
+func TestHostsAssignedToSurfacesAFailedCheck(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+	client := &gophercloud.ServiceClient{ProviderClient: &gophercloud.ProviderClient{}, Endpoint: srv.URL + "/"}
+	// The delete guard is fail-closed, so this error has to reach it rather than read as
+	// "no hosts are assigned".
+	if _, err := hostsAssignedTo(context.Background(), client, "hc-1"); err == nil {
+		t.Fatal("got no error from a resmgr that would not answer; the guard would have deleted")
+	}
+}
+
+func TestWaitUnassigned(t *testing.T) {
+	interval, timeout := unassignPollInterval, unassignPollTimeout
+	unassignPollInterval, unassignPollTimeout = time.Millisecond, 50*time.Millisecond
+	defer func() { unassignPollInterval, unassignPollTimeout = interval, timeout }()
+
+	t.Run("returns once the binding clears", func(t *testing.T) {
+		client := hostList(t, twoHosts, `[{"id":"host-b","hostconfig_id":"hc-2"}]`)
+		if err := waitUnassigned(context.Background(), client, "host-a", "hc-1"); err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+	})
+
+	// resmgr answers the unassign 204 without necessarily doing it; believing that is what
+	// leaves a binding behind for a later host-config delete to make permanent.
+	t.Run("fails when resmgr never applies it", func(t *testing.T) {
+		err := waitUnassigned(context.Background(), hostList(t, twoHosts, ""), "host-a", "hc-1")
+		if err == nil {
+			t.Fatal("got no error from a binding that never cleared")
+		}
+		if !strings.Contains(err.Error(), "host-a") || !strings.Contains(err.Error(), "hc-1") {
+			t.Fatalf("error does not say what is still bound: %v", err)
+		}
+	})
+}

internal/services/resmgr/host_config_assignment_resource.go

@@ -134,12 +134,17 @@ func (r *hostConfigAssignmentResource) Delete(ctx context.Context, req resource.
 		return
 	}
 
-	url := client.ServiceURL("hosts", state.HostID.ValueString(), "hostconfig", state.HostConfigID.ValueString())
-	if _, err := client.Delete(ctx, url, &gophercloud.RequestOpts{OkCodes: []int{200, 202, 204}}); err != nil {
-		if isNotFound(err) {
-			return
-		}
+	hostID, hostConfigID := state.HostID.ValueString(), state.HostConfigID.ValueString()
+	url := client.ServiceURL("hosts", hostID, "hostconfig", hostConfigID)
+	if _, err := client.Delete(ctx, url, &gophercloud.RequestOpts{OkCodes: []int{200, 202, 204}}); err != nil && !isNotFound(err) {
 		resp.Diagnostics.AddError("resmgr: unassigning host config", err.Error())
+		return
+	}
+	// A 204 here does not mean the binding is gone — resmgr answers the same whether or
+	// not it unbound anything — so confirm against the host list before reporting the
+	// assignment destroyed.
+	if err := waitUnassigned(ctx, client, hostID, hostConfigID); err != nil {
+		resp.Diagnostics.AddError("resmgr: host config still assigned", err.Error())
 	}
 }
 

internal/services/resmgr/host_config_resource.go

@@ -5,6 +5,8 @@ package resmgr
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	"github.com/gophercloud/gophercloud/v2"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
@@ -72,7 +74,7 @@ func (r *hostConfigResource) Schema(_ context.Context, _ resource.SchemaRequest,
 		return schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: desc, PlanModifiers: useState}
 	}
 	resp.Schema = schema.Schema{
-		MarkdownDescription: "Manages a PCD host configuration: the mapping of traffic types (management, VM console, " +
+		MarkdownDescription: "Manages a PCD host configuration: the mapping of traffic types (management, VM console,  Destroying one is refused while any host is still assigned to it: PCD leaves such a host unable to be assigned a host configuration ever again, so remove the `pcd_host_config_assignment` first." +
 			"tunnels, image library, live migration, host liveness) to network interfaces, plus physical-network labels.",
 		Attributes: map[string]schema.Attribute{
 			"id":                       schema.StringAttribute{Computed: true, MarkdownDescription: "The host configuration ID.", PlanModifiers: useState},
@@ -204,7 +206,46 @@ func (r *hostConfigResource) Delete(ctx context.Context, req resource.DeleteRequ
 		return
 	}
 
-	if _, err := client.Delete(ctx, client.ServiceURL("hostconfigs", state.ID.ValueString()), &gophercloud.RequestOpts{OkCodes: []int{200, 202, 204}}); err != nil {
+	// Deleting a host configuration a host is still assigned to cannot be undone, so it is
+	// checked rather than attempted. resmgr accepts the delete, keeps the assignment, and
+	// from then on refuses to remove it (404 — it validates that the target host config
+	// exists before unbinding) and refuses every re-assignment (409
+	// HostToHostconfigConflict). Deleting the host record does not clear it either, and a
+	// host configuration cannot be re-created under the id that was removed: the host can
+	// never be assigned one again, so it can never be onboarded again.
+	//
+	// The guard is deliberately fail-closed. If the check itself cannot be completed the
+	// delete does not proceed — an unverified assumption here costs a hypervisor.
+	id := state.ID.ValueString()
+	assigned, err := hostsAssignedTo(ctx, client, id)
+	if err != nil {
+		resp.Diagnostics.AddError(
+			"resmgr: cannot confirm the host configuration is unused",
+			fmt.Sprintf("Host configuration %s was not deleted because the check for hosts still "+
+				"assigned to it could not be completed: %s\n\nDeleting one while a host is still "+
+				"assigned to it strands that host permanently, so Terraform will not do it on an "+
+				"unverified answer. Retry when resmgr is reachable.", id, err),
+		)
+		return
+	}
+	if len(assigned) > 0 {
+		resp.Diagnostics.AddError(
+			"Host configuration is still assigned to a host",
+			fmt.Sprintf("Host configuration %s is still assigned to %s, and deleting it now would "+
+				"leave that host unable to be assigned any host configuration ever again: resmgr "+
+				"would refuse both the unassign (404) and every re-assignment (409), with no way "+
+				"back.\n\nRemove the assignment first, then delete the host configuration:\n"+
+				"  - destroy the pcd_host_config_assignment resource that binds them, or\n"+
+				"  - DELETE /resmgr/v2/hosts/<host_id>/hostconfig/%s if the assignment was made "+
+				"outside Terraform, then confirm hostconfig_id has cleared in GET /resmgr/v2/hosts.\n\n"+
+				"This guard is here because of a defect in PCD, not in your configuration, and will "+
+				"be lifted once resmgr refuses the unsafe delete itself.",
+				id, strings.Join(assigned, ", "), id),
+		)
+		return
+	}
+
+	if _, err := client.Delete(ctx, client.ServiceURL("hostconfigs", id), &gophercloud.RequestOpts{OkCodes: []int{200, 202, 204}}); err != nil {
 		if isNotFound(err) {
 			return
 		}

internal/services/resmgr/resmgr.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"time"
 
 	"github.com/gophercloud/gophercloud/v2"
 	"github.com/hashicorp/terraform-plugin-framework/diag"
@@ -84,6 +85,86 @@ func getJSON(ctx context.Context, client *gophercloud.ServiceClient, url string,
 	return json.Unmarshal(raw, out)
 }
 
+// getJSONList issues an authenticated GET for a collection. Unlike getJSON, a null or
+// empty body is an empty collection rather than an absence: the distinction getJSON
+// draws only makes sense for a request naming one object.
+func getJSONList(ctx context.Context, client *gophercloud.ServiceClient, url string, out any) error {
+	var raw json.RawMessage
+	if _, err := client.Get(ctx, url, &raw, &gophercloud.RequestOpts{OkCodes: []int{200}}); err != nil {
+		if errors.Is(err, io.EOF) {
+			return nil
+		}
+		return err
+	}
+	if isNullJSON(raw) {
+		return nil
+	}
+	return json.Unmarshal(raw, out)
+}
+
+// hostsAssignedTo returns the ids of the hosts resmgr still reports as carrying the
+// given host configuration. The host *list* is the authority: the per-host endpoints
+// answer 404 for minutes after a host is deauthorised, while the list keeps reporting
+// the host and its hostconfig_id throughout.
+func hostsAssignedTo(ctx context.Context, client *gophercloud.ServiceClient, hostConfigID string) ([]string, error) {
+	var hosts []hostAPI
+	if err := getJSONList(ctx, client, client.ServiceURL("hosts"), &hosts); err != nil {
+		return nil, err
+	}
+	var assigned []string
+	for _, h := range hosts {
+		if h.HostConfigID == hostConfigID {
+			assigned = append(assigned, h.ID)
+		}
+	}
+	return assigned, nil
+}
+
+// unassignPollInterval / unassignPollTimeout bound the wait for an unassign to show up
+// in the host list. Short: this confirms a write resmgr has already accepted.
+// var, not const, so a test can drive the clock instead of sleeping through it.
+var (
+	unassignPollInterval = 5 * time.Second
+	unassignPollTimeout  = 60 * time.Second
+)
+
+// waitUnassigned blocks until the host stops reporting the host configuration.
+//
+// resmgr answers the unassign 204 whether or not it unbound anything, and 404 while a
+// freshly deauthorised host is not describable, so the status code proves nothing. A
+// binding Terraform believes is gone while resmgr still holds it is what later strands
+// the host: the next apply cannot re-create the assignment (409) and deleting the host
+// configuration in that state makes it permanent.
+func waitUnassigned(ctx context.Context, client *gophercloud.ServiceClient, hostID, hostConfigID string) error {
+	deadline := time.Now().Add(unassignPollTimeout)
+	for {
+		assigned, err := hostsAssignedTo(ctx, client, hostConfigID)
+		if err != nil {
+			return fmt.Errorf("checking whether host %s still carries host config %s: %w", hostID, hostConfigID, err)
+		}
+		still := false
+		for _, h := range assigned {
+			if h == hostID {
+				still = true
+				break
+			}
+		}
+		if !still {
+			return nil
+		}
+		if time.Now().After(deadline) {
+			return fmt.Errorf("host %s still reports host config %s %s after the unassign was accepted; "+
+				"resmgr did not apply it, and deleting the host config while this stands would leave the "+
+				"host unable to be assigned one again", hostID, hostConfigID, unassignPollTimeout)
+		}
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-time.After(unassignPollInterval):
+		}
+	}
+}
+
 // postJSON issues an authenticated POST with a JSON body, optionally decoding
 // the response into out (pass nil to ignore the body).
 func postJSON(ctx context.Context, client *gophercloud.ServiceClient, url string, body, out any) error {