search

scottwinkler / terraform-provider-shell

Terraform provider for executing shell commands and saving output to state file
Mozilla Public License 2.0
279 stars 60 forks source link

Should not use cached lifecycle commands for read or otherwise #59

Closed mrzor closed 4 years ago

mrzor commented 4 years ago

Greetings!

I believe I stumbled upon a bug. I changed my lifecycle commands on a shell_script resource (was moving the script around), and couldn't even plan after that because the read would fail - I had to terraform state rm the resource to get going again.

If this is intended behavior (I can't see a reason - which doesn't mean there isn't one), then maybe it should behave in a "delete+create (recreate)" - but this feels like a tough thing to do when read fails.

scottwinkler commented 4 years ago

i understand the frustration, as I have experienced it a lot myself. the original reason for doing it, is that any change you make to the create/read/update/delete scripts fundamentally changes the behavior/state of the resource. It's as strict as it is in the hope that you would test resources thoroughly before using in production. That being said, it no longer appears to be useful. In fact, It appears I have already removed in from delete and update some time ago, because of the headache it caused.

I think removing ForceNew from read and create to be consistent is a good idea. Would you like to make a PR for this? it's an easy fix.

mrzor commented 4 years ago

any change you make to the create/read/update/delete scripts fundamentally changes the behavior/state of the resource.

That's unlikely isn't it ? I can only speak for myself on this, but my resource block was always about the same thing - what changed was the way of handling it.

It's as strict as it is in the hope that you would test resources thoroughly before using in production.

OTOH, I spend most of my time writing Terraform config for already existing infrastructure - that has to import cleanly, and for which delete+create is highly undesirable.

Allright - I'm going to give this a shot. I have zero+epsilon experience with Go though. Edit: That was indeed extremely easy. No need to understand Go required :smile:

More involved (and more enlightening) would be to add a test for it - but tbh I don't see value in such tests (we would be testing schema.Resource behavior more than ours - a bit pointless imo).