chore: Adds import checks to tests #3039

EspenAlbert · 2025-02-06T21:26:52Z

Description

Adds import checks to tests

Link to any related issue(s): CLOUDP-298071

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

…luster

EspenAlbert · 2025-02-06T21:28:21Z

internal/testutil/acc/advanced_cluster.go

@@ -53,6 +53,7 @@ var (
 func TestStepImportCluster(resourceName string, ignorePrefixFields ...string) resource.TestStep {
 	ignorePrefixFields = append(ignorePrefixFields,
 		"retain_backups_enabled", // This field is TF specific and not returned by Atlas, so Import can't fill it in.
+		"mongo_db_major_version", // Risks plan change of 8 --> 8.0 (always normalized to `major.minor`)


Could potentially use a custom type to avoid plan changes, but seems overkill.
Both the apply for retain_backups_enabled and mongo_db_major_version are instant

although it's instant, I think it can be confusing for the user to see these plan changes, and think for example that cluster version will change and could lose data / affect cluster config.

we might do something like in Move/Upgrade setOptionalModelAttrs :

if mongoDBMajorVersion := getAttrFromStateObj[string](stateObj, "mongo_db_major_version"); mongoDBMajorVersion != nil { model.MongoDBMajorVersion = types.StringPointerValue(mongoDBMajorVersion) }

in Import we don't have previous state but we can get from Atlas.
Or if we shouldn't call Atlas from Import, then we can set a dummy value in mongo_db_major_version so it's not null and the good value will be set in Read.

Interesting approach. But I don't see how this will solve the problem.
The problem is the support of two different formats in the config:

8

8.0
We cannot know at import time which format they used, so we use what is returned by the API (8.0)

ah, ok, the problem is only when the user specifies mongo_db_major_version in the config? there are no plan changes if mongo_db_major_version is not in the config?

in that case ok to leave it as it is.

Exactly 👍

I'm good then, thanks!

EspenAlbert · 2025-02-06T21:28:53Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

@@ -965,6 +984,7 @@ func TestAccAdvancedCluster_replicaSetScalingStrategyAndRedactClientLogDataOldSc
 				Config: configReplicaSetScalingStrategyAndRedactClientLogDataOldSchema(t, true, orgID, projectName, clusterName, "NODE_TYPE", false),
 				Check:  checkReplicaSetScalingStrategyAndRedactClientLogData(true, "NODE_TYPE", false),
 			},
+			acc.TestStepImportCluster(resourceName, "replication_specs"), // Import with old schema will NOT use `num_shards`


Example of explicit replication_specs ignore when the old sharding configuration is used

nice to add an explanation

EspenAlbert · 2025-02-06T21:29:30Z

internal/service/advancedclustertpf/move_upgrade_state.go

@@ -151,7 +151,7 @@ func getTimeoutFromStateObj(stateObj map[string]tftypes.Value) timeouts.Value {
 	return timeouts.Value{Object: obj}
 }

-func setOptionalModelAttrs(ctx context.Context, stateObj map[string]tftypes.Value, model *TFModel) {
+func setOptionalModelAttrs(stateObj map[string]tftypes.Value, model *TFModel) {


opportunistic removal of unused param

EspenAlbert · 2025-02-06T21:32:48Z

Ran all tests for TPF locally (a few failures due to replication_specs and mongo_db_major_version, fix in 12ef64d

Running for SDKv2 here. A few failures.
Fixes in 17835c9 and aafc86a

…event flaky tests

…aky tests

lantoli · 2025-02-07T07:28:57Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

@@ -166,6 +166,7 @@ func TestAccMockableAdvancedCluster_tenantUpgrade(t *testing.T) {
 				Config: acc.ConvertAdvancedClusterToSchemaV2(t, true, configTenantUpgraded(projectID, clusterName, defaultZoneName)),
 				Check:  checksTenantUpgraded(projectID, clusterName),
 			},
+			acc.TestStepImportCluster(resourceName),


nice to add more test checks/steps

* master: doc: Renames env var to `MONGODB_ATLAS_PREVIEW_PROVIDER_V2_ADVANCED_CLUSTER` (#3062) chore: Updates Atlas Go SDK (#3053) test: Tenant clusters with disk_size_gb is always 5 now (#3057) chore: Updates CHANGELOG.md for #3051 fix: Removes `interval_min` plan modifier to avoid error on update (#3051) build(deps): bump go.mongodb.org/atlas-sdk (#3049) doc: Adds pinned_fcv.version as exported attribute in cluster and advanced cluster resource (#3047) update Go to 1.23.6 (#3045) chore: Adds HTTP status check helper methods (#3035) chore: Adds import checks to tests (#3039) chore: Updates CHANGELOG.md header for v1.26.1 release chore: Updates examples link in index.md for v1.26.1 release fix: Adds "state_name" to keepUnknown list to prevent copying volatile attributes from state (#3040)

EspenAlbert added 5 commits February 6, 2025 08:59

chore: removed unused ctx

8c29578

test: Add import checks to all test cases

c5149ec

ignore replication_specs in import verification

12ef64d

test: Add mongo_db_major_version to ignored fields in TestStepImportC…

d85afeb

…luster

chore: Updates mock files to include import step

e1bb3b6

EspenAlbert commented Feb 6, 2025

View reviewed changes

EspenAlbert added 2 commits February 7, 2025 07:05

test: Add state_name to ignored fields in TestStepImportCluster to pr…

aafc86a

…event flaky tests

test: Add id to ignored fields in TestStepImportCluster to prevent fl…

17835c9

…aky tests

EspenAlbert marked this pull request as ready for review February 7, 2025 07:14

EspenAlbert requested a review from a team as a code owner February 7, 2025 07:14

lantoli reviewed Feb 7, 2025

View reviewed changes

lantoli approved these changes Feb 7, 2025

View reviewed changes

EspenAlbert merged commit cfbf75b into master Feb 7, 2025
44 checks passed

EspenAlbert deleted the CLOUDP-298071_check_import branch February 7, 2025 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Adds import checks to tests #3039

chore: Adds import checks to tests #3039

EspenAlbert commented Feb 6, 2025

EspenAlbert Feb 6, 2025 •

edited

Loading

lantoli Feb 7, 2025 •

edited

Loading

EspenAlbert Feb 7, 2025

lantoli Feb 7, 2025 •

edited

Loading

EspenAlbert Feb 7, 2025

lantoli Feb 7, 2025

EspenAlbert Feb 6, 2025

lantoli Feb 7, 2025

EspenAlbert Feb 6, 2025

EspenAlbert commented Feb 6, 2025 •

edited

Loading

lantoli Feb 7, 2025

chore: Adds import checks to tests #3039

chore: Adds import checks to tests #3039

Conversation

EspenAlbert commented Feb 6, 2025

Description

Type of change:

Required Checklist:

Further comments

EspenAlbert Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

lantoli Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

EspenAlbert Feb 7, 2025

Choose a reason for hiding this comment

lantoli Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

EspenAlbert Feb 7, 2025

Choose a reason for hiding this comment

lantoli Feb 7, 2025

Choose a reason for hiding this comment

EspenAlbert Feb 6, 2025

Choose a reason for hiding this comment

lantoli Feb 7, 2025

Choose a reason for hiding this comment

EspenAlbert Feb 6, 2025

Choose a reason for hiding this comment

EspenAlbert commented Feb 6, 2025 • edited Loading

lantoli Feb 7, 2025

Choose a reason for hiding this comment

EspenAlbert Feb 6, 2025 •

edited

Loading

lantoli Feb 7, 2025 •

edited

Loading

lantoli Feb 7, 2025 •

edited

Loading

EspenAlbert commented Feb 6, 2025 •

edited

Loading