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 new resources: aws_lightsail_container_service and aws_lightsail_container_service_deployment_version #20625

Merged
merged 14 commits into from
Jun 14, 2022

Conversation

KaguraKagura
Copy link
Contributor

@KaguraKagura KaguraKagura commented Aug 19, 2021

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

For TestAcc, because Lightsail container service only allows at most 5 container services being in creating/updating state, I am running the tests section by section, instead of using a single $ make testacc command.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_basic -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_basic
=== PAUSE TestAccAWSLightsailContainerService_basic
=== CONT  TestAccAWSLightsailContainerService_basic
--- PASS: TestAccAWSLightsailContainerService_basic (85.06s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       85.272s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_disappears'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_disappears -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_disappears
=== PAUSE TestAccAWSLightsailContainerService_disappears
=== CONT  TestAccAWSLightsailContainerService_disappears
--- PASS: TestAccAWSLightsailContainerService_disappears (80.99s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       81.147s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_Name'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_Name -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_Name
=== PAUSE TestAccAWSLightsailContainerService_Name
=== CONT  TestAccAWSLightsailContainerService_Name
--- PASS: TestAccAWSLightsailContainerService_Name (129.20s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       129.267s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_Deployment'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_Deployment -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_DeploymentContainerBasic
=== PAUSE TestAccAWSLightsailContainerService_DeploymentContainerBasic
=== RUN   TestAccAWSLightsailContainerService_DeploymentContainerEnvironment
=== PAUSE TestAccAWSLightsailContainerService_DeploymentContainerEnvironment
=== RUN   TestAccAWSLightsailContainerService_DeploymentContainerPort
=== PAUSE TestAccAWSLightsailContainerService_DeploymentContainerPort
=== RUN   TestAccAWSLightsailContainerService_DeploymentPublicEndpoint
=== PAUSE TestAccAWSLightsailContainerService_DeploymentPublicEndpoint
=== CONT  TestAccAWSLightsailContainerService_DeploymentContainerBasic
=== CONT  TestAccAWSLightsailContainerService_DeploymentPublicEndpoint
=== CONT  TestAccAWSLightsailContainerService_DeploymentContainerEnvironment
=== CONT  TestAccAWSLightsailContainerService_DeploymentContainerPort
--- PASS: TestAccAWSLightsailContainerService_DeploymentContainerBasic (574.05s)
--- PASS: TestAccAWSLightsailContainerService_DeploymentContainerPort (756.88s)
--- PASS: TestAccAWSLightsailContainerService_DeploymentContainerEnvironment (761.18s)
--- PASS: TestAccAWSLightsailContainerService_DeploymentPublicEndpoint (1122.84s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1122.945s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_IsDisabled'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_IsDisabled -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_IsDisabled
=== PAUSE TestAccAWSLightsailContainerService_IsDisabled
=== CONT  TestAccAWSLightsailContainerService_IsDisabled
--- PASS: TestAccAWSLightsailContainerService_IsDisabled (1337.36s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1337.425s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_Power'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_Power -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_Power
=== PAUSE TestAccAWSLightsailContainerService_Power
=== CONT  TestAccAWSLightsailContainerService_Power
--- PASS: TestAccAWSLightsailContainerService_Power (118.42s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       118.506s

$  make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_PublicDomainNames'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_PublicDomainNames -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_PublicDomainNames
=== PAUSE TestAccAWSLightsailContainerService_PublicDomainNames
=== CONT  TestAccAWSLightsailContainerService_PublicDomainNames
--- PASS: TestAccAWSLightsailContainerService_PublicDomainNames (15.78s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       15.856s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_Scale'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_Scale -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_Scale
=== PAUSE TestAccAWSLightsailContainerService_Scale
=== CONT  TestAccAWSLightsailContainerService_Scale
--- PASS: TestAccAWSLightsailContainerService_Scale (95.35s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       95.409s

$ make testacc TESTARGS='-run=TestAccAWSLightsailContainerService_Tags'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLightsailContainerService_Tags -timeout 180m
=== RUN   TestAccAWSLightsailContainerService_Tags
=== PAUSE TestAccAWSLightsailContainerService_Tags
=== CONT  TestAccAWSLightsailContainerService_Tags
--- PASS: TestAccAWSLightsailContainerService_Tags (88.18s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       88.244s

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/lightsail Issues and PRs that pertain to the lightsail service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/XL Managed by automation to categorize the size of a PR. labels Aug 19, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @KaguraKagura 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@KaguraKagura KaguraKagura changed the title Add resource_aws_lightsail_container_service Add new resource_aws_lightsail_container_service Aug 19, 2021
@AdamTylerLynch
Copy link
Collaborator

AWS contribution.

@KaguraKagura KaguraKagura changed the title Add new resource_aws_lightsail_container_service [WIP] Add new resource_aws_lightsail_container_service Aug 23, 2021
@KaguraKagura KaguraKagura changed the title [WIP] Add new resource_aws_lightsail_container_service Add new resource_aws_lightsail_container_service Aug 26, 2021
@KaguraKagura KaguraKagura marked this pull request as ready for review August 26, 2021 16:52
@breathingdust breathingdust added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 26, 2021
Copy link
Collaborator

@brittandeyoung brittandeyoung left a comment

Choose a reason for hiding this comment

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

Take a Look at my suggestions regarding moving your waiters to an internal waiter service.

return diag.FromErr(err)
}

stateChangeConf := &resource.StateChangeConf{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome! excited to see this change (Big Lighsail Fan). I was actually going to work on this myself once i had time. This will pair well with my aws_lightsail_database resource i have an open PR for. Might I suggest migrating waiters to an internal service waiter like in my PR? https://github.com/hashicorp/terraform-provider-aws/pull/18663/files aws/internal/service/lightsail/waiter/status.go. This way the same waiter can be used for all lightsail resources without having to re-write it on each CRUD operation.

Copy link
Contributor Author

@KaguraKagura KaguraKagura Aug 31, 2021

Choose a reason for hiding this comment

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

Hi Brittan, thank you for being a Lightsail fan! I also like Lightsail very much! 😎
For the StateChangeConf, the reason I rewrite them is that Lightsail container service does not return Operation like some other Lightsail resources, and Lightsail container service has relatively more complicated Pending and Target (there are 7 StateChangeConf in total and 6 of them are different). My thinking is that it might be better to leave them there and provide some context in the comments. But my lightsailContainerServiceRefreshFunc can definitely be moved to aws/internal/service/lightsail/waiter/status.go in future after the merge of your PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that the logic should be moved to the aws/internal/service/lightsail/waiter/status.go.

Check out how we did the multiple pending states and single target state in StorageGateway:
https://github.com/hashicorp/terraform-provider-aws/blob/main/aws/internal/service/storagegateway/waiter/waiter.go

Choose a reason for hiding this comment

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

I support @KaguraKagura's plan

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense @KaguraKagura. I am glad we have another lightsail fan working on adding functionality to the provider =) . Hopefully with this PR getting so much attention, some other Lightsail PRs that have been sitting for a while will get some attention as well.

@zhelding
Copy link
Contributor

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.

@mac-r
Copy link

mac-r commented Jan 11, 2022

@KaguraKagura @AdamTylerLynch you did huge amount of work. Do you guys plan to resolve merge conflicts in the near future? It would be awesome to see more Lightsail representation in Terraform.

… into f-lightsail-container-service while resolving the conflicts introduced by the refactor in hashicorp#20000
@KaguraKagura
Copy link
Contributor Author

@mac-r Thanks for reminding me of this PR! It surely has been a long time since I first opened it last summer!

@KaguraKagura
Copy link
Contributor Author

Acctests after resolving the conflicts introduced by the refactor in #20000:

$ make testacc TESTS=TestAccContainerService_basic PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_basic'  -timeout 180m
=== RUN   TestAccContainerService_basic
=== PAUSE TestAccContainerService_basic
=== CONT  TestAccContainerService_basic
--- PASS: TestAccContainerService_basic (341.23s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  341.304s

$ make testacc TESTS=TestAccContainerService_disappears PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_disappears'  -timeout 180m
=== RUN   TestAccContainerService_disappears
=== PAUSE TestAccContainerService_disappears
=== CONT  TestAccContainerService_disappears
--- PASS: TestAccContainerService_disappears (61.73s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  61.805s

$ make testacc TESTS=TestAccContainerService_Name PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_Name'  -timeout 180m
=== RUN   TestAccContainerService_Name
=== PAUSE TestAccContainerService_Name
=== CONT  TestAccContainerService_Name
--- PASS: TestAccContainerService_Name (135.19s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  135.255s

$ make testacc TESTS=TestAccContainerService_Deployment PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_Deployment'  -timeout 180m
=== RUN   TestAccContainerService_DeploymentContainerBasic
=== PAUSE TestAccContainerService_DeploymentContainerBasic
=== RUN   TestAccContainerService_DeploymentContainerEnvironment
=== PAUSE TestAccContainerService_DeploymentContainerEnvironment
=== RUN   TestAccContainerService_DeploymentContainerPort
=== PAUSE TestAccContainerService_DeploymentContainerPort
=== RUN   TestAccContainerService_DeploymentPublicEndpoint
=== PAUSE TestAccContainerService_DeploymentPublicEndpoint
=== CONT  TestAccContainerService_DeploymentContainerBasic
=== CONT  TestAccContainerService_DeploymentPublicEndpoint
=== CONT  TestAccContainerService_DeploymentContainerPort
=== CONT  TestAccContainerService_DeploymentContainerEnvironment
--- PASS: TestAccContainerService_DeploymentContainerBasic (588.54s)
--- PASS: TestAccContainerService_DeploymentContainerEnvironment (684.48s)
--- PASS: TestAccContainerService_DeploymentContainerPort (743.60s)
--- PASS: TestAccContainerService_DeploymentPublicEndpoint (1067.74s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  1067.812s

$ make testacc TESTS=TestAccContainerService_IsDisabled PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_IsDisabled'  -timeout 180m
=== RUN   TestAccContainerService_IsDisabled
=== PAUSE TestAccContainerService_IsDisabled
=== CONT  TestAccContainerService_IsDisabled
--- PASS: TestAccContainerService_IsDisabled (1300.97s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  1301.050s

$ make testacc TESTS=TestAccContainerService_Power PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_Power'  -timeout 180m
=== RUN   TestAccContainerService_Power
=== PAUSE TestAccContainerService_Power
=== CONT  TestAccContainerService_Power
--- PASS: TestAccContainerService_Power (92.16s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  92.219s

$ make testacc TESTS=TestAccContainerService_PublicDomainNames PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_PublicDomainNames'  -timeout 180m
=== RUN   TestAccContainerService_PublicDomainNames
=== PAUSE TestAccContainerService_PublicDomainNames
=== CONT  TestAccContainerService_PublicDomainNames
--- PASS: TestAccContainerService_PublicDomainNames (7.28s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  7.342s

$ make testacc TESTS=TestAccContainerService_Scale PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_Scale'  -timeout 180m
=== RUN   TestAccContainerService_Scale
=== PAUSE TestAccContainerService_Scale
=== CONT  TestAccContainerService_Scale
--- PASS: TestAccContainerService_Scale (91.36s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  91.426s

$ make testacc TESTS=TestAccContainerService_Tag PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20 -run='TestAccContainerService_Tag'  -timeout 180m
=== RUN   TestAccContainerService_Tags
=== PAUSE TestAccContainerService_Tags
=== CONT  TestAccContainerService_Tags
--- PASS: TestAccContainerService_Tags (77.60s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lightsail  77.667s

@brittandeyoung
Copy link
Collaborator

@KaguraKagura For Awareness I am working on making an entirely separate provider for AWS Lightsail. This way it will get its own dedicated attentions when it comes to PRs and development. I would like to allow you the opportunity to submit this resource as a PR before I do it myself. Not much refactoring should be needed other than the lightsail provider is using the aws-sdk-go-v2.

Here is the repo : https://github.com/DeYoungTech/terraform-provider-awslightsail

The provider is published here: https://registry.terraform.io/providers/DeYoungTech/awslightsail/latest

I already have resource Parity with the current aws provider for lightsail resources and have added a couple additional ones that I had open PRs for ( for over a year with no reviews).

@mlburggr
Copy link

It'd be good to get this change into your Lightsail-specific provider, but it would be ideal if we can get this change into the main Terraform AWS provider, so that users don't have to use a separate provider for Lightsail than they do for other AWS services.

Is there anything blocking this from being merged to the main AWS provider? If so, I'm happy to resolve them.

@brittandeyoung
Copy link
Collaborator

I agree @mlburggr it would be ideal to get these resources into the main AWS provider. However, I have open pull requests for Lightsail resources that have been open for over a year and refactored multiple times as required with no reviews.

This is what brought me to start a separate Lightsail provider. I would love for some pull requests for lightsail on the main AWS provider to get some love.

For awareness this resource is now published with the latest version of the awslightsail provider (v0.7.0). https://registry.terraform.io/providers/DeYoungTech/awslightsail/latest

I decided to split it into three separate resources:
container_service
container_deployment
container_public_domain_names
certificate

I tagged @KaguraKagura as a co-author on relevant resources. Tried to give credit, where credit was due as I used a lot of this pull request when making these resources.

@AdamTylerLynch
Copy link
Collaborator

Reviewing Now.

Copy link
Collaborator

@AdamTylerLynch AdamTylerLynch left a comment

Choose a reason for hiding this comment

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

Really thorough PR. I have asked for a few changes. Once those have been made ping me, and I will conduct another review.

internal/service/lightsail/container_service.go Outdated Show resolved Hide resolved
internal/service/lightsail/container_service.go Outdated Show resolved Hide resolved
internal/service/lightsail/container_service.go Outdated Show resolved Hide resolved
internal/service/lightsail/container_service.go Outdated Show resolved Hide resolved
Required: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"container_name": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The AWS Public API documentation for Container does not have container_name? Please verify.

I do see it defined on the public_endpoint, maybe a documentation issue?

https://docs.aws.amazon.com/lightsail/2016-11-28/api-reference/API_Container.html

Copy link
Contributor Author

@KaguraKagura KaguraKagura Apr 19, 2022

Choose a reason for hiding this comment

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

In here https://docs.aws.amazon.com/lightsail/2016-11-28/api-reference/API_CreateContainerServiceDeployment.html, I think containers is a "String to Container object map", where that string is the container's name. Terraform requires a concrete name, and so I just give it the name container_name.


command = []

environment {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you provide a sample command. Maybe just yum update -y ?

Copy link
Contributor Author

@KaguraKagura KaguraKagura Apr 19, 2022

Choose a reason for hiding this comment

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

I believe command overrides all the commands the docker image contains when the image is built (kind of a not often used feature?). I'm not sure of a good example. Perhaps @mlburggr has some?

specify are used when you create a deployment with a container configured as the public endpoint of your container
service. If you don't specify public domain names, then you can use the default domain of the container service.
Defined below.
* WARNING: You must create and validate an SSL/TLS certificate before you can use public domain names with your
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can a certificate be created via Terraform for this to help the user do everything through the HCL interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I was doing this pull request as an intern at Lightsail, it was not possible then. Perhaps @mlburggr has newer information.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@AdamTylerLynch and @KaguraKagura I currently have an implementation of the certificate resource in my separate lightsail provider and plan on opening up a PR to this provider with the resource as soon as my other PR is merged #18663 as this PR has internal waiters written that I would like to use when writing other lightsail resources. The example of the certificate resource is currently here: https://github.com/DeYoungTech/terraform-provider-awslightsail/blob/main/internal/lightsail/certificate.go

website/docs/r/lightsail_container_service.html.markdown Outdated Show resolved Hide resolved
"availability_zone": {
Type: schema.TypeString,
Computed: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would expect to see created_at, isDisabled, state_detail.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any other elements returned from the payload.

Copy link
Contributor Author

@KaguraKagura KaguraKagura Apr 19, 2022

Choose a reason for hiding this comment

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

is_disabled was there; added created_at. I omitted state_detail because in here https://docs.aws.amazon.com/lightsail/2016-11-28/api-reference/API_ContainerServiceStateDetail.html, it says

Note
The state detail is populated only when a container service is in a PENDING, DEPLOYING, or UPDATING state.

and for Lightsail container service in terreform, all operations won't return unless the state moves out of the above states.

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Jun 8, 2022
@KaguraKagura KaguraKagura reopened this Jun 8, 2022
@KaguraKagura
Copy link
Contributor Author

Sorry I clicked the wrong button and closed this PR, but I reopened it now...
As for separating out the deployment configuration, I don't have much concern. The only thing that might need attention is that a deployment cannot exist without a valid container service.

@anGie44
Copy link
Contributor

anGie44 commented Jun 8, 2022

No worries @KaguraKagura thanks for your response! I'll push up a commit to remove the deployment logic then and aim to get this into an upcoming release since the PR already went through review.

@brittandeyoung
Copy link
Collaborator

@anGie44 Here is the pull request for the certificate resource mentioned earlier: #25283

@github-actions github-actions bot added client-connections Pertains to the AWS Client and service connections. sweeper Pertains to changes to or issues with the sweeper. labels Jun 14, 2022
@anGie44 anGie44 force-pushed the f-lightsail-container-service branch 4 times, most recently from b066a4f to 28ab845 Compare June 14, 2022 16:03
@anGie44 anGie44 force-pushed the f-lightsail-container-service branch from 28ab845 to e9d5e06 Compare June 14, 2022 16:15
@github-actions github-actions bot added client-connections Pertains to the AWS Client and service connections. and removed client-connections Pertains to the AWS Client and service connections. labels Jun 14, 2022
@anGie44 anGie44 force-pushed the f-lightsail-container-service branch from 7598492 to 249f89f Compare June 14, 2022 16:34
@anGie44 anGie44 force-pushed the f-lightsail-container-service branch from 249f89f to 0bf975d Compare June 14, 2022 16:40
@anGie44 anGie44 added this to the v4.19.0 milestone Jun 14, 2022
@anGie44
Copy link
Contributor

anGie44 commented Jun 14, 2022

Output of acceptance tests (commercial):

 make testacc TESTARGS='-run=TestAccLightsailContainerService' PKG=lightsail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 20  -run=TestAccLightsailContainerService -timeout 180m
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_Basic
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_Basic
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_Multiple
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_Multiple
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_Environment
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_Environment
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_Ports
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_Ports
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_PublicEndpoint
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_PublicEndpoint
=== RUN   TestAccLightsailContainerServiceDeploymentVersion_Container_EnableService
=== PAUSE TestAccLightsailContainerServiceDeploymentVersion_Container_EnableService
=== RUN   TestAccLightsailContainerService_basic
=== PAUSE TestAccLightsailContainerService_basic
=== RUN   TestAccLightsailContainerService_disappears
=== PAUSE TestAccLightsailContainerService_disappears
=== RUN   TestAccLightsailContainerService_Name
=== PAUSE TestAccLightsailContainerService_Name
=== RUN   TestAccLightsailContainerService_IsDisabled
=== PAUSE TestAccLightsailContainerService_IsDisabled
=== RUN   TestAccLightsailContainerService_Power
=== PAUSE TestAccLightsailContainerService_Power
=== RUN   TestAccLightsailContainerService_PublicDomainNames
=== PAUSE TestAccLightsailContainerService_PublicDomainNames
=== RUN   TestAccLightsailContainerService_Scale
=== PAUSE TestAccLightsailContainerService_Scale
=== RUN   TestAccLightsailContainerService_tags
=== PAUSE TestAccLightsailContainerService_tags
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_Basic
=== CONT  TestAccLightsailContainerService_disappears
=== CONT  TestAccLightsailContainerService_Scale
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_PublicEndpoint
=== CONT  TestAccLightsailContainerService_Power
=== CONT  TestAccLightsailContainerService_Name
=== CONT  TestAccLightsailContainerService_basic
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_Ports
=== CONT  TestAccLightsailContainerService_tags
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_Environment
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_Multiple
=== CONT  TestAccLightsailContainerServiceDeploymentVersion_Container_EnableService
=== CONT  TestAccLightsailContainerService_IsDisabled
=== CONT  TestAccLightsailContainerService_PublicDomainNames
--- PASS: TestAccLightsailContainerService_PublicDomainNames (12.92s)
--- PASS: TestAccLightsailContainerService_Power (127.00s)
--- PASS: TestAccLightsailContainerService_IsDisabled (165.15s)
--- PASS: TestAccLightsailContainerService_disappears (185.28s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_EnableService (262.58s)
--- PASS: TestAccLightsailContainerService_Scale (275.90s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_Basic (321.99s)
--- PASS: TestAccLightsailContainerService_Name (350.86s)
--- PASS: TestAccLightsailContainerService_basic (355.31s)
--- PASS: TestAccLightsailContainerService_tags (387.94s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_Multiple (675.39s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_Ports (945.80s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_Environment (1120.79s)
--- PASS: TestAccLightsailContainerServiceDeploymentVersion_Container_PublicEndpoint (1205.98s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lightsail	1209.483s

Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

Thanks again @KaguraKagura for your contributions and @brittandeyoung for your feedback! I've gone ahead and done the refactoring bits to pull out the deployment configuration as a standalone resource, namely aws_lightsail_container_service_deployment_version as the Lightsail service treats each deployment as a new version.

@anGie44 anGie44 merged commit cf68c67 into hashicorp:main Jun 14, 2022
@anGie44 anGie44 changed the title Add new resource_aws_lightsail_container_service Add new resources: aws_lightsail_container_service and aws_lightsail_container_service_deployment_version Jun 14, 2022
@github-actions
Copy link

This functionality has been released in v4.19.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
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
client-connections Pertains to the AWS Client and service connections. documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/lightsail Issues and PRs that pertain to the lightsail service. size/XL Managed by automation to categorize the size of a PR. sweeper Pertains to changes to or issues with the sweeper. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants