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

Fix for EBS volumes created when the instance has been disabled, plus some vars description improvement #102

Merged
merged 7 commits into from Jun 14, 2021

Conversation

nnsense
Copy link
Contributor

@nnsense nnsense commented Jun 14, 2021

  • Fixing EBS volumes will be created if enabled = false #74 (EBS volumes will be created if enabled = false)
  • Fixing a wrong description provided for ebs_volume_encrypted in variables.tf
  • Changing some EBS related description to be clear those are "additional volumes"

what

Adding a local variable and a && to the creation of the EBS volumes we can avoid the creation of the additional volumes if the instance creation has been disabled.

references

- Ficing a wrong description provided for ebs_volume_encrypted in variables.tf
- Changing some EBS related description to be clear those are "additional volumes"
@nnsense nnsense requested review from a team as code owners June 14, 2021 12:50
@nnsense nnsense requested review from adamcrews and 3h4x June 14, 2021 12:50
Copy link
Contributor

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

LGTM, but I requested a minor change.

main.tf Outdated Show resolved Hide resolved
Co-authored-by: Yonatan Koren <me@yonatankoren.com>
@mergify mergify bot dismissed korenyoni’s stale review June 14, 2021 14:13

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

@korenyoni
Copy link
Contributor

/test all

Copy link
Contributor

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

Sorry to be a nag — I just realized we should drop the s in local.volumes_count to be more consistent with var.ebs_volume_count

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
Co-authored-by: Yonatan Koren <me@yonatankoren.com>
@mergify mergify bot dismissed korenyoni’s stale review June 14, 2021 16:25

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

nnsense and others added 3 commits June 14, 2021 17:25
Co-authored-by: Yonatan Koren <me@yonatankoren.com>
Co-authored-by: Yonatan Koren <me@yonatankoren.com>
@korenyoni
Copy link
Contributor

/test all

@korenyoni korenyoni merged commit 1fc9595 into cloudposse:master Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EBS volumes will be created if enabled = false
3 participants