chore: Updates advanced_cluster migration tests to primarily use TPF #3555

maastha · 2025-08-05T19:49:02Z

Description

Updates advanced_cluster migration tests to primarily use TPF. This is a follow-up to #3547

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

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

maastha · 2025-08-06T01:09:08Z

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go

 package advancedcluster_test

 import (
-	"fmt"


internal/service/advancedcluster/resource_advanced_cluster_migration_test.go and internal/service/advancedcluster/resource_advanced_cluster_test.go will be moved to tpf package in follow-up PRs

…34497-disable_adv_clusterSDKv2-b-make-tpf-primary

This reverts commit be64674.

maastha · 2025-08-06T11:22:30Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

+				Config: configGeoShardedOldSchema(t, projectID, clusterName, 2, 2, false, isSDKv2),
 				Check: resource.ComposeAggregateTestCheckFunc(
-					checkGeoShardedOldSchema(usePreviewProvider, clusterName, 2, 2, true, false),
+					checkGeoShardedOldSchema(isTPF, clusterName, 2, 2, true, false),


since migration tests run for both SDK & TPF, I've not updated the check methods but have removed usages of acc.ConvertAdvancedClusterToPreviewProviderV2()

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go

EspenAlbert

What is the migration testing strategy for 1.x.x --> 2.x.x.
Will we test:

SDKv2 implementation --> v2 implementation
TPF implementation --> v2 implementation

I am assuming both?

AgustinBettati · 2025-08-06T13:28:37Z

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go

+			{
+				ExternalProviders: acc.ExternalProviders(versionBeforeTPFGARelease),
+				Config:            configShardedTransitionOldToNewSchema(t, false, projectID, clusterName, true, false),
+				Check:             checkShardedTransitionOldToNewSchema(false, true),
+			},


q: Where you able to confirm this intermediate step of removing num_shards is necessary before 2.x upgrade?

I ran TestMigAdvancedCluster_geoShardedMigrationFromOldToNewSchema without the second step & it did work. Should we update the guide or leave it as is?

I would be more inclined to adjusting the guide. Can we bring this up in the technical sync so we align and make sure we are not missing anything?

AgustinBettati · 2025-08-06T13:40:16Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

 }

-func configShardedNewSchema(t *testing.T, usePreviewProvider bool, orgID, projectName, name string, diskSizeGB int, firstInstanceSize, lastInstanceSize string, firstDiskIOPS, lastDiskIOPS *int, includeMiddleSpec, increaseDiskSizeShard2 bool) string {
+func configShardedNewSchema(t *testing.T, orgID, projectName, name string, diskSizeGB int, firstInstanceSize, lastInstanceSize string, firstDiskIOPS, lastDiskIOPS *int, includeMiddleSpec, increaseDiskSizeShard2 bool, useSDKv2 ...bool) string {


why do we pass useSDKv2 as a boolean list?

to keep it as an optional parameter, so not every caller has to use/specify it, only the migration functions

AgustinBettati · 2025-08-06T13:42:19Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

-			name = %[3]q
-			backup_enabled = false
-			cluster_type   = "SHARDED"
+	dataSourcesConfig := `


Thinking if for cases where we need both SDK v2 and TPF syntax configs (believe it would only be migration tests), we continue leveraging a conversion function similar to ConvertAdvancedClusterToPreviewProviderV2 to avoid duplicating configs. Maybe this is only for select number of tests and not needed.

yeah that's why for now I chose to duplicate the configs and handle migration tests separately. If we see a need for much such test cases arising then we can consider a conversion function but at this point I don't think that's required

marcosuma · 2025-08-06T12:17:16Z

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go

 const versionBeforeISSRelease = "1.17.6"

+// last version that supported mongodbatlas_advanced_cluster SDKv2 resource
+const versionBeforeTPFGARelease = "1.39.0"


based on the q-plan, we'll still have until 1.41.0

updated the workflow to use MONGODB_ATLAS_LAST_1X_VERSION env var instead of hardcoding, lmk WYT

…34497-disable_adv_clusterSDKv2-b-make-tpf-primary

EspenAlbert · 2025-08-08T11:46:53Z

.github/workflows/acceptance-tests-runner.yml

      - name: Acceptance Tests
        env:
          MONGODB_ATLAS_LAST_VERSION: ${{ needs.get-provider-version.outputs.provider_version }}
+          MONGODB_ATLAS_LAST_1X_VERSION: ${{ inputs.mongodb_atlas_last_1x_version }}


Would it make sense to create a advanced_cluster_tpf_mig_from_tpf? This way we will have a clean separation between acc/mig tests?

EspenAlbert · 2025-08-08T11:47:31Z

.github/workflows/code-health.yml

    uses: ./.github/workflows/acceptance-tests.yml
    with:
-        reduced_tests: true
+        reduced_tests: false


Is this for debugging? Shouldn't it be true by default?

EspenAlbert · 2025-08-08T11:51:32Z

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go


 	resource.ParallelTest(t, resource.TestCase{
-		PreCheck:     func() { mig.PreCheckBasic(t) },
+		PreCheck:     func() { mig.PreCheckBasic(t); mig.PreCheckLast1XVersion(t) },


I see this pre check only in 2 migration tests. Do we plan on using it in more?
I guess, all mig tests will use the latests version, but as soon as we release 2.0 we will only have two tests covering this.
An alternative would be to use a different test group for testing 1.x --> 2.latest and pass LAST_VERSION env-var explicilty using the MONGODB_ATLAS_LAST_1X_VERSION

EspenAlbert · 2025-08-08T11:55:04Z

internal/testutil/acc/advanced_cluster_preview_provider_v2.go

 }

-func TestCheckResourceAttrPreviewProviderV2(usePreviewProvider bool, name, key, value string) resource.TestCheckFunc {
+func TestCheckResourceAttrTPF(usePreviewProvider bool, name, key, value string) resource.TestCheckFunc {


I'm struggling a bit to understand this function now.
We are using TPF in the name, but arguments and calls uses previewProvider.
Any way to simplify this? Can we remove previewProvider everywhere?

init

fc7154a

maastha changed the base branch from master to CLOUDP-334497-disable_adv_clusterSDKv2-b August 5, 2025 19:49

maastha added 2 commits August 5, 2025 22:40

update mig configs

49fcdc3

fmt

6e780bf

maastha changed the title ~~chore: tmp~~ chore: Adds tmp Aug 5, 2025

maastha added 5 commits August 5, 2025 22:42

tmp revert

b61e65d

fmt

fca04c5

nit

3ec3f19

nit

0b0b206

fix

835cb4a

maastha changed the title ~~chore: Adds tmp~~ chore: Updates advanced_cluster migration tests to primarily use TPF Aug 6, 2025

maastha commented Aug 6, 2025

View reviewed changes

maastha added 5 commits August 6, 2025 03:18

git

ba2d8f1

Merge branch 'CLOUDP-334497-disable_adv_clusterSDKv2-b' into CLOUDP-3…

3292aee

…34497-disable_adv_clusterSDKv2-b-make-tpf-primary

lint

575bb3b

mig

be64674

Revert "mig"

09c56c6

This reverts commit be64674.

maastha commented Aug 6, 2025

View reviewed changes

maastha marked this pull request as ready for review August 6, 2025 11:25

maastha requested a review from a team as a code owner August 6, 2025 11:25

EspenAlbert reviewed Aug 6, 2025

View reviewed changes

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go Show resolved Hide resolved

EspenAlbert reviewed Aug 6, 2025

View reviewed changes

AgustinBettati reviewed Aug 6, 2025

View reviewed changes

marcosuma reviewed Aug 6, 2025

View reviewed changes

maastha added 5 commits August 6, 2025 19:41

Merge branch 'CLOUDP-334497-disable_adv_clusterSDKv2-b' into CLOUDP-3…

c18821a

…34497-disable_adv_clusterSDKv2-b-make-tpf-primary

add env var

c4b93be

nit

6a32db0

fmt

489a96b

np

e1ee3ab

EspenAlbert approved these changes Aug 7, 2025

View reviewed changes

marcosuma approved these changes Aug 7, 2025

View reviewed changes

Merge branch 'CLOUDP-334497-disable_adv_clusterSDKv2-b' into CLOUDP-3…

9b8d7c9

…34497-disable_adv_clusterSDKv2-b-make-tpf-primary

EspenAlbert reviewed Aug 8, 2025

View reviewed changes

maastha merged commit df5313f into CLOUDP-334497-disable_adv_clusterSDKv2-b Aug 8, 2025
72 of 74 checks passed

maastha deleted the CLOUDP-334497-disable_adv_clusterSDKv2-b-make-tpf-primary branch August 8, 2025 20:38

chore: Updates advanced_cluster migration tests to primarily use TPF #3555

chore: Updates advanced_cluster migration tests to primarily use TPF #3555

Uh oh!

Conversation

maastha commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change:

Required Checklist:

Further comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EspenAlbert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maastha Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maastha commented Aug 5, 2025 •

edited

Loading

maastha Aug 6, 2025 •

edited

Loading