fix: Preserve mongo_db_major_version state after out-of-band major version upgrade#4400
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts advanced cluster state reconciliation so MongoDB major version values are only preserved from prior state when the difference is purely formatting (e.g., "8" vs "8.0"), while allowing real major-version transitions reported by the Atlas API to be reflected in Terraform state.
Changes:
- Updates
overrideAttributesWithPrevStateValueto preservemongo_db_major_versiononly for formatting-only diffs. - Introduces
ShouldUsePreviousMongoDBMajorVersionhelper encapsulating the formatting comparison logic. - Adds a unit test covering the helper’s expected behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/service/advancedcluster/resource_compatiblity.go |
Refines state override logic for mongo_db_major_version and adds a helper for formatting-only comparisons. |
internal/service/advancedcluster/resource_compatiblity_test.go |
Adds unit test coverage for the new helper behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| modelOut.UseEffectiveFields = modelIn.UseEffectiveFields | ||
| } | ||
|
|
||
| func ShouldUsePreviousMongoDBMajorVersion(beforeVersion, afterVersion string) bool { |
There was a problem hiding this comment.
This function helps preserve the prior state value when the major component is the same, which handles two cases:
- Formatting differences: user sets
"8"but Atlas returns"8.0"which is the same version, but different format. This is to preserve SDKv2 behaviour version_release_system = "CONTINUOUS": Atlas can return a minor bump (e.g. "8.2") that is not user-controlled. Letting it through causes:provider produced an unexpected new value: .mongo_db_major_version: was cty.StringVal("8.0"), but now cty.StringVal("8.2").`
The reason why step 2 is implemented, is to avoid unit test failure
|
APIx bot: a message has been sent to Docs Slack channel |
🤖 Augment PR SummarySummary: Fixes perpetual plan drift for Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| beforeVersion := conversion.NilForUnknown(modelIn.MongoDBMajorVersion, modelIn.MongoDBMajorVersion.ValueStringPointer()) | ||
| if beforeVersion != nil && !modelIn.MongoDBMajorVersion.Equal(modelOut.MongoDBMajorVersion) { | ||
| afterVersion := conversion.NilForUnknown(modelOut.MongoDBMajorVersion, modelOut.MongoDBMajorVersion.ValueStringPointer()) | ||
| // Only preserve prior state value for formatting differences (e.g. "8" vs "8.0"), not real version transitions. |
There was a problem hiding this comment.
internal/service/advancedcluster/resource_compatiblity.go:99 — This comment says the override is only for formatting differences, but ShouldUsePreviousMongoDBMajorVersion also preserves the prior value when the minor version changes within the same major (e.g., CONTINUOUS returning 8.2). Consider clarifying the comment so future readers don’t assume this logic is formatting-only.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ) | ||
|
|
||
| func TestAdvancedCluster_ShouldUsePreviousMongoDBMajorVersion(t *testing.T) { | ||
| testCases := []struct { |
There was a problem hiding this comment.
internal/service/advancedcluster/resource_compatibility_test.go:11 — The table covers major-change vs same-major outcomes, but it doesn’t exercise the beforeVersion == afterVersion early-return branch in ShouldUsePreviousMongoDBMajorVersion. Consider adding an explicit equality case to lock in that behavior.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
EspenAlbert
left a comment
There was a problem hiding this comment.
Nice! Really good PR description 👏
AgustinBettati
left a comment
There was a problem hiding this comment.
Changes LGTM, would just confirm with Alex and team on potential impact
| return majorComponent(beforeVersion) == majorComponent(afterVersion) | ||
| } | ||
|
|
||
| func majorComponent(version string) string { |
There was a problem hiding this comment.
Users that have an outdate mongo_db_major_version value would now receive a non-empty plan (cases where major version upgrade was done outside of TF). Would confirm with Alex we are okay with this impact.
| return majorComponent(beforeVersion) == majorComponent(afterVersion) | ||
| } | ||
|
|
||
| func majorComponent(version string) string { |
There was a problem hiding this comment.
q: Does version_release_system = "CONTINUOUS" only upgrade minor versions?
There was a problem hiding this comment.
Both, per my understanding of the docs:
Latest Version With Auto Upgrades: If you set your cluster to this release option, it receives automatic upgrades to the latest available MongoDB major or minor version
There was a problem hiding this comment.
Okay, would make sure to test if an error is returned if version_release_system = "CONTINUOUS" and mongo_db_major_version attribute are used together (as per our docs). Using this combination would lead to non-empty plan once automatic major version upgrades are made.
Description
Fixes a bug where
mongodbatlas_advanced_clusterwould retain a stalemongo_db_major_versionin Terraform state after an out-of-band major version upgrade (e.g. upgrading from MongoDB 7.0 to 8.0 via Atlas UI and unpinning FCV). Even after updating the config to match the new version, the provider would perpetually show a diff.The root cause was in
overrideAttributesWithPrevStateValue: it unconditionally overwrote the API value with the prior state value whenever they differed, which was intended to suppress cosmetic formatting differences (e.g."8"vs"8.0") but also masked real version transitions.The fix narrows the override to cases where the major component is the same, handling two scenarios: formatting differences (e.g. "8" vs "8.0") and minor version bumps from
version_release_system = "CONTINUOUS"(e.g. "8.0" vs "8.2") where the version is no longer user-controlled.Link to any related issue(s): CLOUDP-399538
Closes #4398
Type of change:
Required Checklist:
Further comments
Related external PR: #4399 (opened by the issue reporter with the same fix — this PR adapts it to follow our internal conventions).