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

feat: add cloudwatch output config support for run command tasks #14302

Closed
wants to merge 10 commits into from

Conversation

mycodeself
Copy link

@mycodeself mycodeself commented Jul 22, 2020

I have added support for output the logs of run command in a maintenance window tasks to cloudwatch. I've done this based on the PR #12167 that it's inactive for some time. I simply split the tests as proposed in the other PR.

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 OR Closes #14136
Other PR Related #12167

Release note for CHANGELOG:

Add support for CloudWatch output configuration in run command maintenance window tasks 

Output from acceptance testing:

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

@ghost ghost added size/M Managed by automation to categorize the size of a PR. service/ssm Issues and PRs that pertain to the ssm service. needs-triage Waiting for first response or review from a maintainer. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Jul 22, 2020
@mycodeself mycodeself marked this pull request as ready for review July 24, 2020 17:26
@mycodeself mycodeself requested a review from a team July 24, 2020 17:26
@tzvifriedman
Copy link

Just checking in on this PR. It would be really great if we could get this in place, I'm kind of depending on it for SSM patch compliance reporting. Thanks!

@mycodeself
Copy link
Author

mycodeself commented Oct 26, 2020

Just checking in on this PR. It would be really great if we could get this in place, I'm kind of depending on it for SSM patch compliance reporting. Thanks!

I have fixed the existing conflicts, I am still waiting for a review by a maintainer in order to merge the PR.

I hope someone will give me feedback soon

Edit: Seems something goes wrong, some checks has failed, I'm going to take a look on it deeper later

@ghost ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Oct 26, 2020
@ghost ghost added size/M 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 Oct 26, 2020
@mycodeself
Copy link
Author

I don't know exactly what is happening with the checks, when I launch terrafmt locally I get many errors not related to my PR.

When I have some time I will give another look, if someone wants to contribute to my PR welcome

@tzvifriedman
Copy link

was wondering if you ever got any time to take another swing at this. I probably don't know enough go to make a meaningful contribution but this would be an amazing feature to get active!

@tlaukkan
Copy link

This would be really good to have.

@mycodeself mycodeself requested a review from a team as a code owner January 14, 2021 19:49
@mycodeself
Copy link
Author

Finally I have had some time and I think I have managed to fix all the issues 🤞

I hope that someone can review it soon :)

@tzvifriedman
Copy link

tzvifriedman commented Jan 14, 2021

Thanks so much for coming back to this!

Base automatically changed from master to main January 23, 2021 00:58
@bflad
Copy link
Contributor

bflad commented Feb 12, 2021

Hi @mycodeself 👋 Thank you for working on this. This functionality is being merged in from the earlier #11774 and will be available in version 3.28.0 of the Terraform AWS Provider releasing later today.

@bflad bflad closed this Feb 12, 2021
@ghost
Copy link

ghost commented Mar 14, 2021

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 as resolved and limited conversation to collaborators Mar 14, 2021
@breathingdust breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/ssm Issues and PRs that pertain to the ssm service. size/M 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
Development

Successfully merging this pull request may close these issues.

Allow output logs to CloudWatch for aws_ssm_maintenance_window_task resource
5 participants