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

Add ability to set Performance Mode in aws_efs_file_system. #7791

Merged

Conversation

kwilczynski
Copy link
Contributor

The Elastic File System (EFS) allows for setting a Performance Mode during
creation, thus enabling anyone to chose performance of the file system according
to their particular needs. This commit adds an optional "performance_mode"
attribute to the aws_efs_file_system resource so that an appropriate mode can be
set as needed.

Signed-off-by: Krzysztof Wilczynski krzysztof.wilczynski@linux.com

The Elastic File System (EFS) allows for setting a Performance Mode during
creation, thus enabling anyone to chose performance of the file system according
to their particular needs. This commit adds an optional "performance_mode"
attribte to the aws_efs_file_system resource so that an appropriate mode can be
set as needed.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
@kwilczynski kwilczynski changed the title Add ability to set Performance Mode in aws_efs_file_system. [WIP] Add ability to set Performance Mode in aws_efs_file_system. Jul 24, 2016
@kwilczynski
Copy link
Contributor Author

Resolves #7773.

@kwilczynski
Copy link
Contributor Author

Test is passing:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSFileSystem_basic'

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/07/25 03:16:19 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSFileSystem_basic -timeout 120m
=== RUN   TestAccAWSEFSFileSystem_basic
--- PASS: TestAccAWSEFSFileSystem_basic (84.05s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    84.074s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSFileSystem_importBasic'

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/07/25 03:14:23 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSFileSystem_importBasic -timeout 120m
=== RUN   TestAccAWSEFSFileSystem_importBasic
--- PASS: TestAccAWSEFSFileSystem_importBasic (34.85s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    34.873s

Plus, verified from the command line:

$ aws efs describe-file-systems --region us-west-2
{
    "FileSystems": [
        {
            "SizeInBytes": {
                "Value": 6144
            },
            "CreationToken": "terraform-qyipifpi2zcrda6yphpslydvgy",
            "CreationTime": 1469401038.0,
            "PerformanceMode": "maxIO",
            "FileSystemId": "fs-c0e21369",
            "NumberOfMountTargets": 0,
            "LifeCycleState": "available",
            "OwnerId": "635543228030"
        }
    ]
}

@kwilczynski
Copy link
Contributor Author

@phinze a few questions to you.

How does one handle import? Is there something more that needs to be done for the import to work? I've noticed that import test excludes one of the arguments, does this suppose to be like that?

Also, I personally think that "reference_name" as an attribute name is a little bit confusing, perhaps renaming it to something along the lines of "creation_token_prefix" or just "token_prefix" would make it less of a misnomer. We could also let the user decide and introduce "creation_token" and "creation_token_prefix", which would be mutually-exclusive and should the former be absent and/or empty, then the second would be take into account, and should it be empty, then we would auto-generate (so business as usual). What do you think?

@kwilczynski
Copy link
Contributor Author

@stack72 and I had a quick chat. I will add better test coverage e.g. test validation functions, etc.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
@kwilczynski
Copy link
Contributor Author

@stack72 let me know about the attribute name change, if you have any ideas.

Add the "creation_token" attribute so that the resource follows the API more
closely (as per the convention), thus deprecate the "reference_name" attribute.

Update tests and documentation accordingly.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
@kwilczynski
Copy link
Contributor Author

Updated tests are passing:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSFileSystem_basic'

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/07/28 06:56:55 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSFileSystem_basic -timeout 120m
=== RUN   TestAccAWSEFSFileSystem_basic
--- PASS: TestAccAWSEFSFileSystem_basic (81.56s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    81.589s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestResourceAWSEFS*'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/07/28 06:59:26 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestResourceAWSEFS* -timeout 120m
=== RUN   TestResourceAWSEFSReferenceName_validation
--- PASS: TestResourceAWSEFSReferenceName_validation (0.00s)
=== RUN   TestResourceAWSEFSPerformanceMode_validation
--- PASS: TestResourceAWSEFSPerformanceMode_validation (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    0.021s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSFileSystem_importBasic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/07/28 07:01:51 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSFileSystem_importBasic -timeout 120m
=== RUN   TestAccAWSEFSFileSystem_importBasic
--- PASS: TestAccAWSEFSFileSystem_importBasic (35.37s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    35.397s

And verified from the command line:

$ aws efs describe-file-systems --region us-west-2
{
    "FileSystems": [
        {
            "SizeInBytes": {
                "Value": 6144
            },
            "CreationToken": "supercalifragilisticexpialidocious",
            "CreationTime": 1469656720.0,
            "PerformanceMode": "maxIO",
            "FileSystemId": "fs-ae1bea07",
            "NumberOfMountTargets": 0,
            "LifeCycleState": "available",
            "OwnerId": "635543228030"
        }
    ]
}

@kwilczynski kwilczynski changed the title [WIP] Add ability to set Performance Mode in aws_efs_file_system. Add ability to set Performance Mode in aws_efs_file_system. Jul 27, 2016
@kwilczynski
Copy link
Contributor Author

Over to you @phinze 🚀

@stack72
Copy link
Contributor

stack72 commented Jul 28, 2016

Hi @kwilczynski

This LGTM!

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEFSFileSystem_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEFSFileSystem_ -timeout 120m
=== RUN   TestAccAWSEFSFileSystem_importBasic
--- PASS: TestAccAWSEFSFileSystem_importBasic (33.59s)
=== RUN   TestAccAWSEFSFileSystem_basic
--- PASS: TestAccAWSEFSFileSystem_basic (74.61s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    108.226s

@stack72 stack72 merged commit 63a14be into hashicorp:master Jul 28, 2016
@kwilczynski kwilczynski deleted the feature/expose-efs-performance-mode branch May 20, 2018 06:10
@ghost
Copy link

ghost commented Apr 3, 2020

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants