From ef0b2199c14c86af87a5c72e8fd022f075e4f486 Mon Sep 17 00:00:00 2001 From: Yumeng Bao Date: Tue, 26 Nov 2019 05:57:22 -0800 Subject: [PATCH] Refactor v2 arq api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users can make GET /v2/accelerator_requests request with parameters instance_uuid and bind_state to arq.get_all API to retrieve specific arqs. There are six possible request cases: 1.instance is not none，bind_state is none /v2/accelerator_requests?instance={instance_uuid} 2.instance is not none, bind_state=resolved /v2/accelerator_requests?instance={instance_uuid}&bind_state=resolved 3.instance is not none, bind_state=started (raise error：ARQInvalidState) /v2/accelerator_requests?instance={instance_uuid}&bind_state=started *Note:bind_state=started here is just an example,it can be any InvalidState 4.instance is none，bind_state is none /v2/accelerator_requests 5.instance is none, bind_state=resolved /v2/accelerator_requests?bind_state=resolved 6.instance is none, bind_state=started (raise error：ARQInvalidState) /v2/accelerator_requests?bind_state=started *Note:bind_state=started here is just an example,it can be any InvalidState But case 5 is missing, this patch fixed this, added the missing case 5 and added unit test. Change-Id: I783c3d0d257c16bf6c92c118f7b16f7f180f7026 --- cyborg/api/controllers/v2/arqs.py | 33 ++++++++++------- .../unit/api/controllers/v2/test_arqs.py | 37 ++++++++++++++++++- 2 files changed, 55 insertions(+), 15 deletions(-) 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')