Skip to content
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

Update SDK and make machine pool cloud_credential_secret_name optional to fix plan bug #1070

Merged
merged 2 commits into from Feb 17, 2023

Conversation

a-blender
Copy link
Contributor

@a-blender a-blender commented Feb 8, 2023

Issue:

#835

Problem

When creating an RKE2 cluster via Terraform on any hosted provider (Amazon EC2, Azure, Linode driver so far), Terraform computes a new value for a duplicate field cloud_credential_secret_name in the machine pool and then throws an error on a terraform apply pertaining to that new value. It works on a second run of terraform apply but is frustrating and widely seen by many customers.

Solution

That field currently exists in both rancher_cluster_v2 and rancher_cluster_v2.machine_pool. I couldn't find docs on why the cloud credential secret was added to the machine pool. Rancher pulls the cloud credential secret from the RKECommonNodeConfig and sets it on the newly created machine here as well as the cluster management Spec but doesn't do anything further with it. This looks like a legacy-related change from this PR but I don't fully know the context of the design decision here.

Rancher allows you to mix and match secrets, possibly to support the use case where a customer has machine pools in separate AWS/cloud accounts. The TF provider rancher2 must have parity with Rancher so this field cannot be removed (as I originally thought) because that would cause a regression.

Update 2/10: I believe this is a bug/compatibility issue in the Terraform plugin SDK. See comment #835 (comment) for further details.

This PR has the following fix

  • Update machine_pool.cloud_credential_secret_name to be Optional. This keeps parity with Terraform and fixes the plan bug
  • Update docs
  • Update Terraform plugin SDK to 1.17.2

Testing

Engineering Testing

Manual Testing

Tested with RKE2 clusters on Amazon EC2. Each cluster provisions successfully on the first try of terraform apply. If you do not specify a cloud credential, the cluster hangs with Waiting for viable init node trying to create the machine. I am open to discussion on this behavior.

Testing

  • only machine_pool.cloud_credential_secret_name
  • only global cloud_credential_secret_name
  • both global cloud_credential_secret_name and machine_pool.cloud_credential_secret_name
  • no cc (cannot create nodes as expected)

@a-blender a-blender force-pushed the fix-tf-inconsistent-plan branch 3 times, most recently from 4c73cb6 to 2ca64e6 Compare February 8, 2023 18:37
@a-blender a-blender changed the title Remove v2 machine pool cloud_credential_secret_name to fix plan bug Update SDK and make machine pool cloud_credential_secret_name optional to fix plan bug Feb 10, 2023
@a-blender a-blender requested review from a team and removed request for a team February 14, 2023 22:26
@a-blender a-blender added this to the v2.7.2 - Terraform milestone Feb 16, 2023
Copy link
Contributor

@HarrisonWAffel HarrisonWAffel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change looks fine, though the PR description talks about docs updates which I don't see yet

@a-blender a-blender marked this pull request as ready for review February 17, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants