Seed number_of_retries and retry_interval when importing aviatrix_edge_spoke_external_device_conn - #2484
Open
eastagiletracker wants to merge 1 commit into
Conversation
…e_conn number_of_retries and retry_interval drive the provider-side create retry loop and are not stored by the Controller, so importing a connection left them out of the state while the configuration kept applying the schema defaults. Every plan after the import then reported + number_of_retries = 0 + retry_interval = 300 which no apply could settle. The importer now seeds both attributes with their schema defaults, and the defaults are shared constants so the two cannot drift apart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes seeding
number_of_retriesandretry_intervalwhen importing anaviatrix_edge_spoke_external_device_conn, so an imported connection stops reporting a change that no apply can settle (Fixes #2482). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/293. You can sign in with your GitHub ID to claim ownership of the project.What was wrong
number_of_retriesandretry_intervalare provider-side knobs: they only drive the create retry loop inresourceAviatrixEdgeSpokeExternalDeviceConnCreateandresourceAviatrixEdgeSpokeExternalDeviceConnEnableHa, and the Controller never stores them. The resource imported throughschema.ImportStatePassthroughContext, and the Read function has nothing to read them back from, so both attributes stayed absent from the imported state while the configuration kept resolving them to their schema defaults. Every plan after the import then reported+ number_of_retries = 0and+ retry_interval = 300regardless of what the configuration said, leavinglifecycle { ignore_changes = [...] }as the only workaround.The change
Importer.StateContextnow points atresourceAviatrixEdgeSpokeExternalDeviceConnImport, which seeds both attributes with their schema defaults and then delegates toschema.ImportStatePassthroughContext(so the "missing import ID" check is unchanged). The two defaults moved into shared constants that the schema and the importer both use, so they cannot drift apart. Nothing on the create, read, update or delete paths is touched: the seeding runs only during an import, where the state starts empty, so a connection that is already in state keeps whatever value it was applied with. A configuration that sets an explicit non-default value still wins on the first plan after the import, and settles from there.I also added a row to the 10.1.0 Bug Fixes table in
docs/guides/release-note.md, per the release-notes item in the contribution checklist — happy to restate it against an internal AVX id if you prefer.Reproducing and verifying
Both new tests exercise the real
Importer.StateContextof the registered resource, with no Controller involved. The first asserts the imported state carries the two attributes; the second plans that imported state against a configuration that omits them and asserts nothing changes — that is the reported symptom, at plan level.On
masterat1165aa5fthe second test reports exactly the diff from the issue:With the change applied both pass, and the full suite is unchanged against the pre-change baseline —
go test ./...was green before (ok aviatrix,ok goaviatrix) and is green after, with no new failures.go vet ./aviatrix/is clean andgofumpt -lflags none of the touched files. The acceptance test needs a live Controller, so I could not run it here; the fix is what makes the imported state match the applied state for these two attributes, which is what the existingImportState/ImportStateVerifystep inTestAccAviatrixEdgeSpokeExternalDeviceConn_basiccompares.How this was managed
We tracked this work on a story on a live board built from this repository's own issues and pull requests — 2477 stories and 18 labels imported: the story for this fix is https://eastagiletracker.com/projects/293/stories/184161 and the board it lives on is https://eastagiletracker.com/projects/293.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com