-
Notifications
You must be signed in to change notification settings - Fork 213
fix: Preserve mongo_db_major_version state after out-of-band major version upgrade #4400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
32ca815
93cbb4d
5d1bfd1
0b83ff7
e3afc28
dab5e4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ```release-note:bug | ||
| resource/mongodbatlas_advanced_cluster: Enables drift detection in `mongo_db_major_version` after performing an upgrade outside of Terraform | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package advancedcluster_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedcluster" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestAdvancedCluster_ShouldUsePreviousMongoDBMajorVersion(t *testing.T) { | ||
| testCases := []struct { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. internal/service/advancedcluster/resource_compatibility_test.go:11 — The table covers major-change vs same-major outcomes, but it doesn’t exercise the Severity: low 🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage. |
||
| name string | ||
| before string | ||
| after string | ||
| expected bool | ||
| }{ | ||
| { | ||
| name: "keeps previous version when only formatting differs", | ||
| before: "8", | ||
| after: "8.0", | ||
| expected: true, | ||
| }, | ||
| { | ||
| name: "uses API value when major version actually changed", | ||
| before: "7.0", | ||
| after: "8.0", | ||
| expected: false, | ||
| }, | ||
| { | ||
| name: "keeps previous version when minor version differs within same major", | ||
| before: "8.0", | ||
| after: "8.2", | ||
| expected: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| assert.Equal(t, tc.expected, advancedcluster.ShouldUsePreviousMongoDBMajorVersion(tc.before, tc.after)) | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "go.mongodb.org/atlas-sdk/v20250312018/admin" | ||
|
|
||
|
|
@@ -94,7 +95,9 @@ func overrideAttributesWithPrevStateValue(modelIn, modelOut *TFModel) { | |
| return | ||
| } | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. internal/service/advancedcluster/resource_compatiblity.go:99 — This comment says the override is only for formatting differences, but Severity: low 🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage. |
||
| if beforeVersion != nil && afterVersion != nil && ShouldUsePreviousMongoDBMajorVersion(*beforeVersion, *afterVersion) { | ||
| modelOut.MongoDBMajorVersion = types.StringPointerValue(beforeVersion) | ||
| } | ||
| overrideMapStringWithPrevStateValue(&modelIn.Labels, &modelOut.Labels) | ||
|
|
@@ -109,6 +112,18 @@ func overrideAttributesWithPrevStateValue(modelIn, modelOut *TFModel) { | |
| modelOut.UseEffectiveFields = modelIn.UseEffectiveFields | ||
| } | ||
|
|
||
| func ShouldUsePreviousMongoDBMajorVersion(beforeVersion, afterVersion string) bool { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function helps preserve the prior state value when the major component is the same, which handles two cases:
The reason why step 2 is implemented, is to avoid unit test failure |
||
| if beforeVersion == afterVersion { | ||
| return false | ||
| } | ||
| return majorComponent(beforeVersion) == majorComponent(afterVersion) | ||
| } | ||
|
|
||
| func majorComponent(version string) string { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Users that have an outdate
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: Does version_release_system = "CONTINUOUS" only upgrade minor versions?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both, per my understanding of the docs:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| major, _, _ := strings.Cut(FormatMongoDBMajorVersion(version), ".") | ||
| return major | ||
| } | ||
|
|
||
| func overrideMapStringWithPrevStateValue(mapIn, mapOut *types.Map) { | ||
| if mapIn == nil || mapOut == nil || len(mapOut.Elements()) > 0 { | ||
| return | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.