New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google compute] Add node stop/start support #442

Closed
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
2 participants
@erjohnso
Member

erjohnso commented Jan 30, 2015

GCE now supports node (instance) stop/start. This PR adds the two new driver methods and supporting tests to include these new features. Note that these methods only return a boolean, so the node object must be queried again to get the new status. For example,

>>> node = gce_driver.ex_get_node('foo', 'us-central1-f')
>>> print node.extra['status']
u'RUNNING'
>>> gce_driver.stop_node(node)
>>> node = gce_driver.ex_get_node('foo', 'us-central1-f')
>>> print node.extra['status']
u'TERMINATED'

Note that the correct state for a stopped node is TERMINATED.

Upstream docs are at:
https://cloud.google.com/compute/docs/instances#stopping_an_instance
https://cloud.google.com/compute/docs/instances#restarting_a_stopped_instance

/cc @Kami - I'd like to get this into the next release if you're OK with that.

@asfgit asfgit closed this in 5395ccf Feb 2, 2015

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 2, 2015

Member

Sorry for a late comment.

Currently both "terminated" and "stopped" states map to Libcloud "TERMINATED" state. Does GCE internally use the same value for a terminated / destroy node (node.destroy) and stopped node (ex_stop_node)?

If it doesn't, we should map "stopped" to Libcloud.STOPPED state, otherwise it's rather painful for our users - usually, actions you can perform on a server different depending if the node is destroyed or stopped (e.g. usually you can don't anything with a terminated node, but you can start a stopped node).

Member

Kami commented Feb 2, 2015

Sorry for a late comment.

Currently both "terminated" and "stopped" states map to Libcloud "TERMINATED" state. Does GCE internally use the same value for a terminated / destroy node (node.destroy) and stopped node (ex_stop_node)?

If it doesn't, we should map "stopped" to Libcloud.STOPPED state, otherwise it's rather painful for our users - usually, actions you can perform on a server different depending if the node is destroyed or stopped (e.g. usually you can don't anything with a terminated node, but you can start a stopped node).

@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 2, 2015

Member

No worries, sorry for not waiting longer. I was anxious to get this in on the off chance you decided to cut a new release.

GCE uses "TERMINATED" when the node is stopped. On node.destroy(), the node is removed from the system and any subsequent requests to it would result in a ResourceNotFoundError. If the node exists and is 'started', it is in the RUNNING state. Since stop/start are async_requests, the user won't see any other intermediate states, but the flow works like this:

If it's running and we 'stop' it: RUNNING->STOPPING->TERMINATED
If stopped and we start it: TERMINATED->PROVISIONING->RUNNING

Based on that, are there any changes I should make?

Member

erjohnso commented Feb 2, 2015

No worries, sorry for not waiting longer. I was anxious to get this in on the off chance you decided to cut a new release.

GCE uses "TERMINATED" when the node is stopped. On node.destroy(), the node is removed from the system and any subsequent requests to it would result in a ResourceNotFoundError. If the node exists and is 'started', it is in the RUNNING state. Since stop/start are async_requests, the user won't see any other intermediate states, but the flow works like this:

If it's running and we 'stop' it: RUNNING->STOPPING->TERMINATED
If stopped and we start it: TERMINATED->PROVISIONING->RUNNING

Based on that, are there any changes I should make?

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 2, 2015

Member

Ah, OK, thanks for the clarification.

In this case it sounds like we can and should just map TERMINATED -> STOPPED. This way it's consistent and works consistently across different providers. It would also be good to add a comment to the code so people know why terminated is mapped to stopped.

A lot of Libcloud users dynamically build actions which are available for a particular node based on the node state and other information (are there any volumes / ip addresses attached, etc.). Other providers usually don't immediately delete node from a list when destroying a server, but the simply change state to TERMINATED and when a node is in this state no action can be performed on it.

Member

Kami commented Feb 2, 2015

Ah, OK, thanks for the clarification.

In this case it sounds like we can and should just map TERMINATED -> STOPPED. This way it's consistent and works consistently across different providers. It would also be good to add a comment to the code so people know why terminated is mapped to stopped.

A lot of Libcloud users dynamically build actions which are available for a particular node based on the node state and other information (are there any volumes / ip addresses attached, etc.). Other providers usually don't immediately delete node from a list when destroying a server, but the simply change state to TERMINATED and when a node is in this state no action can be performed on it.

@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 2, 2015

Member

Ok, here's how it stands right now for the GCE driver (lines 793-800)[1]. Note that it's been this way for a while. I did not introduce this mapping in #442.

    NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPED": NodeState.TERMINATED,
        "STOPPING": NodeState.TERMINATED,
        "TERMINATED": NodeState.TERMINATED
    }

I think what you're proposing is to change it to:

    NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPING": NodeState.STOPPED,
        "TERMINATED": NodeState.STOPPED
    }

Note, that I removed STOPPED since we don't (no longer?) have that state[2]. Does this look correct?

[1] https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/gce.py#L793-L800
[2] https://cloud.google.com/compute/docs/instances#checkmachinestatus

Member

erjohnso commented Feb 2, 2015

Ok, here's how it stands right now for the GCE driver (lines 793-800)[1]. Note that it's been this way for a while. I did not introduce this mapping in #442.

    NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPED": NodeState.TERMINATED,
        "STOPPING": NodeState.TERMINATED,
        "TERMINATED": NodeState.TERMINATED
    }

I think what you're proposing is to change it to:

    NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPING": NodeState.STOPPED,
        "TERMINATED": NodeState.STOPPED
    }

Note, that I removed STOPPED since we don't (no longer?) have that state[2]. Does this look correct?

[1] https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/gce.py#L793-L800
[2] https://cloud.google.com/compute/docs/instances#checkmachinestatus

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 2, 2015

Member

Yeah, this should work:

 NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPING": NodeState.PENDING,
        "TERMINATED": NodeState.STOPPED
    }

(I just changed stopping to map to pending)

Member

Kami commented Feb 2, 2015

Yeah, this should work:

 NODE_STATE_MAP = {
        "PROVISIONING": NodeState.PENDING,
        "STAGING": NodeState.PENDING,
        "RUNNING": NodeState.RUNNING,
        "STOPPING": NodeState.PENDING,
        "TERMINATED": NodeState.STOPPED
    }

(I just changed stopping to map to pending)

@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 2, 2015

Member

Ok, sgtm. Heading out in a bit, but I'll get this fixed first thing tomorrow.

Thank you @Kami !

Member

erjohnso commented Feb 2, 2015

Ok, sgtm. Heading out in a bit, but I'll get this fixed first thing tomorrow.

Thank you @Kami !

@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 3, 2015

Member

Ok, just submitted #445

Member

erjohnso commented Feb 3, 2015

Ok, just submitted #445

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment