cleanup scheduler tasks in resources

IMO this must be removed only when we have the whole infrastructure of workers running in separate processes/threads in place, when each worker processes a given resource exclusively until completion. Otherwise on current architecture we will penalize performance greatly.

I considered whether this could be done on current architecture, but ended up with simplified version of TaskRunner :( currently I am thinking about trying out python futures, but open to any ideas :)

Fix proposed to branch: master
Review: https://review.openstack.org/147825

Fix proposed to branch: master
Review: https://review.openstack.org/147826

Fix proposed to branch: master
Review: https://review.openstack.org/147874

Fix proposed to branch: master
Review: https://review.openstack.org/147953

Just to put a perspective on this bug, here are the reasons of why the current state it suboptimal AFAIU:

1) every top-level resource operation like create, delete etc are already scheduler.TaskRunner instances and wrapped in @wrappertask to drive subtasks, so introducing another layer is making things more complex.

2) when during "convergence" implementation we move to the framework of independent workers, each worker will be a simple single-threaded, not micro-threaded process processing a single resource action exclusively until completion or failure, so again scheduler logic will be an unnecessary level of complexity

3) if we ever decide that handle_* and check_*_complete must be handled by different workers, serializing the handler_data [1] will become a real issue if it holds such complex objects/functions like scheduler.* classes and methods

[1] https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L506

Reviewed: https://review.openstack.org/147825
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=307a3be1024d56e221ae2474abc04d05a74ee5d9
Submitter: Jenkins
Branch: master

commit 307a3be1024d56e221ae2474abc04d05a74ee5d9
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Jan 16 10:22:39 2015 +0000

    Remove TaskRunner from WaitCondition

    Also, cleanup one Py2.6 compat leftover.

    Change-Id: Id3964db53cab597032e0e46fe4dfdd1efabb037f
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/147826
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=423041730fe18cadf381dfeb8efc561d791538b6
Submitter: Jenkins
Branch: master

commit 423041730fe18cadf381dfeb8efc561d791538b6
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Jan 16 12:14:02 2015 +0000

    Remove TaskRunner from SwiftSignal

    Change-Id: I0484979f0c1bf42f8537f631e7bcd83324085862
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/147874
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=0f5c51d6dea8bc6ea77274f2002ba1cb30445cb7
Submitter: Jenkins
Branch: master

commit 0f5c51d6dea8bc6ea77274f2002ba1cb30445cb7
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Jan 16 14:06:47 2015 +0000

    Remove TaskRunner from most Neutron resources

    Only OS::Neutron::Pool is left still using it.

    Change-Id: Ib487de4a4f247416f71081098ab326c56641c2e9
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/147953
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=d54332ebc4af0aea7a120ba00a40a799cc9e6a43
Submitter: Jenkins
Branch: master

commit d54332ebc4af0aea7a120ba00a40a799cc9e6a43
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Jan 16 17:39:10 2015 +0000

    Remove TaskRunner from Neutron Pool resource

    Change-Id: Ia3abe373a690cced3a0e0bf0d7a1f84864359984
    Partial-Bug: #1393268

Related fix proposed to branch: master
Review: https://review.openstack.org/154429

Fix proposed to branch: master
Review: https://review.openstack.org/154977

Reviewed: https://review.openstack.org/154429
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=2971d8a345fb953aeff076bbce8bbfd420bbebd2
Submitter: Jenkins
Branch: master

commit 2971d8a345fb953aeff076bbce8bbfd420bbebd2
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Mon Feb 2 19:17:20 2015 +0000

    Do not use volume objects in volume resources

    This patch is a prerequiste for moving away from TaskRunner instances.

    Currently our mocking in tests relies heavily on our code using the rich cinder
    objects returned by cinderclient. The purpose of this patch then is to
    first move away from using those and modify mocking in tests accordingly,
    so that subsequent patch that removes TaskRunner instances from volume
    resources will modify as few tests as possible, which will better prove
    its correctness.

    Also, modified all deprecated rsrc.cinder() calls to rsrc.client() calls
    (using default_client_name) and optimized reuse of clients inside single
    methods.

    Change-Id: I10c991f9949149c2f4d4b7de00f17ddc4980ae96
    Related-Bug: #1393268

Related fix proposed to branch: master
Review: https://review.openstack.org/166166

Just a note - I've found several more places returning rich, hard-to-serialize objects from handle_<action>

