r/aws_emrcontainers_virtual_cluster - new resource

[mantoine96]

This PR continues the work of @shuheiktgw in #17355 by finishing up the acceptance tests and fixing a few odds and ends.

It also adds the data source for aws_emrcontainers_virtual_cluster and the documentation for both resource and datasource.

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #16717.
Relates #17355.

Output from acceptance testing:

# With Import
$ make testacc TESTARGS='-run=TestAccAwsEMRContainersVirtualCluster_basic' # Currently failing, working on fixing it
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsEMRContainersVirtualCluster_basic -timeout 180m
=== RUN   TestAccAwsEMRContainersVirtualCluster_basic
=== PAUSE TestAccAwsEMRContainersVirtualCluster_basic
=== CONT  TestAccAwsEMRContainersVirtualCluster_basic
     resource_aws_emrcontainers_virtual_cluster_test.go:71: Step 2/2 error running import: exit status 1
        
        Error: Invalid provider configuration
        
          on /var/folders/24/3lvxwbqj7sx7tcs0b9rk8jg00000gn/T/plugintest319521152/work193135056/terraform_plugin_test.tf line 172:
         172: provider "kubernetes" {
        
        The configuration for provider["registry.terraform.io/hashicorp/kubernetes"]
        depends on values that cannot be determined until apply.
        
    testing_new.go:63: Error running post-test destroy, there may be dangling resources: Expected EMR containers virtual cluster to be destroyed, ef5wykjr8l891lr5nyw0hd7j4 found
--- FAIL: TestAccAwsEMRContainersVirtualCluster_basic (1212.39s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       1214.196s
FAIL
make: *** [testacc] Error 1
# With import commented out
make testacc TESTARGS='-run=TestAccAwsEMRContainersVirtualCluster_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsEMRContainersVirtualCluster_basic -timeout 180m
=== RUN   TestAccAwsEMRContainersVirtualCluster_basic
=== PAUSE TestAccAwsEMRContainersVirtualCluster_basic
=== CONT  TestAccAwsEMRContainersVirtualCluster_basic
--- PASS: TestAccAwsEMRContainersVirtualCluster_basic (1172.81s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1174.952s
...
$ make testacc TESTARGS='-run=TestAccAWSEMRContainersVirtualClusterDataSource_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSEMRContainersVirtualClusterDataSource_basic -timeout 180m
=== RUN   TestAccAWSEMRContainersVirtualClusterDataSource_basic
=== PAUSE TestAccAWSEMRContainersVirtualClusterDataSource_basic
=== CONT  TestAccAWSEMRContainersVirtualClusterDataSource_basic
--- PASS: TestAccAWSEMRContainersVirtualClusterDataSource_basic (1190.07s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1192.108s

Observations:

• Due to [EKS] [request]: On create: only return ACTIVE when endpoint actually usable aws/containers-roadmap#654 the test is flaky. Indeed sometimes the EKS cluster will show as available despite not being available. One solution could be to use the custom http provider used in feat: Replace the local-exec script with a http datasource for waiting cluster terraform-aws-modules/terraform-aws-eks#1339, however I decided against it for now as to not create a dependency on an external, non-Hashicorp owned, provider. Would love some guidance on this as I'm not sure if having flaky acceptance tests is acceptable here?
• Thanks @shuheiktgw for doing the groundwork in this PR.

Questions:

I'm currently trying to fix the failure to import the aws_emrcontainers_virtual_cluster.

In order to create the Kubernetes resources, I am using ExternalProviders to initialize and download the Kubernetes provider. According to the comments on ExternalProviders we should not need this since we don't rely on the provider being present to test the import.

ExternalProviders are providers the TestCase relies on that should
be downloaded from the registry during init. This is only really
necessary to set if you're using import, as providers in your config
will be automatically retrieved during init. Import doesn't use a
config, however, so we allow manually specifying them here to be
downloaded for import tests.

However when I don't specify ExternalProviders, I get this error

 data_source_aws_emrcontainers_virtual_cluster_test.go:23: Step 1/1 error: Error running pre-apply refresh: 
        Error: Could not load plugin
        
        
        Plugin reinitialization required. Please run "terraform init".
        
        Plugins are external binaries that Terraform uses to access and manipulate
        resources. The configuration provided requires plugins which can't be located,
        don't satisfy the version constraints, or are otherwise incompatible.
        
        Terraform automatically discovers provider requirements from your
        configuration, including providers used in child modules. To see the
        requirements and constraints, run "terraform providers".
        
        Failed to instantiate provider "registry.terraform.io/hashicorp/kubernetes" to
        obtain schema: unknown provider "registry.terraform.io/hashicorp/kubernetes"

If I specify the ExternalProviders for the kubernetes provider, I will not be able to test the import of a cluster

    resource_aws_emrcontainers_virtual_cluster_test.go:71: Step 2/2 error running import: exit status 1
       
       Error: Invalid provider configuration
       
         on /var/folders/24/3lvxwbqj7sx7tcs0b9rk8jg00000gn/T/plugintest319521152/work193135056/terraform_plugin_test.tf line 172:
        172: provider "kubernetes" {
       
       The configuration for provider["registry.terraform.io/hashicorp/kubernetes"]
       depends on values that cannot be determined until apply.

I think I am hitting the issue documented here hashicorp/terraform-plugin-sdk#562

I'm not sure what is the best way forward:

• Not supporting import and therefore not testing it
• Supporting import but not testing it

I feel either way is pretty suboptimal.

I would appreciate some advice on this problem!

[andormarkus]

+1
It would be great if this PR could be merged.

[dacort]

I've tested this live in my own account and it seems to work well. I was able to create a new EKS cluster, create a dedicated namespace, add the appropriate roles, and successfully run a sample job.

@thehunt33r One thing that doesn't seem to be documented here are all the getting started steps including creating service accounts, RBAC roles, IAM roles, etc.

Is that work you've already done and/or something that should be included in the documentation?

[dacort]

I was trying to run the acceptance tests as well, but wasn't able to get around the EMRContainers service role already existing without just deleting the aws_iam_service_linked_role.emrcontainers resource. :)

=== CONT  TestAccAwsEMRContainersVirtualCluster_basic
    resource_aws_emrcontainers_virtual_cluster_test.go:71: Step 1/2 error: Error running apply: exit status 1

        Error: Error creating service-linked role with name emr-containers.amazonaws.com: InvalidInput: Service role name AWSServiceRoleForAmazonEMRContainers has been taken in this account, please try a different suffix.
        	status code: 400, request id: 7b9e02da-4f3c-4a5b-8168-05071591cef6

          with aws_iam_service_linked_role.emrcontainers,
          on terraform_plugin_test.tf line 168, in resource "aws_iam_service_linked_role" "emrcontainers":
         168: resource "aws_iam_service_linked_role" "emrcontainers" {


        Error: Post "https://D937572A9472E4453D3834ED57F530E9.gr7.us-east-1.eks.amazonaws.com/apis/rbac.authorization.k8s.io/v1/namespaces/default/roles": dial tcp 44.195.172.250:443: i/o timeout

          with kubernetes_role.emrcontainers_role,
          on terraform_plugin_test.tf line 182, in resource "kubernetes_role" "emrcontainers_role":
         182: resource "kubernetes_role" "emrcontainers_role" {

[mantoine96]

Hey @dacort

The service linked role is a role you need to create for any new AWS account. Given that I expect acceptance tests to run in an empty account (everything should be destroyed once Terraform is finished running acceptance tests), then it makes sense to create the service linked role as part of the test.

Is that work you've already done and/or something that should be included in the documentation?

I didn't want to make the documentation too heavy by adding that, but I now realize those are prerequisites to be able to use the resource, so yeah I'll get that added to the documentation!

[dacort]

I expect acceptance tests to run in an empty account

Ah ok, that makes sense. Per the docs the role will get created automatically by EMR on EKS, but I don't see any issue with creating it manually.

Let me know if there's anything else I can help with. I've been messing around with the ExternalProvider issue as well, but haven't made any progress.

[dacort]

@thehunt33r I think the problem with the import test is that the values in the kubernetes provider can only be determined after a terraform apply. And, this is just a guess as I've never developed a plugin, I don't think the acceptance tests can be run in a way where it's possible to determine those values.

I've tried a number of different things including aliasing the provider, creating two tests where the first builds the infrastructure and the second adds the k8s provider, and a few other things but nothing seemed to work. I also tried looking at the k8s and EKS plugins to see if they used the provider in a similar way, but it doesn't look like they do. :(

[sahotay]

@dacort I see you mentioned you tried using @thehunt33r code in this PR and you got the EMR cluster created on EKS. Are there any steps that you can share with me to get the EMR up in EKS?

[dacort]

Are there any steps that you can share with me to get the EMR up in EKS?

Yes, will try to create a repo tomorrow with example. Most of what you need is defined in the acceptance tests, but you'll need to download this repo, change to this branch, and build the AWS plugin with this change. Then override the AWS provider in your .terraformrc. On mobile now, but should be able to put full example together by tomorrow!

[dacort]

@sahotay I added my example here. https://github.com/dacort/emr-eks-terraform

It shows:

• How to clone/build this PR
• Update your Terraform config to use it
• An example end-to-end terraform config

Let me if you give it a shot and just open an issue on my repo if you run into any issues.

[sahotay]

@thehunt33r I got this working and created the virtual cluster. Thanks, @dacort for your help and for creating the sample module.

few things I would like to mention:-

1. TF version should be above v0.14
2. conflict in provider.go

[WillemSFV]

@thehunt33r How close is this to the merge ✅ ?

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[dacort]

Happy to help review this if wanted. 😃

[ewbankkit]

LGTM 🚀.

% make testacc TESTS=TestAccEMRContainersVirtualCluster PKG=emrcontainers ACCTEST_PARALLELISM=1
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/emrcontainers/... -v -count 1 -parallel 1 -run='TestAccEMRContainersVirtualCluster'  -timeout 180m
=== RUN   TestAccEMRContainersVirtualClusterDataSource_basic
=== PAUSE TestAccEMRContainersVirtualClusterDataSource_basic
=== RUN   TestAccEMRContainersVirtualCluster_basic
=== PAUSE TestAccEMRContainersVirtualCluster_basic
=== RUN   TestAccEMRContainersVirtualCluster_disappears
=== PAUSE TestAccEMRContainersVirtualCluster_disappears
=== RUN   TestAccEMRContainersVirtualCluster_tags
=== PAUSE TestAccEMRContainersVirtualCluster_tags
=== CONT  TestAccEMRContainersVirtualClusterDataSource_basic
--- PASS: TestAccEMRContainersVirtualClusterDataSource_basic (1053.96s)
=== CONT  TestAccEMRContainersVirtualCluster_disappears
--- PASS: TestAccEMRContainersVirtualCluster_disappears (1172.94s)
=== CONT  TestAccEMRContainersVirtualCluster_tags
--- PASS: TestAccEMRContainersVirtualCluster_tags (1144.52s)
=== CONT  TestAccEMRContainersVirtualCluster_basic
--- PASS: TestAccEMRContainersVirtualCluster_basic (1028.82s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/emrcontainers	5101.662s

[ewbankkit]

@thehunt33r @shuheiktgw Thanks for the contribution 🎉 👏.

[github-actions]

This functionality has been released in v4.13.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.