[DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 #1224

felipe-colussi · 2023-09-08T20:52:23Z

DNM: waiting for release of 4.0.0

Issue: #1080

Observation: I Had a lot of conflicts on the PR #1207 IMO it was safer to close that one and create a new one cherry-picking my changes than it was to resolve the conflicts on that pr.

Problem

Migrate SDK1 to SDK2

Solution

Migrate SDK1 to SDK2:
Main changes:

Change Create, Read, Update, Delete fields to CreateContext, ReadContext . . .
Change function signatures from dataSourceRancher2....( d *schema.ResourceData, meta interface{}) error to dataSourceRancher2...(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics
Change some error handling to behave with diag.Diagnostics.
Remove MaxItems from TF arguments with Compiled: true. Required by migrating sdk1 to 2.
Add some argument that were being set without being on the schema. Required by migrating sdk1 to 2.
Changed some types and added some so they correspond to the schemas. Required by migrating sdk1 to 2.

Observation:
I had a loot of errors on the Tests, mainly 401 during the delete of TestAccRancher2Upgrade. I Did 2 commits to add some debug logic to that test and on that moment It just worked. I'm letting the debug (prints) on this PR so if I go back having that error I can confirm if the envs were being overwritten without need (I do believe that this was causing the error).

Testing

Engineering Testing

Manual Testing

I Tested on TF trying to create, destroy scale and change objects.

DO-RKE2-Basic.txt
AWS-RKE2-CUSTOM.txt
aws-rke1-custom.txt

Automated Testing

I ran all automated tests, also had to change some to match the SDK upgrade.

QA Testing Considerations

This is a huge change, I wasn't able to get a bug that happened due to the migration, that said please test this intensively.

Regressions Considerations

rancher2/data_source_rancher2_notifier.go

rancher2/import_rancher2_catalog_v2.go

rancher2/data_source_rancher2_cluster.go

rancher2/data_source_rancher2_cluster_v2.go

main.go

rancher2/structure_agent_deployment_customization_v2.go

rancher2/structure_cluster_v2.go

rancher2/0_provider_upgrade_test.go

a-blender · 2023-09-18T17:25:24Z

@felipe-colussi In general, nice job! Had some comments following up our discussion on the source code and schema updates last week. Please do the following

Remove all comments/TODO comments left in the source code
Clean up the error messages I referenced
Post more details in Manual Testing (types of cluster tested, test upgrade tf from old version to version with SDK v2 updates, tf config files)
Post test results for provisioning an RKE and v2 cluster with cluster_agent_deployment_customization and fleet_agent_deployment_customization defined. Verify it works and is showing the correct values in the tf state and cluster mgmt yaml

rancher2/structure_cluster_v2.go

rancher2/0_provider_upgrade_test.go

a-blender · 2023-09-22T18:02:35Z

@felipe-colussi Can you also please squash the top 8-10 commits https://github.com/rancher/terraform-provider-rancher2/pull/1224/commits? They are all pretty small updates.

rancher2/0_provider_upgrade_test.go

rancher2/data_source_rancher2_cluster.go

rancher2/data_source_rancher2_project.go

a-blender · 2023-10-19T15:16:09Z

Looks like the build is failing due to go fmt. Run gofmt -w on these files

./rancher2/structure_cluster_v2_test.go
./rancher2/resource_rancher2_global_role.go

- Add missing fields that are required in the new schema - Update agentDeploymentCustomization types - Update error to diag - Fix build errors and tests, rebase with master

a-blender · 2023-10-19T21:37:42Z

The changes got massively confusing on this PR so we did the following

git rebase master and resolved the merge conflicts
verified changes by the upgrade to sdk v2 were still the same, checked build and tests are working and squashed 30 commits into 1 with a readable message.

@felipe-colussi If there are additional review comments, please include them in only 1-2 additional commits that are for addressed comments.

Issues with the auto-generated /vendor dir turned out to be unique to my local environment. Unused dependency fix will be put into a separate PR if needed.

felipe-colussi · 2023-10-20T16:48:05Z

Smoke tests after migration:
Rke2 - AWS = Create Auth -> Cluster creation -> scale down -> scale Up -> Delete. OK.

RKE2 - DO = Create Auth -> Create Cluster -> Upgrade K8S version -> Delete. Ok.

RKE - DO = "Data" Auth -> Create Cluster with cluster_agent_deployment_customization -> Remove override_affinity+override_resource_requirements+fleet_agent_deployment_customization -> Increase node count -> Delete. Ok
rke-do-customization.txt

RKE - AWS -> Create Auth -> Create Cluster (k8s version and network plugin only) -> increase node count -> dete. Ok.

jakefhyde · 2023-10-23T20:27:23Z

rancher2/0_provider_upgrade_test.go

@@ -53,15 +53,14 @@ resource "rancher2_namespace" "testacc" {
 	testAccCheckRancher2UpgradeCatalogV24 = testAccRancher2CatalogGlobal + testAccRancher2CatalogCluster + testAccRancher2CatalogProject
 	testAccCheckRancher2UpgradeCertificateV24 = testAccRancher2Certificate + testAccRancher2CertificateNs
 	testAccCheckRancher2BootstrapV23 = `
-provider "rancher2" {


For my own understanding can you explain why this was necessary

This is the fix for the flaky test that was happening.

I didn't found anything related to it on the migration guides, but once I started reading the code docs the new test framework explicitly says that we can't use alias on providers and recommend to use multiple providers instead.

https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource@v2.28.0#TestCase.ProviderFactories

felipe-colussi mentioned this pull request Sep 8, 2023

Migrate sdk v1 to v2 #1207

Closed

felipe-colussi requested review from a-blender, a team and jakefhyde September 8, 2023 20:53

a-blender suggested changes Sep 18, 2023

View reviewed changes

a-blender suggested changes Sep 22, 2023

View reviewed changes

rancher2/structure_cluster_v2.go Outdated Show resolved Hide resolved

a-blender approved these changes Sep 22, 2023

View reviewed changes

a-blender self-requested a review September 22, 2023 17:51

a-blender suggested changes Sep 22, 2023

View reviewed changes

rancher2/0_provider_upgrade_test.go Show resolved Hide resolved

felipe-colussi force-pushed the migrate-sdk2-resolve branch 2 times, most recently from c1f810f to 86630d5 Compare September 22, 2023 18:29

jakefhyde requested changes Sep 22, 2023

View reviewed changes

rancher2/0_provider_upgrade_test.go Outdated Show resolved Hide resolved

rancher2/data_source_rancher2_cluster.go Show resolved Hide resolved

rancher2/data_source_rancher2_project.go Outdated Show resolved Hide resolved

felipe-colussi force-pushed the migrate-sdk2-resolve branch 4 times, most recently from 8e72589 to ea3ec1f Compare September 27, 2023 18:19

felipe-colussi requested review from a-blender and jakefhyde October 2, 2023 19:08

a-blender force-pushed the migrate-sdk2-resolve branch from 4859bdf to f6e126a Compare October 19, 2023 20:25

Migrate provider to SDK v2

59f6565

- Add missing fields that are required in the new schema - Update agentDeploymentCustomization types - Update error to diag - Fix build errors and tests, rebase with master

a-blender force-pushed the migrate-sdk2-resolve branch from f6e126a to 59f6565 Compare October 19, 2023 21:30

a-blender approved these changes Oct 23, 2023

View reviewed changes

jakefhyde reviewed Oct 23, 2023

View reviewed changes

jakefhyde approved these changes Oct 23, 2023

View reviewed changes

felipe-colussi changed the title ~~Migrate from sdk-v1 to sdk2-v2~~ [DNM] Migrate from sdk-v1 to sdk2-v2 Oct 25, 2023

a-blender changed the title ~~[DNM] Migrate from sdk-v1 to sdk2-v2~~ [DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 Oct 26, 2023

kkaempf added the area/dependencies label Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 #1224

[DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 #1224

felipe-colussi commented Sep 8, 2023 •

edited

Loading

a-blender commented Sep 18, 2023 •

edited

Loading

a-blender commented Sep 22, 2023 •

edited

Loading

a-blender commented Oct 19, 2023 •

edited

Loading

a-blender commented Oct 19, 2023 •

edited

Loading

felipe-colussi commented Oct 20, 2023 •

edited

Loading

jakefhyde Oct 23, 2023

felipe-colussi Oct 24, 2023

[DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 #1224

Are you sure you want to change the base?

[DNM until after 2.8 release] Migrate from sdk-v1 to sdk2-v2 #1224

Conversation

felipe-colussi commented Sep 8, 2023 • edited Loading

DNM: waiting for release of 4.0.0

Issue: #1080

Observation: I Had a lot of conflicts on the PR #1207 IMO it was safer to close that one and create a new one cherry-picking my changes than it was to resolve the conflicts on that pr.

Problem

Solution

Testing

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

a-blender commented Sep 18, 2023 • edited Loading

a-blender commented Sep 22, 2023 • edited Loading

a-blender commented Oct 19, 2023 • edited Loading

a-blender commented Oct 19, 2023 • edited Loading

felipe-colussi commented Oct 20, 2023 • edited Loading

jakefhyde Oct 23, 2023

Choose a reason for hiding this comment

felipe-colussi Oct 24, 2023

Choose a reason for hiding this comment

felipe-colussi commented Sep 8, 2023 •

edited

Loading

a-blender commented Sep 18, 2023 •

edited

Loading

a-blender commented Sep 22, 2023 •

edited

Loading

a-blender commented Oct 19, 2023 •

edited

Loading

a-blender commented Oct 19, 2023 •

edited

Loading

felipe-colussi commented Oct 20, 2023 •

edited

Loading