OS::Trove::Instance

  - handle_create returns instance object
  - handle_delete returns instance object

OS::Trove::Cluster

  - handle_create returns cluster object
  - handle_delete returns cluster object

OS::Swift::Container

  - handle_delete returns list of "swift object" objects

OS::Heat::SoftwareDeployment

  - all methods might return a rich SD object, although it is not actually needed

Also need to doupbe-check all *Group resources and StackResource - the dependencies are quite entangled there.

And several more found in contrib resources

OS::Barbican::Order

  - handle_create returns ``order_href`` (probably harmless)

OS::Zaqar::Queue

  - handle_create returns queue object

Rackspace::Cloud::LoadBalancer

  - handle_create returns loadbalancer object
  - handle_delete returns taskrunner object
  - handle_update returns (possibly very long) list of taskrunner objects

Rackspace::Cloud::Network

  - handle_delete returns a network object as part of dictionary

Rackspace CloudServer

  - check_create_complete accepts a server object (as currently does Nova Server, need to be fixed in the same commit)

Reviewed: https://review.openstack.org/166166
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=5781b740a34c0c8551d7e653388eb7c54c084932
Submitter: Jenkins
Branch: master

commit 5781b740a34c0c8551d7e653388eb7c54c084932
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Mar 13 18:40:59 2015 +0200

    Do not detach/re-attach volumes in AWS Instance

    Currently during resource suspend/resume/delete we manually
    detach/re-attach volumes on AWS Instance resource.

    This is not needed, as Nova already keeps volumes attached
    to suspended instances, so the volume attachments are readily
    available on resume, and automatically detaches volumes on
    instance termination.

    Besides, since AWS CFN has no support for stack/resource suspend/resume,
    we do not have to keep compatibility with any CFN behavior
    in this regard.

    Change-Id: I38234d68467b5a4036e1a474a1e89d10686e1c3e
    Related-Bug: #1393268

Fix proposed to branch: master
Review: https://review.openstack.org/177481

Fix proposed to branch: master
Review: https://review.openstack.org/177488

Related fix proposed to branch: master
Review: https://review.openstack.org/178091

Reviewed: https://review.openstack.org/177488
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=860c7f6338c7e9e9ba987cc1f1ecf2cb7864671f
Submitter: Jenkins
Branch: master

commit 860c7f6338c7e9e9ba987cc1f1ecf2cb7864671f
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Apr 24 16:07:10 2015 -0700

    Do not pass rich objects in Zaqar Queue

    Do not return queue object from handle_create.
    An extra API call is introduced in check_create_complete.

    Change-Id: Ic46f50b011fe9e5b397e5f822c9e9c86e8544c81
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/177481
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=cbac15dfa1071ef90ce27d7cf9d0f407c35e15d9
Submitter: Jenkins
Branch: master

commit cbac15dfa1071ef90ce27d7cf9d0f407c35e15d9
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu Apr 23 16:50:48 2015 -0700

    Do not pass around rich objects in Trove resources

    As described by the ref'd bug, handle_* methods should not return rich
    objects to check_*_complete. This patch fixed such violations in Trove
    Istance and Trove Cluster resources.

    Change-Id: I65b317a17827258acd11ffc08dc4f2fd5cca517c
    Partial-Bug: #1393268

Great job so far Pavlo. After https://review.openstack.org/#/c/154977/ lands could we have another update of resources yet to fix?

Reviewed: https://review.openstack.org/178091
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=e4d193fc69453055a2362ca0d1c154ba2cb9b64c
Submitter: Jenkins
Branch: master

commit e4d193fc69453055a2362ca0d1c154ba2cb9b64c
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Apr 24 16:17:41 2015 -0700

    Add note on SoftwareDeployment object structure

    So not to worry about it in the sence of ref'd bug.

    Change-Id: I1f9a1211a97812e69faf21d4e37dc665cd02841a
    Related-Bug: #1393268

Fix proposed to branch: master
Review: https://review.openstack.org/185429

Related fix proposed to branch: master
Review: https://review.openstack.org/185435

Reviewed: https://review.openstack.org/185429
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=9de16459287e956b0fa08fc58fb20c07f6650bec
Submitter: Jenkins
Branch: master

