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.

Sign up for GitHub

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

Jump to bottom

Feature/add support for deprecate aws ssm maintenance window task #7823

Merged
merged 13 commits into from
Jul 16, 2019
Merged

Feature/add support for deprecate aws ssm maintenance window task #7823

merged 13 commits into from
Jul 16, 2019

Conversation

teraken0509
Copy link
Contributor

@teraken0509 teraken0509 commented Mar 6, 2019 •
edited by bflad
Loading

Closes #7870
Closes #7746
Closes #7154
Closes #6431
Closes #5045

Reference #5700
Reference #4408
Reference #3218

Changes proposed in this pull request:

  • Fix to use GetMaintenanceWindowTask method when read resource
  • Add Support Update resource
  • Fix to not return error when resource is already deleted
  • Add support import exist resource
  • Add attribute task_invocation_parameters
  • Add deprecated message for logging_info and task_parameters
  • Add test case for automation_parameters
  • Add test case for lambda_parameters
  • Add test case for run_command_parameters
  • Add test case for step_functions_parameters
  • Update document

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMMaintenanceWindowTask_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSSMMaintenanceWindowTask_ -timeout 120m
=== RUN   TestAccAWSSSMMaintenanceWindowTask_basic
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_basic
=== RUN   TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_basic
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters (147.07s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters (163.38s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters (169.52s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (173.94s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters (204.06s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (232.43s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	232.506s

@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/ssm Issues and PRs that pertain to the ssm service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 6, 2019
@teraken0509 teraken0509 force-pushed the feature/add-support-for-deprecate-aws_ssm_maintenance_window_task branch 2 times, most recently from 08fe093 to e0ab932 Compare March 7, 2019 09:07
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Mar 7, 2019
@teraken0509 teraken0509 changed the title [WIP] Feature/add support for deprecate aws ssm maintenance window task Feature/add support for deprecate aws ssm maintenance window task Mar 7, 2019
@teraken0509 teraken0509 marked this pull request as ready for review March 7, 2019 09:08
@e-moshaya
Copy link

any update on this PR when will this get merged?

@aeschright aeschright requested a review from a team June 26, 2019 00:46
@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 8, 2019
bflad
bflad suggested changes Jul 8, 2019
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Hi @kterada0509 👋 Thanks for submitting this and sorry for the delayed review. For future reference, the maintainers can more easily review and get changes merged in if they are incrementally pull requested (e.g. the first 3 commits for update and import support vs the last large one with deprecations).

Please reach out if you have any questions or do not have time to implement the feedback items. 😄

aws/resource_aws_ssm_maintenance_window_task.go Show resolved Hide resolved
aws/resource_aws_ssm_maintenance_window_task.go Show resolved Hide resolved
aws/resource_aws_ssm_maintenance_window_task.go Show resolved Hide resolved
aws/resource_aws_ssm_maintenance_window_task.go Show resolved Hide resolved
aws/resource_aws_ssm_maintenance_window_task.go Show resolved Hide resolved
aws/resource_aws_ssm_maintenance_window_task_test.go Outdated Show resolved Hide resolved
website/docs/r/ssm_maintenance_window_task.html.markdown Outdated Show resolved Hide resolved
website/docs/r/ssm_maintenance_window_task.html.markdown Outdated Show resolved Hide resolved
website/docs/r/ssm_maintenance_window_task.html.markdown Outdated Show resolved Hide resolved
website/docs/r/ssm_maintenance_window_task.html.markdown Outdated Show resolved Hide resolved
@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 8, 2019
@bflad bflad self-assigned this Jul 8, 2019
@AndrewCi
Copy link

Will this address #2876?

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 10, 2019
@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 11, 2019
@bflad bflad mentioned this pull request Jul 11, 2019
Closed
@teraken0509 teraken0509 force-pushed the feature/add-support-for-deprecate-aws_ssm_maintenance_window_task branch from e0ab932 to b36e54b Compare July 12, 2019 09:50
@teraken0509
Copy link
Contributor Author

Re-run acctest

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMMaintenanceWindowTask_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSSMMaintenanceWindowTask_ -timeout 120m
=== RUN   TestAccAWSSSMMaintenanceWindowTask_basic
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_basic
=== RUN   TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_basic
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters (148.07s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters (151.84s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters (183.01s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (188.49s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters (194.87s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (214.83s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	214.939s

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 12, 2019
@bflad
Copy link
Contributor

bflad commented Jul 12, 2019

Hi again @kterada0509 👋 Thank you for those updates. Did you see the feedback that the GitHub user interface hid behind the "19 hidden conversations" link?

Screen Shot 2019-07-12 at 3 39 10 PM

It looks like some or many of them are still pending as well. You can also see all review feedback by clicking the link there or going to the Files tab of the pull request. Please let us know. Thanks.

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 12, 2019
@teraken0509
Copy link
Contributor Author

Re-run acctest.

ssm_maintenance_window_task *)$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMMaintenanceWindowTask_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSSSMMaintenanceWindowTask_ -timeout 120m
=== RUN   TestAccAWSSSMMaintenanceWindowTask_basic
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_basic
=== RUN   TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== RUN   TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_basic
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters
=== CONT  TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters (163.76s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters (186.35s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (196.74s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (210.43s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters (229.00s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters (234.95s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	235.059s

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 14, 2019
@bflad bflad added this to the v2.20.0 milestone Jul 16, 2019
bflad
bflad approved these changes Jul 16, 2019
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for sticking through this one, @kterada0509 🚀 We may followup with some additional testing to close out some additional open issues relating to this resource.

Output from acceptance testing:

--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters (116.69s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters (117.03s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters (119.36s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters (120.27s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (227.62s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (236.55s)

@bflad bflad merged commit 7070855 into hashicorp:master Jul 16, 2019
bflad added a commit that referenced this pull request Jul 16, 2019
@bflad
Update CHANGELOG for #7823
6d6d4c1
@teraken0509 teraken0509 deleted the feature/add-support-for-deprecate-aws_ssm_maintenance_window_task branch July 17, 2019 05:39
@ghost
Copy link

ghost commented Nov 2, 2019

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad approved these changes

Assignees

@bflad bflad

Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ssm Issues and PRs that pertain to the ssm service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v2.20.0
4 participants
@teraken0509 @e-moshaya @AndrewCi @bflad