openstack-qa-team team mailing list archive

[Daryl Walleck <daryl.walleck@xxxxxxxxxxxxx>]

What I'm saying is that wait_for_server_status is just a function that, at least at the moment, has no concept of this state machine. Coding in a whitelist of good states may be confusing/frustrating as the different projects are expanded. Unless you know to add in a good state for feature X that was just added, writing a test will be a headache. Instead, what I'm proposing is to make it a standard (documented in our HACKING file) that waiting for one of these in-between states is, in general, bad practice. This can then be controlled through code reviews. That's just my opinion.

Daryl

On Jun 18, 2012, at 12:43 PM, David Kranz wrote:

> I am not sure what you mean. I meant that the status argument of wait_for_server_status should be an enum type, whether that be an actual enum type or an allowed set of strings. It should be a static type error, or at least as close as we can come, to pass a value that always means a coding error. Or are you objecting to any kind of static type checking?
> 
> I also made a mistake in that DELETED should not be in the allowed set because this method will bomb if the server is actually in that state. The call to get_server fill fail which is why we also have wait_for_server_termination.
> 
> -David
> 
> On 6/18/2012 1:37 PM, Daryl Walleck wrote:
>> I'm not sure I agree with causing a failure based on what the user inputs to a function. I feel like this is something we should enforce through standards and not in code.
>> 
>> Daryl
>> 
>> On Jun 18, 2012, at 12:34 PM, David Kranz wrote:
>> 
>>> On 6/18/2012 1:07 PM, Jay Pipes wrote:
>>>> On 06/18/2012 12:49 PM, Daryl Walleck wrote:
>>>>> I can verify that rescue is a non-race state. The transition is active
>>>>> to rescue on setting rescue, and rescue to active when leaving rescue.
>>>> I don't see a RESCUE state. I see a RESCUED state. Is that what you are referring to here? Want to make sure, since the semantics and tenses of the power, VM, and task states are a bit inconsistent.
>>>> 
>>>> Best,
>>>> -jay
>>>> 
>>>>> -
>>> For a black-box test what we have is 'status', which is neither vm-state not task state. I believe 'status'  contains the values of the attributes in the below code. I am going to add an assertion to wait_for_server_status that will fail if you give it an ephemeral state.
>>>> From this list and the comments of Daryl and Jay, I propose the list of
>>> allowed states for this check:
>>> 
>>> ACTIVE, VERIFY_RESIZE, STOPPED, SHUTOFF, PAUSED, SUSPENDED, RESCUE, ERROR, DELETED
>>> 
>>> Any comments?
>>> 
>>> 
>>> From nova/nova/api/openstack/common.py:
>>> 
>>> _STATE_MAP = {
>>>    vm_states.ACTIVE: {
>>>        'default': 'ACTIVE',
>>>        task_states.REBOOTING: 'REBOOT',
>>>        task_states.REBOOTING_HARD: 'HARD_REBOOT',
>>>        task_states.UPDATING_PASSWORD: 'PASSWORD',
>>>        task_states.RESIZE_VERIFY: 'VERIFY_RESIZE',
>>>    },
>>>    vm_states.BUILDING: {
>>>        'default': 'BUILD',
>>>    },
>>>    vm_states.REBUILDING: {
>>>        'default': 'REBUILD',
>>>    },
>>>    vm_states.STOPPED: {
>>>        'default': 'STOPPED',
>>>    },
>>>    vm_states.SHUTOFF: {
>>>        'default': 'SHUTOFF',
>>>    },
>>>    vm_states.MIGRATING: {
>>>        'default': 'MIGRATING',
>>>    },
>>>    vm_states.RESIZING: {
>>>        'default': 'RESIZE',
>>>        task_states.RESIZE_REVERTING: 'REVERT_RESIZE',
>>>    },
>>>    vm_states.PAUSED: {
>>>        'default': 'PAUSED',
>>>    },
>>>    vm_states.SUSPENDED: {
>>>        'default': 'SUSPENDED',
>>>    },
>>>    vm_states.RESCUED: {
>>>        'default': 'RESCUE',
>>>    },
>>>    vm_states.ERROR: {
>>>        'default': 'ERROR',
>>>    },
>>>    vm_states.DELETED: {
>>>        'default': 'DELETED',
>>>    },
>>>    vm_states.SOFT_DELETE: {
>>>        'default': 'DELETED',
>>>    },
>>> }
>>> 
>>> def status_from_state(vm_state, task_state='default'):
>>>    """Given vm_state and task_state, return a status string."""
>>>    task_map = _STATE_MAP.get(vm_state, dict(default='UNKNOWN_STATE'))
>>>    status = task_map.get(task_state, task_map['default'])
>>>    LOG.debug("Generated %(status)s from vm_state=%(vm_state)s "
>>>              "task_state=%(task_state)s." % locals())
>>>    return status
>>> 
>>> 
>>> def vm_state_from_status(status):
>>>    """Map the server status string to a vm state."""
>>>    for state, task_map in _STATE_MAP.iteritems():
>>>        status_string = task_map.get("default")
>>>        if status.lower() == status_string.lower():
>>>            return state
>>> 
>>> 
>>> 
>>> -- 
>>> Mailing list: https://launchpad.net/~openstack-qa-team
>>> Post to     : openstack-qa-team@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~openstack-qa-team
>>> More help   : https://help.launchpad.net/ListHelp
>