Skip to content

fix(resmgr): an imported pcd_host_cluster_role plans clean - #50

Merged
PF9-pushkar merged 10 commits into
mainfrom
pushkar/host-cluster-role-import
Sep 4, 2026
Merged

fix(resmgr): an imported pcd_host_cluster_role plans clean#50
PF9-pushkar merged 10 commits into
mainfrom
pushkar/host-cluster-role-import

Conversation

@PF9-pushkar

Copy link
Copy Markdown
Collaborator

Importing a pcd_host_cluster_role always left the next plan with one change:

  ~ resource "pcd_host_cluster_role" "hyp1" {
      + wait_until_converged = false
    }

and applying it re-PUT the role assignment to resmgr for a change resmgr never sees. Found while importing the CE lab's roles for the PCD-9783 destroy test (the imports had to be done with the CLI to avoid the PUTs).

Root cause

wait_until_converged is Optional + Computed + Default(false). ImportState set only id, host_id and role, and Read never touches the flag, so the imported state held null; the schema default then planned null -> false. Update PUT unconditionally, so applying that diff was a real write. lifecycle { ignore_changes } cannot suppress a provider-side default.

Change

  • ImportState writes wait_until_converged = false, so the imported state already equals what the default will plan.
  • Update compares the two server-side options, host_cluster and backends, between plan and prior state through a pure helper roleOptionsChanged (null and "" host cluster are the same omitted value; backends compared as ordered lists with null/unknown as "not sent" and [] as a value, mirroring what the PUT body carries). When nothing server-side changed it stores the plan and returns: no PUT, no convergence wait. A real change takes the untouched path.
  • Schema unchanged, so existing states (which already hold false) see no upgrade diff. host_cluster and backends are still not read back; a configuration that sets them on an imported role plans the update it always did.
  • Behavior change called out in the changelog: flipping the flag false -> true on an already-assigned role no longer re-PUTs and waits; taint or -replace to block on an existing role.

Design spec and implementation plan are committed under docs/superpowers/.

Verification

  • Unit (no lab): table test for roleOptionsChanged; Update no-op test through the framework request types with a nil client, so any regression that reaches resmgr fails; ImportState tests through a schema-built tfsdk.State (the same null-root construction the framework uses).
  • Acceptance (opt-in): TestAccResmgrHostClusterRoleImport, gated by PCD_ACC_RESMGR=1 and PCD_ACC_HOST_ID, creates the role, imports it with ImportStateVerify (which fails on the pre-fix provider because the imported state lacked the flag), and polls the host after destroy until the role clears. Ran green against the CE lab (2026.4, host 136fc11a, role image-library) in 65s.
  • gofmt, go vet, golangci-lint (0 issues), go test ./internal/... green.
  • Reviewed per task and as a whole branch; one final-review round fixed the changelog wording and added the acceptance test, one live-run round dropped an ImportStatePersist that made Terraform refuse the import ("Resource already managed").

Follow-up, not in this PR: Update carries its own copy of assignBody's body-building because assignBody takes a *resource.CreateResponse; changing it to take *diag.Diagnostics would let both paths share one function.

- add TestAccResmgrHostClusterRoleImport (resmgr_test.go), gated on
  PCD_ACC_RESMGR and PCD_ACC_HOST_ID, with a polling
  testAccCheckHostClusterRoleDestroy since deauthorising a role is
  asynchronous
- note in roleOptionsChanged's doc comment that the comparison is
  deliberately role-agnostic: ForceNew role plus ValidateConfig's
  option-to-role pairing means a mismatch can only push toward sending
  the PUT, never toward silently skipping one
- TestUpdateSkipsResmgrForClientSideChanges now also asserts host_cluster
  survived the short-circuit, and documents why the response state is
  seeded from prior rather than a null root
- CHANGELOG: the Unreleased fixed-entry now says a false -> true flip on
  an already-assigned role no longer waits for convergence either
  (previously it PUT and then waited), and names taint / apply -replace
  as the way to force a wait
- plan: record in Self-Review that the acceptance test was added at
  final review by user decision, so the plan and the branch agree
@PF9-pushkar
PF9-pushkar merged commit 56e233f into main Sep 4, 2026
5 checks passed
@PF9-pushkar
PF9-pushkar deleted the pushkar/host-cluster-role-import branch September 4, 2026 23:01
@PF9-pushkar PF9-pushkar mentioned this pull request Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant