scottwinkler / terraform-provider-shell

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

Optional read not working, legacy plugin warnings #16

Closed jemc closed 4 years ago

jemc commented 4 years ago

First of all, thanks for building this. It's a great idea for a utility that should have been included in Terraform itself from the start.

However, I'm having issues with getting the documented behaviour of leaving out read in the lifecycle commands of my resource. That is, when I leave out read expecting to use the other commands to output the state, the state from those other commands is ignored. I have tried this both with and without the update command present, and it doesn't seem to make a difference.

I thought maybe I was doing something wrong on my end, but then I saw this warning in the terraform output:

2019/10/16 09:49:23 [WARN] Provider "shell" produced an unexpected new value for shell_script.test, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .lifecycle_commands[0].read: was null, but now cty.StringVal("")
      - .lifecycle_commands[0].update: was null, but now cty.StringVal("")

It seems maybe that due to changes in the plugin SDK, maybe optional lifecycle commands are not being handled correctly anymore. That is, maybe the plugin is expecting it to be missing (null), but terraform is coercing it to an empty string? Or vice versa?

My terraform version is Terraform v0.12.6. I didn't yet find any documentation (or CI/CD) showing which Terraform versions this plugin is supposed to support - didn't see it in the README, and CI/CD doesn't seem to be testing with terraform from what I could tell.

scottwinkler commented 4 years ago

Let me see if I understand this correctly. You have a shell_script resource with create and delete lifecycle_commands set and do not have read or update set. The problem you are having is with not being able to get state out from the resource.

Here is an example resource without read/update that sets state and outputs it with an output variable:

provider "shell" {}

//test resource with no read/update
resource "shell_script" "example" {
  lifecycle_commands {
    create = <<EOF
whoami=$(whoami)
echo '{"whoami": "'$whoami'"}' >> ex.json
cat ex.json >&3
EOF
    delete = "rm -rf ex.json"
  }
}

output "whoami" {
  value =shell_script.example.output["whoami"]
}

When i run this I get the following output:

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

whoami = swinkler

As far as your concern with the warnings for the plugin SDK, I can assure you these are nothing to worry about. It has nothing to do with compatibility with the latest version of Terraform, I am in fact running a slightly newer version than you, 0.12.9. This provider SDK was announced about a week ago and I have already merged the changes to accommodate this, but the error messages still persist, which tells me that the error messages are not exactly correct. Even the major cloud providers such as AWS/Azure/GCP all suffer from these annoying warning messages. It does not mean the provider is broken or that a breaking change is about to come that will break it.

You can find out more information on the Terraform Plugin SDK here: https://www.hashicorp.com/blog/announcing-the-terraform-plugin-sdk https://www.terraform.io/docs/extend/plugin-sdk.html

Let me know if you have any other questions or else I will close this issue

jemc commented 4 years ago

If you're using Terraform 0.12 and everything behaves correctly for your expectations, I'll just assume that it was my expectations that are wrong and I don't understand how it's supposed to work when read is not present.

Sorry that I didn't explain very well. The difference in my expectation comes in running terraform apply multiple times. When I run your example again, I get zero changes being executed:

$ terraform apply --auto-approve
shell_script.example: Creating...
shell_script.example: Creation complete after 0s [id=bmk857jk8gjofu0j7m4g]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

whoami = jemc
$ terraform apply --auto-approve
shell_script.example: Refreshing state... [id=bmk857jk8gjofu0j7m4g]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

whoami = jemc

I was expecting a resource with only create and delete commands set to run the create command every time, since there is no read command present to know whether the state has changed or not. I assumed that the bug was that it was using an empty command for the read, thus was taking the state outputted from the read to be empty, and thus the same every time, and that's why it only ran once.

However, it seems now that the behaviour is what you intended and not a bug - that I just had a misunderstanding about the intended/documented semantics.

jemc commented 4 years ago

I'm going to close the issue ticket, since it was just a misunderstanding, and the solution I ended up using included a read and update command anyway.

By the way, I name-dropped this provider from another terraform issue ticket here, and I included the approach I took to use this provider for orchestrating kubectl commands: https://github.com/terraform-providers/terraform-provider-kubernetes/issues/215#issuecomment-542831683

Thanks again for this great provider!

scottwinkler commented 4 years ago

Haha thanks! Your use case is exactly why I created this provider. I hate having to deal with buggy and unsupported Terraform providers. Sometimes it's easier just to run a few shell commands instead.

I just want to explain about why create works the way it does: I could have made it not require outputting state, but if you don't care about state then you probably would be fine using creation-time and destruction-time local-exec provisioners on a null_resource instead of this provider. Also, create/read/update/delete are basically just proxies to functionality stored in shell scripts. In real providers, the create still needs to return state (for example, even the null_resource sets an id in the create method). Sometimes people have create() call read(), but this is not done by default. Therefore, I favored consistently with how normal Terraform providers work even if it may be confusing when coming from the local-exec way of thinking. Again, the big difference between this provider and local-exec is that this provider cares about state. Hope this makes sense.