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

Update support #1

Closed nhoughto closed 6 years ago

nhoughto commented 6 years ago

Your project seems to solve some of the problems i'm having with using local-exec to shell out to get stuff done that terraform doesn't have providers etc for. So thanks!

One use case we have is for a change to a script resource to not trigger a destroy and create but an update, as destruction and creation is expensive, but updates are cheap and simple (shelling out to a process that already supports idempotent updates). It would be great to be able to apply updates and avoid the destroy/create cycle.

Happy to work on a PR, got any tips?

scottwinkler commented 6 years ago

@nhoughto hi, thanks for your interest in this project

Yes, I had thought about enabling update instead of just create and destroy. Technically, it would not be difficult to do. We just need to do a diff on the environment variables, and if there is a change, then trigger the update script. The update script must receive input for the previous state as well as the new state. Additionally, if an update script is not provided, it should default to doing a destroy and then an apply. If you want to take a stab at this, you are more than welcome to, otherwise I'll look into it this weekend.

nhoughto commented 6 years ago

Happy to wait =)

Would have to skill up on writing terraform providers to be of any use, so happy to be patient 😉

Have you had any gotchas with using this component in anger? We are currently tossing up between using an escape hatch like this to access terraform lifecycle and fire scripts, or writing a provider directly to do what we want (which seems less flexible than arbitrary scripts).

Thanks either way 👍🏼 On 7 Sep 2018, 5:35 AM +1000, scottwinkler notifications@github.com, wrote:

@nhoughto hi, thanks for your interest in this project Yes, I had thought about enabling update instead of just create and destroy. Technically, it would not be difficult to do. We just need to do a diff on the environment variables, and if there is a change, then trigger the update script. The update script must receive input for the previous state as well as the new state. Additionally, if an update script is not provided, it should default to doing a destroy and then an apply. If you want to take a stab at this, you are more than welcome to, otherwise I'll look into it this weekend. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

scottwinkler commented 6 years ago

If you want to greatest amount of customization, I would recommend writing your own provider. Writing your own provider however requires that you have access to an sdk written in golang, which is not always available. I am not sure if I have any gotchas except to say that using this provider for complex resources is probably just as difficult, if not moreso, compared to writing a resource in a proper provider. For simple resources it is adequate.

nhoughto commented 6 years ago

Good info, I’ll try it out and see how it goes.

Our current approach is local-exec with triggers, which is awkward and fragile with no connection / ability to lookup remote state. So either is a step forward I think. On 7 Sep 2018, 7:35 AM +1000, scottwinkler notifications@github.com, wrote:

If you want to greatest amount of customization, I would recommend writing your own provider. Writing your own provider however requires that you have access to an sdk written in golang, which is not always available. I am not sure if I have any gotchas except to say that using this provider for complex resources is probably just as difficult, if not moreso, compared to writing a resource in a proper provider. For simple resources it is adequate. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

nhoughto commented 6 years ago

More specifically about the update process, my assumption was it would go thru the following steps:

Most of this seems like terraform core, so you expose read, create, destroy and update primitive and they get call appropriately to fulfil the above steps? or am I missing something 🤔

scottwinkler commented 6 years ago

Close, the actual process is 1) if the resource exists in the state file, perform the read operation to update the state file. You must specify a script for how this read occurs, and the output of this command becomes the new state. Currently I do not use this feature in my own code, because it would only matter if you expect some sort of configuration drift due to people changing things manually which they shouldn't be touching. How I handle this normally is simply returning the result of whatever my old state was, which is lazy but works fine for most applications. 2) Perform a diff on the inputs to the resource. If there is a change in any of the inputs, you specify in the resource code how you want the update to occur. Really the only change that matters is for environment variables, because if the scripts change, you're pulling the carpet out from under you and you're basically screwed anyways. Since we are using a shell script to handle this, the shell script performing the update needed to receive the old inputs as well as the new inputs, and the current state. Then you specify in your shell script logic whatever you need to do, and the output from that update will be the new state 3) local state is compared to remote state, but the only change that matters is changes to the inputs. Any output variables are simply overridden with the latest state.

