From 9f125629fe48cecc72d07a3003cac9f4aeacbd4a Mon Sep 17 00:00:00 2001 From: Anton Arefiev Date: Wed, 22 Feb 2017 18:56:44 +0200 Subject: [PATCH] Add new transaction starting -> error on timeout Exception along the `starting` inspection code may causes `no defined transition` error as there is no transition for `starting` state on timeout event. Also it could happen if small timeout configured, so there is not enough time for starting -> waiting transition, e.g. new greenthread spawn is blocked due to missing of available slots. Closes-Bug: #1662494 Change-Id: I66cf0374a2ba4ab4692110daafd4a3d2d20d56d6 --- doc/source/images/states.svg | 246 +++++++++--------- ironic_inspector/introspection_state.py | 1 + ironic_inspector/node_cache.py | 5 + ironic_inspector/test/unit/test_node_cache.py | 20 ++ ...ing-error-on-timeout-904aeeeb319ecb2b.yaml | 8 + 5 files changed, 160 insertions(+), 120 deletions(-) create mode 100644 releasenotes/notes/add-transition-starting-error-on-timeout-904aeeeb319ecb2b.yaml diff --git a/doc/source/images/states.svg b/doc/source/images/states.svg index 236ca411f..d87d9d1c5 100644 --- a/doc/source/images/states.svg +++ b/doc/source/images/states.svg @@ -4,171 +4,177 @@ - - + + Ironic Inspector states - + enrolling - -enrolling - - -error - -error - - -enrolling->error - - -error + +enrolling -processing - -processing +processing + +processing -enrolling->processing - - -process +enrolling->processing + + +process - -error->error - - -abort + +error + +error - -error->error - - -error - - -starting - -starting - - -error->starting - - -start - - -reapplying - -reapplying - - -error->reapplying - - -reapply + +enrolling->error + + +error -processing->error - - -error +processing->error + + +error finished - -finished + +finished -processing->finished - - -finish +processing->finished + + +finish + + +error->error + + +abort + + +error->error + + +error + + +starting + +starting + + +error->starting + + +start + + +reapplying + +reapplying + + +error->reapplying + + +reapply -starting->error - - -error +starting->error + + +timeout + + +starting->error + + +error -starting->starting - - -start +starting->starting + + +start waiting - -waiting + +waiting starting->waiting - - -wait + + +wait -reapplying->error - - -error +reapplying->error + + +error -reapplying->reapplying - - -reapply +reapplying->reapplying + + +reapply -reapplying->finished - - -finish +reapplying->finished + + +finish finished->starting - - -start + + +start finished->reapplying - - -reapply + + +reapply finished->finished - - -finish - - -waiting->error - - -timeout - - -waiting->error - - -abort + + +finish -waiting->processing - - -process +waiting->processing + + +process + + +waiting->error + + +abort + + +waiting->error + + +timeout -waiting->starting - - -start +waiting->starting + + +start diff --git a/ironic_inspector/introspection_state.py b/ironic_inspector/introspection_state.py index 9e48452d6..c36cbd8ff 100644 --- a/ironic_inspector/introspection_state.py +++ b/ironic_inspector/introspection_state.py @@ -127,6 +127,7 @@ State_space = [ Events.error: States.error, Events.start: States.starting, Events.wait: States.waiting, + Events.timeout: States.error }, }, { diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 5c17ec1e8..056e9904c 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -881,6 +881,11 @@ def clean_up(): try: if node_info.finished_at or node_info.started_at > threshold: continue + if node_info.state != istate.States.waiting: + LOG.error('Something went wrong, timeout occurs' + 'while introspection in "%s" state', + node_info.state, + node_info=node_info) node_info.fsm_event(istate.Events.timeout) node_info.finished(error='Introspection timeout') finally: diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index d06371ad4..d41bfac8b 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -376,6 +376,26 @@ class TestNodeCacheCleanUp(test_base.NodeTest): get_lock_mock.assert_called_once_with(self.uuid) get_lock_mock.return_value.acquire.assert_called_once_with() + @mock.patch.object(node_cache, '_get_lock', autospec=True) + @mock.patch.object(timeutils, 'utcnow') + def test_timeout_starting(self, time_mock, get_lock_mock): + time_mock.return_value = self.started_at + session = db.get_session() + db.model_query(db.Node, session=session).filter_by( + uuid=self.uuid).update({'state': istate.States.starting}) + + CONF.set_override('timeout', 1) + current_time = self.started_at + datetime.timedelta(seconds=2) + time_mock.return_value = current_time + + self.assertEqual([self.uuid], node_cache.clean_up()) + + res = [(row.state, row.finished_at, row.error) for row in + db.model_query(db.Node).all()] + self.assertEqual( + [(istate.States.error, current_time, 'Introspection timeout')], + res) + def test_old_status(self): CONF.set_override('node_status_keep_time', 42) session = db.get_session() diff --git a/releasenotes/notes/add-transition-starting-error-on-timeout-904aeeeb319ecb2b.yaml b/releasenotes/notes/add-transition-starting-error-on-timeout-904aeeeb319ecb2b.yaml new file mode 100644 index 000000000..dad1286f4 --- /dev/null +++ b/releasenotes/notes/add-transition-starting-error-on-timeout-904aeeeb319ecb2b.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Adds new transition ``starting`` -> ``error`` as reaction on ``timeout`` + event. +fixes: + - | + Timeout event on ``starting`` state lead to undefined transition error.