Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Module to enable Data Lifecycle Manager to take snapshots of a EBS #197

Merged
merged 3 commits into from
Oct 17, 2019
Merged

Module to enable Data Lifecycle Manager to take snapshots of a EBS #197

merged 3 commits into from
Oct 17, 2019

Conversation

lpaulmp
Copy link
Contributor

@lpaulmp lpaulmp commented May 17, 2019 •
edited
Loading

Closes #196

Output
This example was deployed in a dev environment.

Screen Shot 2019-05-16 at 19 42 23

@lpaulmp lpaulmp self-assigned this May 17, 2019
@lpaulmp lpaulmp force-pushed the add-ebs-snapshots-policy branch from b404081 to e377a67 Compare June 10, 2019 23:02
ketzacoatl
ketzacoatl suggested changes Jul 5, 2019
View reviewed changes
Copy link
Contributor

@ketzacoatl ketzacoatl left a comment

Choose a reason for hiding this comment

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

@lpaulmp we'll want to parametize the name of the resources, eg include a name_prefix that is a required variable.

@ketzacoatl ketzacoatl mentioned this pull request Jul 5, 2019
Open
@lpaulmp lpaulmp force-pushed the add-ebs-snapshots-policy branch 2 times, most recently from 8c1449d to 3199a0f Compare July 15, 2019 01:47
@lpaulmp
Copy link
Contributor Author

lpaulmp commented Jul 15, 2019

@ketzacoatl I added the variable name_prefix to generate the schedule name of the DLM policy.

@lpaulmp lpaulmp force-pushed the add-ebs-snapshots-policy branch 2 times, most recently from 42649f0 to 7e600e7 Compare July 15, 2019 05:07
@lpaulmp lpaulmp force-pushed the add-ebs-snapshots-policy branch 2 times, most recently from d68066b to 06e1f1f Compare October 2, 2019 21:02
@lpaulmp
Copy link
Contributor Author

lpaulmp commented Oct 2, 2019 •
edited
Loading

@ketzacoatl This PR is ready to merge after upgrading the TF version (0.12) and adding types for the variables the modules creates the snapshots smoothly, I already tested with our infra.

Also, I made a relevant change for the PR moving the IAM role creation modules/dlm-lifecycle-iam-role as part another module with this change the DML policy is clean and does one thing, create the schedule for the snapshot.

@lpaulmp lpaulmp requested a review from ketzacoatl October 2, 2019 22:37
@lpaulmp lpaulmp force-pushed the add-ebs-snapshots-policy branch from 06e1f1f to fb2fc63 Compare October 2, 2019 22:48
ketzacoatl
ketzacoatl reviewed Oct 17, 2019
View reviewed changes
modules/dlm-lifecycle-iam-role/main.tf
]
}
EOF
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can merge this as-is, but we should update this code to use the data source so we can use HCL and don't have to do the messy JSON thing.

ketzacoatl
ketzacoatl reviewed Oct 17, 2019
View reviewed changes
modules/dlm-lifecycle-policy/README.md

```
module "ebs-backup-policy" {
source = "github.com/fpco/terraform-aws-foundation//modules/dlm-lifecycle-policy"
Copy link
Contributor

Choose a reason for hiding this comment

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

This example should source the module from the registry (we need a ticket for making sure all of our examples and docs do that - #268).

ketzacoatl
ketzacoatl approved these changes Oct 17, 2019
View reviewed changes
Copy link
Contributor

@ketzacoatl ketzacoatl left a comment

Choose a reason for hiding this comment

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

This LGTM!

@ketzacoatl ketzacoatl merged commit 8a214e5 into master Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ketzacoatl ketzacoatl ketzacoatl approved these changes

Assignees

@lpaulmp lpaulmp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create module to enable Data Lifecycle Manager to take snapshots of a EBS
2 participants
@lpaulmp @ketzacoatl