The last step that you mentioned, "trigger action" is not handled by the terraform core binary because it is unique to the resource being updated. That is one of the reasons why update is harder to implement for a shell provider like this, but it is doable for sure.

nhoughto commented 6 years ago

Ah ok, that explains the behaviour i'm seeing, have compiled master branch, testing on macOS

resource "shell_script" "test" {
  lifecycle_commands {
    read = <<EOF
kubectl get pods -o jsonpath="{..image}"
EOF
    create = <<EOF
kubectl apply -f pod.yml > /dev/null
# crappy way to wait for pod to come up
sleep 5 
kubectl get pods -o jsonpath="{..image}"
EOF
    delete = <<EOF
kubectl delete --ignore-not-found -f pod.yml
EOF
  }
  working_directory = "."
}

What you described in 1 is what I am testing, I want to be able to mutate remote state (cause drift) and have the read pickup that that has happened and trigger a create / destroy (or ideally update). This is the behaviour most(?) providers exhibit, for example if I change an AWS resource terraform will know and fix it.

Is this read remote state behaviour a goal of this component? If not maybe i'm barking up the wrong tree 🤔 My original assumption was shelling out was a really convenient way of reading remote state without having to write it all in golang..

scottwinkler commented 6 years ago

Yes, what you are describing occurs in the read step. In this particular case, since you are concerned with the number of pods, i would assume that when you do a "terraform apply" you would want to rerun the "kubectl apply" part if the number of pods is not what it it should be. Since this isn't really an update as the environment variables/ other inputs are not changing you can actually handle this case fairly easily

read = <<EOF
LOCAL_STATE=$(cat)  # the output of create is the input for read
REMOTE_STATE=$(kubectl get pods -o jsonpath="{..image}") 
/*pseudo code 
#do we have zero pods?
 if (len(REMOTE_STATE)==0){ 
 kubectl apply -f pod.yml > /dev/null
sleep 5 
kubectl get pods -o jsonpath="{..image}"
} 
# else do we have some weird state that we don't know how else to recover from?
else if LOCAL_STATE!=REMOTE_STATE{
kubectl delete --ignore-not-found -f pod.yml
 kubectl apply -f pod.yml > /dev/null
sleep 5 
kubectl get pods -o jsonpath="{..image}"
}
# else if everything as we would expect? 
else {
 echo $LOCAL_STATE
}
*/
EOF

I would also suggest you take a look at my other repo: https://github.com/scottwinkler/terraform-provider-tfe where I have some more examples of using this shell provider. Let me know if you have any other questions

nhoughto commented 6 years ago

Excellent, will do thanks :+1:

nhoughto commented 6 years ago

Wait, so the idea is that you would be running a kubectl apply in the read command? Isn't that dangerous / unexpected? That was why I assumed you would want that to trigger an update, as that is where things are expected to be mutated after consenting "yes" to apply. If you do it in the read command then it will apply before consent, in the refresh phase no?

Also, thanks for the help! =)

scottwinkler commented 6 years ago

If you wrote a custom provider you could do anything you want in the read command as well, so in that regards its not as dangerous/unexpected as you might expect. Also in this case, it is the proper thing to put this in the read, update would not be called because as the inputs to the resource are not changing.

nhoughto commented 6 years ago

I ended up spending a bunch of time on this and added update support based on remote state differences. I’m not sure it aligns with your project anymore tho which is focused more on local changes. Happy to PR if you like. On 7 Sep 2018, 10:13 AM +1000, scottwinkler notifications@github.com, wrote:

If you wrote a custom provider you could do anything you want in the read command as well, so in that regards its not as dangerous/unexpected as you might expect. Also in this case, it is the proper thing to put this in the read, update would not be called because as the inputs to the resource are not changing. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

scottwinkler commented 6 years ago

@nhoughto, on the contrary, I would be happy to review your pull request and merge it. Its something I wanted to implement myself, but I did not get around to it.

scottwinkler commented 6 years ago

resolved. thanks for all your input! 😃