commit 9de16459287e956b0fa08fc58fb20c07f6650bec
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Mon May 25 19:51:51 2015 +0300

    Do not pass rich objects in Swift Container

    from handle_delete to check_delete_complete, pass initial number of
    Swift objects in the container instead (or None)

    Change-Id: Idad1c4bee325f5483a598cd596fe04f4d0bb8cea
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/154977
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=a484c739736c8ccea74ec684da8971b189747193
Submitter: Jenkins
Branch: master

commit a484c739736c8ccea74ec684da8971b189747193
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Jan 23 18:38:57 2015 +0200

    Remove TaskRunner from Volume resources

    All the volume_tasks logic has been redistributed as methods of either
    cinder or nova client plugins.

    The logic of detaching was simplified and a check for out-of-band
    changes is no longer executed.

    The old VolumeAttachTask is left in place as it is still used in
    creating the AWS::EC2::Instance resource.

    Change-Id: I13f8e03ff090dd5f5739e5af231c77d066eb9eac
    Partial-Bug: #1393268

Related fix proposed to branch: master
Review: https://review.openstack.org/186063

Fix proposed to branch: master
Review: https://review.openstack.org/186363

Fix proposed to branch: master
Review: https://review.openstack.org/186424

Reviewed: https://review.openstack.org/185435
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=10626bf7f014de79aa216af8791ce8a926f653c0
Submitter: Jenkins
Branch: master

commit 10626bf7f014de79aa216af8791ce8a926f653c0
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Mon May 25 20:35:48 2015 +0300

    Add note on Barbican Order's order_ref structure

    So not to worry about it the sense of ref'd bug

    Change-Id: Iac6050c868717f385fe45a9c3a13dd30a3023c9a
    Related-Bug: #1393268

Fix proposed to branch: master
Review: https://review.openstack.org/186832

Reviewed: https://review.openstack.org/186063
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=13b3a9af6e6081a0e2691a758690791f5d0e17e5
Submitter: Jenkins
Branch: master

commit 13b3a9af6e6081a0e2691a758690791f5d0e17e5
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu Mar 12 16:06:49 2015 +0000

    Move _check_active to nova client plugin

    The method was practically identical in Instance and Server resources,
    with only difference in reported resource type as part of error message.

    Some mocks that mocked this method directly on resource object were
    modified accordingly in unit tests.

    The new `_check_active` is made to accept both Nova server objects and server
    UUIDs/strings. This will allow changing methods of server resources
    (and tests!) one-by-one.

    Added a new method `fetch_server` that tolerates intermittent failures
    when fetching a fresh server object (similar to existing `refresh_server`).

    Unit tests for `_check_active` and `fetch_server` added.

    Related-Bug: #1393268
    Change-Id: I1b63dbd9182cc8519f43ff859a408d49789263dd

Reviewed: https://review.openstack.org/186363
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=220004a5282c2f8bca0b371d93301d79d5a3f803
Submitter: Jenkins
Branch: master

commit 220004a5282c2f8bca0b371d93301d79d5a3f803
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu May 28 09:55:43 2015 +0000

    Do not pass rich objects on server's resume

    Unit tests for "immediate resume" were removed as they no longer differ
    in principle from "wait resume" tests due to an extra API call
    introduced.

    Partial-Bug: #1393268
    Change-Id: I291bd6c33634727f17c64646f776c0777ce3939c

Reviewed: https://review.openstack.org/186424
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=674569675a8e82234630240bfeb853675dc01de0
Submitter: Jenkins
Branch: master

commit 674569675a8e82234630240bfeb853675dc01de0
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu May 28 14:41:31 2015 +0000

    Do not pass rich objects in server's suspend

    Unit tests for "immediate suspend" were removed as they no longer differ
    in principle from "wait suspend" tests due to an extra API call
    introduced.

    Change-Id: I2f42c664742b208f68364dca8040990c0aa01b19
    Partial-Bug: #1393268

An update on what is left to fix:

Server resources (AWS Instance, Nova Server):
- create
  - a bit complicated for AWS Instance due to volume attachment
  - Rackspace CloudServer's create also needs to be fixed together with Nova Server
- update
  - going to be quite complicated due to many possible update combinations
  - includes rebuild/check_rebuild and resize/check_resize code in Nova client plugin

Rackspace CloudNetwork
- handle_delete returns network object as part of a dict

Rackspace LoadBalancer
- create returns lb object
- delete returns lb object
- update (returns possibly long list of scheduler tasks)

