test: add project display_text compatibility and import coverage - #335
test: add project display_text compatibility and import coverage#335dheeraj12347 wants to merge 4 commits into
Conversation
CloudStack's updateStorageNetworkIpRange API validates a new IP range against the record's own current start/end IPs without excluding the record being updated, so any in-place edit of these fields fails with a self-overlap error (errorcode 530). Switching the update call to only send changed fields (d.HasChange instead of d.GetOk) did not help — the failure is identical even when only the genuinely-changed field is sent, confirming this is a server-side validation bug rather than something the provider can work around. Mark these fields ForceNew so the plan matches reality: Terraform now proposes a replacement instead of an update that is guaranteed to fail. Reproduced and reverified against ACS 4.23.0.0.
Investigated the reported destroy+recreate-on-import issue (apache#305): the importer's resulting state is actually fully hydrated correctly, because Terraform automatically calls Read (visible as "Refreshing state...") right after the custom importer function runs, which overwrites whatever partial state the importer itself set. So populating cpu_number/cpu_speed/ memory inside resourceCloudStackServiceOfferingImport would be a no-op — verified by importing a real lab offering with the unmodified importer and confirming cpu_number/cpu_speed/memory were already correct in state. The actual destroy+recreate happens because Terraform diffs the *config* (which a minimal post-import .tf typically leaves blank for these fields) against the now-correct state; since these fields are ForceNew, the config's implicit zero value differs from the real value and forces replacement. This is a doc/workflow gap, not a code bug: documented that all ForceNew fields must be fully specified in config after import. Verified empty terraform plan after import with a fully-specified config, and unchanged (still-forcing) plan with a minimal one, matching this explanation.
…splaytext Every other resource in this provider uses display_text; cloudstack_project was the outlier still on displaytext, even though its own docs already described display_text as the field name. Add display_text additively (displaytext is a real field in existing users' state files and can't be renamed outright) and mark displaytext Deprecated. Create/Update/Read resolve the effective value via projectDisplayText(), preferring display_text when both are set. Read only refreshes whichever of the two fields is actually in use (config already had displaytext set and display_text unset), matching the existing conditional pattern this file already uses for account/accountid/userid. Setting both unconditionally caused a permanent diff for display_text-only configs, since Read would keep populating the deprecated field the config never referenced. Verified against the lab with two standalone configs (one using display_text, one using the legacy displaytext) against a locally-built dev-override binary: both create cleanly, both produce an empty terraform plan, and the legacy field shows the expected deprecation warning. Both test projects destroyed after verification.
|
Hi @sudo87 and @DaanHoogland , I’ve added a follow-up contribution in PR #335 for the cloudstack_project display_text migration. I focused on validating the compatibility behavior introduced by the new display_text field: Added acceptance coverage for configurations using display_text. The follow-up changes are in commit 37ebc48 on the PR. I’d appreciate your review and any feedback on whether you’d prefer the import compatibility handling or test structure to be adjusted further. |
2 participants
Summary
Add acceptance coverage for the
cloudstack_projectdisplay_textmigration and fix an import-state compatibility issue discovered during testing.The existing resource supports the new snake_case
display_textfield while retaining the deprecateddisplaytextfield for backwards compatibility. These changes exercise both paths against the CloudStack simulator and verify that existing project behavior remains intact.Changes
display_text.display_textand deprecateddisplaytextare configured.display_textvalue.Read()state handling for cases where neither display-text field is present in configuration, such as resource import.displaytextstate representation in that case to avoid an import-state mismatch.Validation
The complete
cloudstack_projectacceptance test suite passes:TestAccCloudStackProject_basicTestAccCloudStackProject_displayTextTestAccCloudStackProject_displayTextPrecedenceTestAccCloudStackProject_updateTestAccCloudStackProject_importTestAccCloudStackProject_accountTestAccCloudStackProject_updateAccountTestAccCloudStackProject_emptyDisplayTextTestAccCloudStackProject_updateUseridTestAccCloudStackProject_updateAccountidTestAccCloudStackProject_listResult: 11/11 passing
Also verified with:
Both pass successfully.
Regression found during validation
The initial full acceptance run exposed an import-state mismatch where the imported resource contained
displaytextwhile the provider read populateddisplay_text.The
Read()logic was adjusted to distinguish between:displaytextconfigurations,display_textconfigurations, andThe import acceptance test now passes without affecting the existing project acceptance tests.