ability to change instance_state #2956
Conversation
agadelshin
force-pushed
the
instance_state
branch
from
9481035 to
e17c192
Compare
|
Should I do something to move this forward? |
| @@ -762,6 +770,15 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error | |||
| d.Set("can_ip_forward", instance.CanIpForward) | |||
| d.Set("machine_type", GetResourceNameFromSelfLink(instance.MachineType)) | |||
|
|
|||
| // PROVISIONING, STAGING, RUNNING, STOPPING, STOPPED, | |||
| // // SUSPENDING, SUSPENDED, and TERMINATED | |||
| switch instance.Status { | |||
There was a problem hiding this comment.
Instead of flattening several states into either RUNNING or STOPPED, let's see if we can approach this from a different direction.
Right now, we use an Operation from the GCP API to tell if the instance has been provisioned. That doesn't tell us the status of the logical instance, just of the GCP resource. What if we also waited in Create for the instance to be RUNNING if the field is set to RUNNING, and stop it if it's set to TERMINATED.
Then, in Read we can wait to enter a terminal value and d.Set the exact value.
Finally, in Update if we're in a different state than expected we need to start / stop depending on which terminal state we want to be in. There's some additional nuance around this section; https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_instance.go#L1253
We'll also need to ensure that TERMINATED instances can be deleted.
The intent of this change would be that we're explicitly entering a terminal state in each of the CRUD methods, so Terraform will always be in a well-known place, and we won't need to worry about how we can transition from PROVISIONING -> TERMINATED for example.
| @@ -964,6 +981,55 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err | |||
| d.SetPartial("tags") | |||
| } | |||
|
|
|||
| if d.HasChange("instance_state") { | |||
There was a problem hiding this comment.
Stated above, but we'll need to watch out for changes to the instance state in this method; https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_instance.go#L1253
| "Error starting an instance (%s): %s", d.Id(), err) | ||
| } | ||
|
|
||
| stateConf := &resource.StateChangeConf{ |
There was a problem hiding this comment.
We can probably extract this whole chunk into a function like waitForTerminalState and wait for either terminal status?
| ), | ||
| }, | ||
| { | ||
| Config: testAccComputeInstance_stopped(instanceName), |
There was a problem hiding this comment.
Good start! We'll need to test a handful of cases though.
- You've covered stopping an existing instance
- And deleting a stopped instance
- Updating a field on a stopped instance
- Also updating a field that requires stopping, like
machine_type - And doing that at the same time as stopping + starting for each of those
- Creating a stopped instance
There was a problem hiding this comment.
Thank you for useful suggestions and review. I've got you point, looks reasonable. I'm going to start with suggested test cases and will ask you for review!
fix state name Co-Authored-By: pondohva <[email protected]>
Co-Authored-By: pondohva <[email protected]>
|
Superseded with #4797 |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
this PR add a feature requested in #1719