Stack resources - I am still not sure if those have to be fixed (and how). Below are suspicious places

StackResource
- update is scheduler-decorated and yields
- update_with_template uses yielding inner function

InstanceGroup
- _replace uses yielding functions and scheduler tasks
- resize uses yielding functions and scheduler tasks

Fix proposed to branch: master
Review: https://review.openstack.org/188766

Reviewed: https://review.openstack.org/186832
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=600608c845e0df932ddc2a888e5de97a682d900f
Submitter: Jenkins
Branch: master

commit 600608c845e0df932ddc2a888e5de97a682d900f
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri May 29 18:13:40 2015 +0300

    Do not pass rich objects on servers' delete

    Partial-Bug: #1393268
    Change-Id: I4034f48f75aad748ac313c385fa4f6b991cd4128

Fix proposed to branch: master
Review: https://review.openstack.org/201321

Fix proposed to branch: master
Review: https://review.openstack.org/204248

Reviewed: https://review.openstack.org/188766
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=4d910cbaa2713d8b35203c2683c8b3084730b7de
Submitter: Jenkins
Branch: master

commit 4d910cbaa2713d8b35203c2683c8b3084730b7de
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu Jun 4 22:26:52 2015 +0300

    Do not use rich server objects in servers' create

    Fixed Rackspace CloudServer and its unit tests as well.

    AWS Instance resource is still using VolumeAttachTasks,
    which will be fixed in a subsequent patch.

    Partial-Bug: #1393268
    Change-Id: Id72e1155dc0ab9b21f2859fe868b4bd090faba7a

Reviewed: https://review.openstack.org/201321
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=4f9371cfc0fd222a65b6b124636bb2fa40788676
Submitter: Jenkins
Branch: master

commit 4f9371cfc0fd222a65b6b124636bb2fa40788676
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Mon Jul 13 21:30:28 2015 +0300

    Do not use VolumeTasks in AWS Instance create

    Use volume progress objects instead (as in volume attachment resources).

    Module volume_tasks and class scheduler.PollingTaskGroup have been removed,
    as they are no longer used anywhere.

    Change-Id: Iff3b92f28d89ad1fdfd22511bb531be463855ccd
    Partial-Bug: #1393268

Reviewed: https://review.openstack.org/204248
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=a1819ff0696635c516d0eb1c59fa4f70cae27d65
Submitter: Jenkins
Branch: master

commit a1819ff0696635c516d0eb1c59fa4f70cae27d65
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Tue Jul 21 23:08:00 2015 +0300

    Do not use rich objects is servers update

    - Use new ServerUpdateProgress tasks that support delayed
      API calls (not only checks).
      As this new task imposes restrictions on compute plugin API,
      move all Server*Progress classes to new, plugin-agnostic module.
      Simple unit tests for this class added.
    - Changed logic in Nova client plugin to not use scheduler tasks.
    - Moved server interface attach/detach to Nova client plugin.
    - Split resize in two distinct methods 'resize' and 'verify_resize',
      with their own checkers. This allows better handling of such 'delayed'
      VM status transitions as
      ACTIVE->[resize]->ACTIVE->RESIZING->VERIFY_RESIZE->[confirm]->
      ->VERIFY_RESIZE->ACTIVE
      It might also make it easier later to plug a user notification in
      verify_resize instead of optimistically confirming it.
    - rebuild/resize and interface attach/detach are also made more resilent,
      tolerating intermittent API failures, allowing update logic
      to retry API call later.
    - Instance.handle_update was refactored similar to Server resource,
      with building tasks for update moved to separate private methods.

    Change-Id: Ic78adf878aac09e56f5c50052ee78bdf5b11aa21
    Partial-Bug: #1393268

fixes to all non-stack in-tree resources are merged.

still not sure what to do with StackResource and its children, and there is a big refactoring going on there now with two competing implementations proposed, so postponing them for now.

Rackspace resources are moved to separate bugs:
- https://bugs.launchpad.net/heat/+bug/1486464
- https://bugs.launchpad.net/heat/+bug/1486463

Sorry for leaving it out for such a long time, going back to it now.

I've double-checked all the StackResource class hierarchy, and apart from couple of small fixes,
the only resource violating the design proposed here currently is

ResourceGroup

in its handle_create and handle_update, that both might return lists of scheduler tasks.

I'll look into it and see what can be done, hopefully to close this for good.