OpenStack Compute (nova)

Merge lp://qastaging/~jk0/nova/confirm_resizes into lp://qastaging/~hudson-openstack/nova/trunk

  1. confirm_resizes
  2. Merge into trunk
Proposed by Josh Kearney
Status: Merged
Approved by: Josh Kearney
Approved revision: 1581
Merged at revision: 1595
Proposed branch: lp://qastaging/~jk0/nova/confirm_resizes
Merge into: lp://qastaging/~hudson-openstack/nova/trunk
Diff against target: 284 lines (+109/-6)
12 files modified
nova/compute/manager.py (+14/-2)
nova/db/api.py (+5/-0)
nova/db/sqlalchemy/api.py (+19/-3)
nova/network/linux_net.py (+0/-1)
nova/tests/test_db_api.py (+25/-0)
nova/tests/test_virt_drivers.py (+4/-0)
nova/virt/driver.py (+5/-0)
nova/virt/fake.py (+3/-0)
nova/virt/hyperv.py (+3/-0)
nova/virt/libvirt/connection.py (+4/-0)
nova/virt/xenapi/vmops.py (+23/-0)
nova/virt/xenapi_conn.py (+4/-0)
To merge this branch: bzr merge lp://qastaging/~jk0/nova/confirm_resizes
Reviewer Review Type Date Requested Status
Matt Dietz (community) Approve
Brian Waldon (community) Approve
Jason Kölker (community) Approve
Review via email: mp+75791@code.qastaging.launchpad.net

Description of the change

Adds the ability to automatically confirm resizes after the `resize_confirm_window` (0/disabled by default).

To post a comment you must log in.
lp://qastaging/~jk0/nova/confirm_resizes updated
1577. By Josh Kearney

Only log migration info if they exist.

Revision history for this message
Jason Kölker (jason-koelker) wrote :

Is Bueno!

review: Approve
Revision history for this message
Brian Waldon (bcwaldon) wrote :

Great stuff. I would only ask that you expand the tests. It seems like the only test here checks for the function to be implemented, not much in the way of expected behavior.

review: Needs Information
Revision history for this message
Josh Kearney (jk0) wrote :

> Great stuff. I would only ask that you expand the tests. It seems like the
> only test here checks for the function to be implemented, not much in the way
> of expected behavior.

Not sure I agree here. What unit could be tested without being completely mocked?

Revision history for this message
Brian Waldon (bcwaldon) wrote :

> > Great stuff. I would only ask that you expand the tests. It seems like the
> > only test here checks for the function to be implemented, not much in the
> way
> > of expected behavior.
>
> Not sure I agree here. What unit could be tested without being completely
> mocked?

Well, all you need is expected input and output. So your expected input would be coming out of the db api layer (perhaps one instance active, one confirm_resize), then you run the poll function and check that both instances are active. I guess that is more of a 'functional' test, but we don't really distinguish between unit and functional in our suite.

lp://qastaging/~jk0/nova/confirm_resizes updated
1578. By Josh Kearney

Merged trunk.

1579. By Josh Kearney

Corrected the status in DB call.

1580. By Josh Kearney

Added unit test.

Revision history for this message
Josh Kearney (jk0) wrote :

> Well, all you need is expected input and output. So your expected input would
> be coming out of the db api layer (perhaps one instance active, one
> confirm_resize), then you run the poll function and check that both instances
> are active. I guess that is more of a 'functional' test, but we don't really
> distinguish between unit and functional in our suite.

Hey Brian,

I'm not convinced that a functional test will be very beneficial to us here, but I hope you will find the new test I added for the DB call useful.

Revision history for this message
Brian Waldon (bcwaldon) wrote :

Works for me, thanks Josh!

review: Approve
Revision history for this message
Matt Dietz (cerberus) wrote :

Pretty

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/tests/test_db_api.py

lp://qastaging/~jk0/nova/confirm_resizes updated
1581. By Josh Kearney

PEP8 cleanup.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.