Facade/state work for action abort #11214
Conversation
hpidcock
force-pushed
the
action-cancel-facade
branch
from
43d02e1
to
4182fd0
Compare
hpidcock
force-pushed
the
action-cancel-facade
branch
from
4182fd0
to
7ec5adb
Compare
hpidcock
force-pushed
the
action-cancel-facade
branch
6 times, most recently
from
4c22f7d
to
5547e6c
Compare
hpidcock
force-pushed
the
action-cancel-facade
branch
2 times, most recently
from
2e0c14d
to
4c3e198
Compare
|
!!build!! weird network issue |
hpidcock
force-pushed
the
action-cancel-facade
branch
from
4c3e198
to
ed621d6
Compare
| var doc actionNotificationDoc | ||
| coll, closer := w.db.GetCollection(actionNotificationsC) | ||
| defer closer() | ||
| iter := coll.Find(nil).Iter() |
There was a problem hiding this comment.
this could get really messy if there's a large number of unpruned completed actions.
can we filter on pending or running...?
(with a test)
There was a problem hiding this comment.
to preserve backwards compatibility, we need to fire for pending and running, at least once. If the uniter crashes it should get the running actions it had started so it can error them.
number of pending/running actions is unbounded, but I believe this being for a legacy facades only we can accept that risk.
A) We could fire all initial events, then subsequently only fire when the notify document is missing a change field, but that requires a read for each notification. This also changes the semantics of the fire once for each action.
B) A way around this is two collections, one for legacy watchers (fires only on pending) and one the new facade. This seems just as heavy as the current solution.
I'll implement option A and change all the semantics of fireOnce to notifyNewPending. But this solution isn't as robust as the current map or option B, future changes to the notify collection will need to keep in mind legacy behaviour.
This would have been substantially easier if the mongo collection watchers send the new doc state in the Change value (or a way to read the document in the past with revno)
There was a problem hiding this comment.
Pending and running yes. I was suggesting that'd we'd filter out anything else - completed, failed, aborted etc. I would not expect the number of pending/running to be too bad.
There was a problem hiding this comment.
There is no notification doc for completed states AFAIK.
hpidcock
force-pushed
the
action-cancel-facade
branch
from
ed621d6
to
c5b378e
Compare
|
|
1 similar comment
|
|
- Added Cancel to Action state for handling running/pending actions. - Added facade call on uniter facade to get ActionStatus. - Extended ActionNotifications to support changes not just new actions. - Added support for running operations to receive updates to remote state.
hpidcock
force-pushed
the
action-cancel-facade
branch
from
c5b378e
to
78337a7
Compare
|
|
Checklist
Facade/state work for action abort
This PR encapsulates state and facade changes to deal with
juju cancel-actionaborting running actions.Actions can enter an Aborting state when
juju cancel-action, this will refire the action notifications watcher, allowing the uniter to handle the abort. The uniter will need to fetch the current action status hence the introduction of the ActionStatus facade call.Second PR will follow to handle aborting action status.
Third PR will follow to handle aborting machine actions.
QA steps
Create an action that sleeps for a few minutes.
juju run-action myapp/0 mysleepyaction
juju show-action-status action-1
Action should be running.
juju cancel-action action-1
Action should enter the aborting state, but not do anything.
Action should complete successfully after sleep has elapsed.
Documentation changes
N/A
Bug reference
N/A