feat: add aws_appconfig_deployment #20172

suzuki-shunsuke · 2021-07-13T23:48:28Z

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

Relates #11973

Output from acceptance testing:

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

Supports AppConfig Deployment.
AppConfig Deployment doesn't support common CRUD API.
Instead of common CRUD API, AppConfig Deployment supports the following API.

get-deployment https://awscli.amazonaws.com/v2/documentation/api/latest/reference/appconfig/get-deployment.html
start-deployment https://awscli.amazonaws.com/v2/documentation/api/latest/reference/appconfig/start-deployment.html
stop-deployment https://awscli.amazonaws.com/v2/documentation/api/latest/reference/appconfig/stop-deployment.html

So I don't know how we should implement Update and Delete.

suzuki-shunsuke · 2021-07-13T23:49:09Z

This is still draft.
No test and document.
The update and deletion aren't supported.

#11973 (comment)

test
document
update
delete

anGie44 · 2021-07-15T21:13:09Z

Hi @suzuki-shunsuke , thank you for this PR ! Since update is only supported for the resource tags, but not the other arguments, I would just add tags_all specific logic to the Update CRUD operation, similar to what is seen here in the EFS access point resource: https://github.com/hashicorp/terraform-provider-aws/blob/main/aws/resource_aws_efs_access_point.go#L158-L170.

And then for the Delete CRUD operation, I would have that operation log a warning similar to what is done in the Default VPC resource (https://github.com/hashicorp/terraform-provider-aws/blob/main/aws/resource_aws_default_vpc.go#L62-L65) and return nil (similar to what schema.Noop does, but with the additional log warning).

One additional thing to note is that it's possible for practitioners to hit a ConflictException if deploying a new version before the previous has reached the Complete state. Thus I would refer to this documentation page https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/retries-and-waiters.md#asynchronous-operations to implement a waiter that waits for the deployment to be complete before moving to the next operation e.g.

// in file aws/internal/service/appconfig/waiter.go

...
const DeploymentCreatedTimeout = 5 * time.Minute

func DeploymentCreated(conn *appconfig.AppConfig, appID, envID string, deployNum int64) error {
	stateConf := &resource.StateChangeConf{
		Pending: []string{appconfig.DeploymentStateBaking, appconfig.DeploymentStateRollingBack, appconfig.DeploymentStateValidating, appconfig.DeploymentStateDeploying},
		Target:  []string{appconfig.DeploymentStateComplete},
		Refresh: DeploymentStatus(conn, appID, envID, deployNum),
		Timeout: DeploymentCreatedTimeout,
	}

	_, err := stateConf.WaitForState()

	return err
}


// in file aws/internal/service/appconfig/status.go

 ...
func DeploymentStatus(conn *appconfig.AppConfig, appID, envID string, deployNum int64) resource.StateRefreshFunc {
	return func() (interface{}, string, error) {
		input := &appconfig.GetDeploymentInput{
			ApplicationId:    aws.String(appID),
			DeploymentNumber: aws.Int64(deployNum),
			EnvironmentId:    aws.String(envID),
		}

		output, err := conn.GetDeployment(input)

		if err != nil {
			return nil, "", err
		}

		if output == nil {
			return nil, "", nil
		}

		return output, aws.StringValue(output.State), nil
	}
}


// in aws/resource_aws_appconfig_deployment.go

...
d.SetId(fmt.Sprintf("%s/%s/%d", appID, envID, deployNum))

if err := waiter.DeploymentCreated(conn, appID, envID, deployNum); err != nil {		
    return fmt.Errorf("error waiting for AppConfig Deployment (%s) creation: %w", d.Id(), err)
}

return resourceAwsAppconfigDeploymentRead(d, meta)

Please reach out if you have any questions!

suzuki-shunsuke · 2021-07-17T08:39:52Z

Hi, @anGie44
Thank you for your kind explanation!
I'll update Update and Delete function.

AWS Go SDK pointer conversion function for `d.Set()` value is extraneous

suzuki-shunsuke · 2021-07-17T11:13:48Z

I have implemented Update and Delete and waiter and tests.
But tests aren't enough yet.

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

suzuki-shunsuke · 2021-07-17T11:30:46Z

I have a consideration about the waiter.

In the code #20172 (comment) , the time is 5 minutes.
But if the deployment strategy requires over 5 minutes, the creation would fail.

suzuki-shunsuke · 2021-07-17T11:34:14Z

Oh, I see. We have to run the waiter before creating the Deployment to wait for the previous version's Deployment.

suzuki-shunsuke · 2021-07-17T11:35:58Z

We have to list deployments and check the latest Deployment's status before starting Deployment.

suzuki-shunsuke · 2021-07-17T11:42:31Z

Hmm...
To get the latest Deployment, do we have to get all Deployments?

https://awscli.amazonaws.com/v2/documentation/api/latest/reference/appconfig/list-deployments.html

suzuki-shunsuke · 2021-07-17T11:48:53Z

Or instead of checking the latest Deployment, we start Deployment and if ConflictException is returned retry to start Deployment.

suzuki-shunsuke · 2021-07-17T11:52:53Z

Maybe the creation should not be retried but be failed immediately in case the other Deployment isn't finished.

This reverts commit 7f4b6bc.

suzuki-shunsuke · 2021-07-17T12:00:56Z

0834f15 Revert 7f4b6bc at once #20172 (comment)

anGie44 · 2021-07-19T17:49:58Z

Thank you for this quick replies @suzuki-shunsuke ! I'll have a re-look at things today and get back to your comments above :)

anGie44 · 2021-07-22T15:07:21Z

Hi @suzuki-shunsuke . so for the waiter functionality, that should occur right after calling StartDeployment, so that the Read operation does not occur until we can determine if the deployment is complete. What you had in a previous commit looked great 👍 For the case of 2 deployments occurring in parallel, which practitioners would then encounter the ConflictException , we can handle this by adding an explicit retry handling in aws/config.go so that the 2nd deployment for example would in turn keep retrying until the 1st finishes; there is no defined timeout time in those cases, but rather we leave it to the SDK to attempt until the max number of retries is hit. I've added these changes to ensure we get your work in for today's release.

Output of acceptance tests (commercial):

--- PASS: TestAccAWSAppConfigDeployment_Tags (239.85s)
--- PASS: TestAccAWSAppConfigDeployment_basic (214.83s)

…verage, and update docs

github-actions · 2021-07-22T22:52:39Z

This functionality has been released in v3.51.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!

suzuki-shunsuke · 2021-07-22T23:14:13Z

Thank you for your support and detailed explanation!
I still concern about DeploymentCreatedTimeout.

https://github.com/hashicorp/terraform-provider-aws/pull/20172/files#diff-3bdbe64fedc07b5106e696f9ec76ff45114e10e91fce53afe2c99a2d60d6c568R10

const DeploymentCreatedTimeout = 20 * time.Minute

I'll test the case it takes over 20 minutes to complement the deployment.

resource "aws_appconfig_deployment_strategy" "test" {
  name                           = "test-2"
  deployment_duration_in_minutes = 25
  final_bake_time_in_minutes     = 25
  growth_factor                  = 1.0
  replicate_to                   = "NONE"
}

suzuki-shunsuke · 2021-07-22T23:41:27Z

#20172 (comment)

It failed to run terraform apply as expected.
I'll create an issue.

aws_appconfig_deployment.test: Still creating... [19m50s elapsed]
aws_appconfig_deployment.test: Still creating... [20m0s elapsed]
╷
│ Error: error waiting for AppConfig Deployment (xxx/xxx/2) creation: timeout while waiting for state to become 'COMPLETE' (last state: 'DEPLOYING', timeout: 20m0s)
│ 
│   with aws_appconfig_deployment.test,
│   on main.tf line 29, in resource "aws_appconfig_deployment" "test":
│   29: resource "aws_appconfig_deployment" "test" {
│ 
╵
terraform apply  4.35s user 2.20s system 0% cpu 20:13.84 total

suzuki-shunsuke · 2021-07-22T23:59:41Z

#20279

github-actions · 2021-08-22T02:05:16Z

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.

suzuki-shunsuke mentioned this pull request Jul 13, 2021

Add support for AWS AppConfig #11973

Closed

github-actions bot added the documentation Introduces or discusses updates to documentation. label Jul 14, 2021

github-actions bot added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 17, 2021

suzuki-shunsuke added 7 commits July 17, 2021 19:48

feat: add aws_appconfig_deployment

f76bf66

docs: add the document of aws_appconfig_deployment

903fce3

fix: fix semgrep error

5486776

AWS Go SDK pointer conversion function for `d.Set()` value is extraneous

feat: implement the update of AppConfig Deployment

2bc303f

fix: add the warning log when AppConfig Deployment resource is removed

c19dcf5

feat: wait for the creation of AppConfig Deployment

7f4b6bc

test: add tests of AppConfig Deployment

ba6c76b

github-actions bot added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Jul 17, 2021

docs: update the example of aws_appconfig_deployment

ab7433f

suzuki-shunsuke marked this pull request as ready for review July 17, 2021 11:15

Revert "feat: wait for the creation of AppConfig Deployment"

0834f15

This reverts commit 7f4b6bc.

anGie44 removed the needs-triage Waiting for first response or review from a maintainer. label Jul 19, 2021

anGie44 added the new-resource Introduces a new resource. label Jul 19, 2021

anGie44 self-assigned this Jul 19, 2021

docs: add CHANGELOG entry

8ff5e6e

update deployment resource to use importstatepassthrough, add test co…

b134b32

…verage, and update docs

anGie44 added this to the v3.51.0 milestone Jul 22, 2021

increase creation timeout

3e93b55

anGie44 merged commit c9985f4 into hashicorp:main Jul 22, 2021

suzuki-shunsuke mentioned this pull request Jul 22, 2021

It failed to create AppConfig Deployment if deployment_duration_in_minutes is over 20 #20279

Closed

suzuki-shunsuke deleted the feat/appconfig_deployment branch July 23, 2021 01:28

github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add aws_appconfig_deployment #20172

feat: add aws_appconfig_deployment #20172

suzuki-shunsuke commented Jul 13, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 13, 2021 •

edited

Loading

anGie44 commented Jul 15, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

anGie44 commented Jul 19, 2021

anGie44 commented Jul 22, 2021 •

edited

Loading

github-actions bot commented Jul 22, 2021

suzuki-shunsuke commented Jul 22, 2021

suzuki-shunsuke commented Jul 22, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 22, 2021

github-actions bot commented Aug 22, 2021

feat: add aws_appconfig_deployment #20172

feat: add aws_appconfig_deployment #20172

Conversation

suzuki-shunsuke commented Jul 13, 2021 • edited Loading

Community Note

suzuki-shunsuke commented Jul 13, 2021 • edited Loading

anGie44 commented Jul 15, 2021 • edited Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 • edited Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 • edited Loading

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021

suzuki-shunsuke commented Jul 17, 2021 • edited Loading

suzuki-shunsuke commented Jul 17, 2021 • edited Loading

anGie44 commented Jul 19, 2021

anGie44 commented Jul 22, 2021 • edited Loading

github-actions bot commented Jul 22, 2021

suzuki-shunsuke commented Jul 22, 2021

suzuki-shunsuke commented Jul 22, 2021 • edited Loading

suzuki-shunsuke commented Jul 22, 2021

github-actions bot commented Aug 22, 2021

suzuki-shunsuke commented Jul 13, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 13, 2021 •

edited

Loading

anGie44 commented Jul 15, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 17, 2021 •

edited

Loading

anGie44 commented Jul 22, 2021 •

edited

Loading

suzuki-shunsuke commented Jul 22, 2021 •

edited

Loading