Enhance README to take advantage of `terraform.tf`

[jmcgeheeiv]

what

• Add terraform.tf to the usage instructions

why

• The current usage does not account for terraform.tf, which is actually quite brilliant functionality

references

• Personal experience using this over the last few months

[jmcgeheeiv]

I added a destroy procedure. Using this module this way works very smoothly. Very nice architecture!

[schollii]

I was just thinking the other day of doing something like this, I didn't know such functionality was already in place just not documented!

However I'm wondering since it is so far undocumented can we make the following changes before this goes "public":

• rename those 3 input variables, they are unnecessarily verbose and clash with rest. Given the context of this module, it would be sufficient to call them just backend_file_path etc.
• the path to the generated file should probably default to root module path, ie almost always the module that ultimately needs a backend. So the count 0/1 test should be whether the filename has been defined, not whether the path has been defined.
• I think you have to empty bucket after state moved locally, before terraform destroy, otherwise you get an error.

I would be happy to submit some PR for same changes in other repos if that helps, I don't remember seeing any of them advertise this approach either.

[osterman]

This is an interesting approach!

[schollii]

pretty sure you will have to set force_destroy to true before doing the final apply since the bucket is versioned

[jmcgeheeiv]

Yes, I encountered that last time I did terraform destroy. I'm also reconsidering whether the backend file should be added to .gitignore, or whether it should be committed to Git. I want to try the workflow a few more times, but right now I'm away from this part of the project now. Still, the basic workflow is quite good. It is CloudPosse that deserves credit for that, @osterman.

[osterman]

Sorry we haven’t been able to review this yet. We’ve been slammed with new projects.

Has anyone else had a chance to test this out?

[jmcgeheeiv]

No rush, @osterman. In fact, I will change this to WIP because I still want to check into the issues I mentioned in my last comment.

[schollii]

@jmcgeheeiv even though the file is auto-generated I think it should be committed: if there is a change in the tf state module, or you make a change in input values, that you didn't realize would affect it, you will be able to see the change with git diff. Whereas without having committed it, you will only see what the plan tells you has changed, there will be no source to check for diff to help understand cause of change.

Also is there any disadvantage? If a timestamp or other attribute in the generated file changed at every apply, that would be a good reason to gitignore it, but that's not the case.

[jmcgeheeiv]

@schollii, in my most recent pass through the workflow I have made these changes:

1. Specify file name ./backend.tf. terraform.tf is not descriptive.
2. Commit ./backend.tf to git
3. Do something more forceful to remove the state bucket. @schollii, could you help by describing the exact command you recommend?

Shall I implement these changes in the documentation?

[schollii]

I gather this will work on v0.16.0? Pretty sure I had issues with 0.17.0 (#47). It seems like it's only the docs that are changing. I believe the workflow is this:

1. create backend-s3-bootstrap.tf
2. terraform init
3. terraform apply: creates backend.tf and local state
4. terraform init: moves local state to s3
5. terraform apply: creates other state if added, so this is optional if no additional resources created
6. ...
7. remove backend.tf
8. terraform init: copy state from s3 to local
9. edit bucket to have force_destroy=true
10. terraform apply: updates only bucket; I'm not 100% this is necessary
11. terraform destroy

However I cannot verify that right now, but if you don't do that, then during terraform destroy you will get an error that the bucket has contents. After I deleted manually from AWS console a couple weeks ago, the destroy worked.

[schollii]

@jmcgeheeiv I have verified the workflow and documented at https://github.com/cloudposse/terraform-aws-tfstate-backend/issues/44#issuecomment-640954116 since PR's are harder to find. HTH.

[Gowiem]

@jmcgeheeiv Definitely like the idea of getting this merged. I personally didn't know about this functionality and it sounds much preferred! Weighing in on a couple things:

1. backend.tf > terraform.tf 👍
2. Committing backend.tf to git -- I only see negatives in not doing this. I think this is the right call.

Happy to get this PR approved, merged, and cut for you as soon as you and @schollii suss out the final changes you want to make to the README. Thanks gents!

[jmcgeheeiv]

Modified to match @schollii's procedure in #44 without question--because he seems like such a trustworthy chap.

@schollii, @Gowiem, please scrutinize.

[jmcgeheeiv]

Today I had an opportunity to destroy a deployment, and I found an error in my destroy procedure (wrong value for force_destroy).

I also added @schollii as a contributor. I hope you do not mind. If this bothers you, I can remove you from my fork and nobody will be the wiser.

[osterman]

/rebuild-readme

[Gowiem]

@jmcgeheeiv Sorry to do it to ya, but it looks like there are some merge conflicts. Mind fixing and we'll get this merged?

[schollii]

@aknysh markdown doesn't require sequence of numbers just first one must be '1'. See https://www.markdownguide.org/basic-syntax/#ordered-lists.

However the readme is understandable without rendering if we use sequence of numbers. For small lists it's manageable, so I'm in favour of fixing.

[RothAndrew]

IMO 1s are preferable, since you dont need to reorder them if you add an item in the middle of the list

[aknysh]

IMO 1s are preferable, since you dont need to reorder them if you add an item in the middle of the list

ah yes, the user will see it as an unordered list, it's just in the code itself we have 1.

[RothAndrew]

They will see it as an ordered list

[aknysh]

They will see it as an ordered list

[Gowiem]

@aknysh Did you mean to close this? I would think we would still want to get this merged. Mind if I reopen?

[aknysh]

@aknysh Did you mean to close this? I would think we would still want to get this merged. Mind if I reopen?

oh sorry, clicked the wrong button, reopening

[jmcgeheeiv]

The GitHub style guide specifies that 1. be used

[Gowiem]

/test all

[Gowiem]

@jmcgeheeiv @schollii Thanks for the contributions and wading through the updates! This is just a docs update, but still... Released as 0.18.1.