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 cloudwatch logging options for runCommand patch manager tasks #48

Merged
merged 1 commit into from
Mar 11, 2024
Merged

Add cloudwatch logging options for runCommand patch manager tasks #48

merged 1 commit into from
Mar 11, 2024

Conversation

gpapakyriakopoulos
Copy link
Contributor

@gpapakyriakopoulos gpapakyriakopoulos commented Mar 8, 2024

Based on AWS documentation [1] runCommand executions support directly logging to cloudwatch instead of an S3 bucket.

To that end we introduce 3 extra module variables, the first 2 (cloudwatch_log_output_enabled & cloudwatch_log_group_name) to configure whether cloudwatch logging is enabled and whether a custom cloudwatch log group name should be provided (if not a default is created, see [2]). The 3rd variable (s3_log_output_enabled) is introduced to optionally disable supplying an S3 bucket for logging, since cloudwatch logging should be enough for some use cases.

[1] https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-rc-setting-up-cwlogs.html
[2] https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_maintenance_window_task#cloudwatch_log_group_name

@gberenice
Copy link

/terratest

1 similar comment
@gberenice
Copy link

/terratest

@gpapakyriakopoulos
Copy link
Contributor Author

I was not able to update the readme as the make command failed for me on MacOS. If you would be so kind as to generate the README files correctly yourselves and update the PR I would appreciate it

@gberenice
Copy link

I was not able to update the readme as the make command failed for me on MacOS. If you would be so kind as to generate the README files correctly yourselves and update the PR I would appreciate it

@gpapakyriakopoulos thanks for your contribution! The issue with README generation will be fixed today, I'll re-run tests when it's done.

@gberenice
Copy link

/terratest

@gberenice
Copy link

@gpapakyriakopoulos could you please run again the following commands locally?

  make init
  make readme

I believe this should work now 👍

Copy link

mergify bot commented Mar 10, 2024

Thanks @gpapakyriakopoulos for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added triage Needs triage needs-test Needs testing labels Mar 10, 2024
@gpapakyriakopoulos
Copy link
Contributor Author

@gpapakyriakopoulos could you please run again the following commands locally?

  make init
  make readme

I believe this should work now 👍

Seemed to worked correctly this time! I've added the README update in the PR, thanks!

@gberenice
Copy link

/terratest

@gberenice gberenice enabled auto-merge (squash) March 10, 2024 17:01
gberenice
gberenice previously approved these changes Mar 10, 2024
Copy link

@gberenice gberenice left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@mergify mergify bot removed the triage Needs triage label Mar 10, 2024
@gberenice
Copy link

@gpapakyriakopoulos mind syncing your fork with the base branch?

Based on AWS documentation [1] runCommand executions support directly
logging to cloudwatch instead of an S3 bucket.

To that end we introduce 3 extra module variables, the first 2 (`cloudwatch_log_output_enabled` & `cloudwatch_log_group_name`)
to configure whether cloudwatch logging is enabled and whether a custom cloudwatch log group
name should be provided (if not a default is created, see [2]). The 3rd variable (`s3_log_output_enabled`) is
introduced to optionally disable supplying an S3 bucket for logging, since cloudwatch logging should be enough
for some use cases.

[1] https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-rc-setting-up-cwlogs.html
[2] https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_maintenance_window_task#cloudwatch_log_group_name
auto-merge was automatically disabled March 11, 2024 07:50

Head branch was pushed to by a user without write access

@mergify mergify bot dismissed gberenice’s stale review March 11, 2024 07:51

This Pull Request has been updated, so we're dismissing all reviews.

@gpapakyriakopoulos
Copy link
Contributor Author

Just rebased using the GitHub button @gberenice. Seems like you need to run terratest again + add your review again.

@gberenice gberenice enabled auto-merge (squash) March 11, 2024 08:04
@gberenice
Copy link

/terratest

@gberenice gberenice merged commit 23d1a1f into cloudposse:main Mar 11, 2024
11 checks passed
@gberenice
Copy link

A new version v1.0.1 was released. Thanks @gpapakyriakopoulos !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-test Needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants