diff --git a/cyborg/api/controllers/v2/arqs.py b/cyborg/api/controllers/v2/arqs.py index 8ff632b1..38fbafb9 100644 --- a/cyborg/api/controllers/v2/arqs.py +++ b/cyborg/api/controllers/v2/arqs.py @@ -24,6 +24,7 @@ from cyborg.api.controllers import base from cyborg.api.controllers import link from cyborg.api.controllers import types from cyborg.api import expose +from cyborg.common import constants from cyborg.common import exception from cyborg.common.i18n import _ from cyborg.common import policy @@ -181,25 +182,31 @@ class ARQsController(base.CyborgController): bind_state or '', instance or '') context = pecan.request.context extarqs = objects.ExtARQ.list(context) + state_map = constants.ARQ_BIND_STATES_STATUS_MAP + valid_bind_states = list(state_map.keys()) arqs = [extarq.arq for extarq in extarqs] # TODO(Sundar): Optimize by doing the filtering in the db layer # Apply instance filter before state filter. - if instance is not None: + if bind_state: + if bind_state != 'resolved': + raise exception.ARQInvalidState(state=bind_state) + if instance: new_arqs = [arq for arq in arqs if arq['instance_uuid'] == instance] arqs = new_arqs - if bind_state is not None: - if bind_state != 'resolved': - raise exception.ARQInvalidState(state=bind_state) - unbound_flag = False - for arq in arqs: - if (arq['state'] != 'Bound' and - arq['state'] != 'BindFailed'): - unbound_flag = True - if instance is not None and unbound_flag: - # if any ARQ for this instance is not resolved. - # Return HTTP code '423 Locked' - return wsme.api.Response(None, status_code=http_client.LOCKED) + if bind_state: + for arq in new_arqs: + if arq['state'] not in valid_bind_states: + # NOTE(Sundar) This should return HTTP code 423 + # if any ARQ for this instance is not resolved. + LOG.warning('Some of ARQs for instance %s is not ' + 'resolved', instance) + return wsme.api.Response( + None, + status_code=http_client.LOCKED) + elif bind_state: + arqs = [arq for arq in arqs + if arq['state'] in valid_bind_states] ret = ARQCollection.convert_with_links(arqs) LOG.info('[arqs:get_all] Returned: %s', ret) diff --git a/cyborg/tests/unit/api/controllers/v2/test_arqs.py b/cyborg/tests/unit/api/controllers/v2/test_arqs.py index deaa1ea1..064461f0 100644 --- a/cyborg/tests/unit/api/controllers/v2/test_arqs.py +++ b/cyborg/tests/unit/api/controllers/v2/test_arqs.py @@ -68,6 +68,7 @@ class TestARQsController(v2_test.APITestV2): @mock.patch('cyborg.objects.ExtARQ.list') def test_get_all(self, mock_extarqs): + # test get_all of any bind_state mock_extarqs.return_value = self.fake_extarqs data = self.get_json(self.ARQ_URL, headers=self.headers) out_arqs = data['arqs'] @@ -78,14 +79,46 @@ class TestARQsController(v2_test.APITestV2): for in_extarq, out_arq in zip(self.fake_extarqs, out_arqs): self._validate_arq(in_extarq.arq, out_arq) - # test get_all response "423 Locked" + @mock.patch('cyborg.objects.ExtARQ.list') + def test_get_all_with_invalid_bind_state(self, mock_extarqs): + # test get_all with bind_state=started + mock_extarqs.return_value = self.fake_extarqs instance_uuid = self.fake_extarqs[0].arq.instance_uuid + url = '%s?instance=%s&bind_state=started' % ( + self.ARQ_URL, instance_uuid) + exc = None + try: + self.get_json(url, headers=self.headers) + except Exception as e: + exc = e + # TODO(all) Cyborg does not have fake HTTPRequest Object now, so just + # use assertIn here, improve this case with assertRaises later. + self.assertIn( + "Accelerator Requests cannot be requested with " + "state started.", exc.args[0]) + + url = '%s?bind_state=started' % (self.ARQ_URL) + exc = None + try: + self.get_json(url, headers=self.headers) + except Exception as e: + exc = e + # TODO(all) Cyborg does not have fake HTTPRequest Object now, so just + # use assertIn here, improve this case with assertRaises later. + self.assertIn( + "Accelerator Requests cannot be requested with " + "state started.", exc.args[0]) + + @mock.patch('cyborg.objects.ExtARQ.list') + def test_get_all_with_invalid_arq_state(self, mock_extarqs): + # test get_all response "423 Locked" # set ARQ state to 'BindStarted' self.fake_extarqs[0].arq.state = 'BindStarted' mock_extarqs.return_value = self.fake_extarqs + instance_uuid = self.fake_extarqs[0].arq.instance_uuid url = '%s?instance=%s&bind_state=resolved' % ( self.ARQ_URL, instance_uuid) - response = self.get_json(url, self.headers) + response = self.get_json(url, headers=self.headers, expect_errors=True) self.assertEqual(http_client.LOCKED, response.status_int) @mock.patch('cyborg.objects.DeviceProfile.get_by_name')