From 7ca02243d103a0f4380c3a347b9fdfae789537c1 Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Tue, 13 Sep 2022 13:59:21 +0000 Subject: [PATCH] Adding details to VIM API Conflict responses When the VIM Stragegy API returns a 409 Conflict for a create, it is much more useful to know details about the conflict. ie: what type of strategy already exists. This fix required: - the strategy creation rpc message adds an error string - pecan.abort 'conflict' must include the error string - the nfv_client code must extract the error string - the strategy types must return knowledge of an existing strategy type - the director class must include details of the existing strategy type in its failure messages. Test Plan: PASS: Create two patch strategies and view the error. PASS: Create two different strategies and view the error. PASS: View errors when creating multiple strategies from horizon. Closes-Bug: #1989485 Signed-off-by: Al Bailey Change-Id: Ic79eab03b70b8bc7fbe2876f95eea3f2f34ab791 --- .../nfv_client/openstack/rest_api.py | 40 +++++++++---------- .../sw_update/_sw_update_strategy.py | 12 +++--- .../nfv_vim/directors/_sw_mgmt_director.py | 10 ++--- .../events/_vim_sw_update_api_events.py | 6 ++- nfv/nfv-vim/nfv_vim/objects/_fw_update.py | 2 +- .../nfv_vim/objects/_kube_rootca_update.py | 2 +- nfv/nfv-vim/nfv_vim/objects/_kube_upgrade.py | 2 +- nfv/nfv-vim/nfv_vim/objects/_sw_patch.py | 2 +- nfv/nfv-vim/nfv_vim/objects/_sw_upgrade.py | 2 +- .../nfv_vim/rpc/_rpc_message_sw_update.py | 3 ++ 10 files changed, 45 insertions(+), 36 deletions(-) diff --git a/nfv/nfv-client/nfv_client/openstack/rest_api.py b/nfv/nfv-client/nfv_client/openstack/rest_api.py index 785cd333..38807400 100755 --- a/nfv/nfv-client/nfv_client/openstack/rest_api.py +++ b/nfv/nfv-client/nfv_client/openstack/rest_api.py @@ -59,17 +59,33 @@ def request(token_id, method, api_cmd, api_cmd_headers=None, except urllib.error.HTTPError as e: headers = list() response_raw = dict() + json_response = False if e.fp is not None: headers = list() # list of tuples for key, value in e.fp.info().items(): if key not in headers_per_hop: - cap_key = '-'.join((ck.capitalize() - for ck in key.split('-'))) - headers.append((cap_key, value)) + header = '-'.join((ck.capitalize() + for ck in key.split('-'))) + headers.append((header, value)) + # set a flag if the response is json + # to assist with extracting faultString + if 'Content-Type' == header: + if 'application/json' == value.split(';')[0]: + json_response = True response_raw = e.fp.read() + reason = '' + if json_response: + try: + response = json.loads(response_raw) + message = response.get('faultstring', None) + if message is not None: + reason = str(message.rstrip('.')) + except ValueError: + pass + if httplib.FOUND == e.code: return response_raw @@ -77,27 +93,11 @@ def request(token_id, method, api_cmd, api_cmd_headers=None, return None elif httplib.CONFLICT == e.code: - raise Exception("Operation failed: conflict detected") + raise Exception("Operation failed: conflict detected. %s" % reason) elif httplib.FORBIDDEN == e.code: raise Exception("Authorization failed") - # Attempt to get the reason for the http error from the response - reason = '' - for header, value in headers: - if 'Content-Type' == header: - if 'application/json' == value.split(';')[0]: - try: - response = json.loads(response_raw) - message = response.get('faultstring', None) - if message is not None: - reason = str(message.rstrip('.')) - print("Operation failed: %s" % reason) - break - - except ValueError: - pass - print("Rest-API status=%s, %s, %s, headers=%s, payload=%s, response=%s" % (e.code, method, api_cmd, api_cmd_headers, api_cmd_payload, response_raw)) diff --git a/nfv/nfv-vim/nfv_vim/api/controllers/v1/orchestration/sw_update/_sw_update_strategy.py b/nfv/nfv-vim/nfv_vim/api/controllers/v1/orchestration/sw_update/_sw_update_strategy.py index ebb6d9c0..301d7fff 100755 --- a/nfv/nfv-vim/nfv_vim/api/controllers/v1/orchestration/sw_update/_sw_update_strategy.py +++ b/nfv/nfv-vim/nfv_vim/api/controllers/v1/orchestration/sw_update/_sw_update_strategy.py @@ -553,6 +553,8 @@ class SwUpdateStrategyAPI(rest.RestController): elif rpc.RPC_MSG_RESULT.FAILED == response.result: DLOG.info("Strategy delete failed") + # TODO(abailey): consider adding error_string to + # DELETE_SW_UPDATE_STRATEGY_RESPONSE return pecan.abort(httplib.CONFLICT) DLOG.error("Unexpected result received, result=%s." % response.result) @@ -607,7 +609,7 @@ class SwPatchStrategyAPI(SwUpdateStrategyAPI): query_data.convert_strategy(strategy) return query_data elif rpc.RPC_MSG_RESULT.CONFLICT == response.result: - return pecan.abort(httplib.CONFLICT) + return pecan.abort(httplib.CONFLICT, response.error_string) DLOG.error("Unexpected result received, result=%s." % response.result) return pecan.abort(httplib.INTERNAL_SERVER_ERROR) @@ -675,7 +677,7 @@ class SwUpgradeStrategyAPI(SwUpdateStrategyAPI): query_data.convert_strategy(strategy) return query_data elif rpc.RPC_MSG_RESULT.CONFLICT == response.result: - return pecan.abort(httplib.CONFLICT) + return pecan.abort(httplib.CONFLICT, response.error_string) DLOG.error("Unexpected result received, result=%s." % response.result) return pecan.abort(httplib.INTERNAL_SERVER_ERROR) @@ -739,7 +741,7 @@ class FwUpdateStrategyAPI(SwUpdateStrategyAPI): query_data.convert_strategy(strategy) return query_data elif rpc.RPC_MSG_RESULT.CONFLICT == response.result: - return pecan.abort(httplib.CONFLICT) + return pecan.abort(httplib.CONFLICT, response.error_string) DLOG.error("Unexpected result received, result=%s." % response.result) return pecan.abort(httplib.INTERNAL_SERVER_ERROR) @@ -829,7 +831,7 @@ class KubeRootcaUpdateStrategyAPI(SwUpdateStrategyAPI): query_data.convert_strategy(strategy) return query_data elif rpc.RPC_MSG_RESULT.CONFLICT == response.result: - return pecan.abort(httplib.CONFLICT) + return pecan.abort(httplib.CONFLICT, response.error_string) DLOG.error("Unexpected result received, result=%s." % response.result) return pecan.abort(httplib.INTERNAL_SERVER_ERROR) @@ -902,7 +904,7 @@ class KubeUpgradeStrategyAPI(SwUpdateStrategyAPI): query_data.convert_strategy(strategy) return query_data elif rpc.RPC_MSG_RESULT.CONFLICT == response.result: - return pecan.abort(httplib.CONFLICT) + return pecan.abort(httplib.CONFLICT, response.error_string) DLOG.error("Unexpected result received, result=%s." % response.result) return pecan.abort(httplib.INTERNAL_SERVER_ERROR) diff --git a/nfv/nfv-vim/nfv_vim/directors/_sw_mgmt_director.py b/nfv/nfv-vim/nfv_vim/directors/_sw_mgmt_director.py index 95549829..7d62b530 100755 --- a/nfv/nfv-vim/nfv_vim/directors/_sw_mgmt_director.py +++ b/nfv/nfv-vim/nfv_vim/directors/_sw_mgmt_director.py @@ -59,7 +59,7 @@ class SwMgmtDirector(object): # Do not schedule the callback - if creation failed because a # strategy already exists, the callback will attempt to operate # on the old strategy, which is not what we want. - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update._sw_update_type return None, reason self._sw_update = objects.SwPatch() @@ -87,7 +87,7 @@ class SwMgmtDirector(object): # Do not schedule the callback - if creation failed because a # strategy already exists, the callback will attempt to operate # on the old strategy, which is not what we want. - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update._sw_update_type return None, reason self._sw_update = objects.SwUpgrade() @@ -119,7 +119,7 @@ class SwMgmtDirector(object): # Do not schedule the callback - if creation failed because a # strategy already exists, the callback will attempt to operate # on the old strategy, which is not what we want. - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update._sw_update_type return None, reason self._sw_update = objects.FwUpdate() @@ -154,7 +154,7 @@ class SwMgmtDirector(object): # Do not schedule the callback - if creation failed because a # strategy already exists, the callback will attempt to operate # on the old strategy, which is not what we want. - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update._sw_update_type return None, reason self._sw_update = objects.KubeRootcaUpdate() @@ -196,7 +196,7 @@ class SwMgmtDirector(object): # Do not schedule the callback - if creation failed because a # strategy already exists, the callback will attempt to operate # on the old strategy, which is not what we want. - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update._sw_update_type return None, reason self._sw_update = objects.KubeUpgrade() diff --git a/nfv/nfv-vim/nfv_vim/events/_vim_sw_update_api_events.py b/nfv/nfv-vim/nfv_vim/events/_vim_sw_update_api_events.py index 88523b47..c8769106 100755 --- a/nfv/nfv-vim/nfv_vim/events/_vim_sw_update_api_events.py +++ b/nfv/nfv-vim/nfv_vim/events/_vim_sw_update_api_events.py @@ -142,6 +142,7 @@ def vim_sw_update_api_create_strategy(connection, msg): else: DLOG.error("Invalid message name: %s" % msg.sw_update_type) response = rpc.APIResponseCreateSwUpdateStrategy() + # todo(abailey): consider adding error_string to other error types response.result = rpc.RPC_MSG_RESULT.FAILED connection.send(response.serialize()) DLOG.verbose("Sent response=%s." % response) @@ -150,10 +151,13 @@ def vim_sw_update_api_create_strategy(connection, msg): if uuid is None: response = rpc.APIResponseCreateSwUpdateStrategy() - if reason == "strategy already exists": + # change this to a prefix... + if reason is not None and reason.startswith("strategy already exists"): response.result = rpc.RPC_MSG_RESULT.CONFLICT + response.error_string = reason else: response.result = rpc.RPC_MSG_RESULT.FAILED + # todo(abailey): consider adding error_string to other error types connection.send(response.serialize()) DLOG.verbose("Sent response=%s." % response) connection.close() diff --git a/nfv/nfv-vim/nfv_vim/objects/_fw_update.py b/nfv/nfv-vim/nfv_vim/objects/_fw_update.py index 2fe87e95..e1098a86 100644 --- a/nfv/nfv-vim/nfv_vim/objects/_fw_update.py +++ b/nfv/nfv-vim/nfv_vim/objects/_fw_update.py @@ -43,7 +43,7 @@ class FwUpdate(SwUpdate): from nfv_vim import strategy if self._strategy: - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update_type return False, reason self._strategy = strategy.FwUpdateStrategy( diff --git a/nfv/nfv-vim/nfv_vim/objects/_kube_rootca_update.py b/nfv/nfv-vim/nfv_vim/objects/_kube_rootca_update.py index ef93ef37..48a209f8 100644 --- a/nfv/nfv-vim/nfv_vim/objects/_kube_rootca_update.py +++ b/nfv/nfv-vim/nfv_vim/objects/_kube_rootca_update.py @@ -49,7 +49,7 @@ class KubeRootcaUpdate(SwUpdate): from nfv_vim import strategy if self._strategy: - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update_type return False, reason self._strategy = \ diff --git a/nfv/nfv-vim/nfv_vim/objects/_kube_upgrade.py b/nfv/nfv-vim/nfv_vim/objects/_kube_upgrade.py index 5e2177eb..61d2d3af 100644 --- a/nfv/nfv-vim/nfv_vim/objects/_kube_upgrade.py +++ b/nfv/nfv-vim/nfv_vim/objects/_kube_upgrade.py @@ -50,7 +50,7 @@ class KubeUpgrade(SwUpdate): from nfv_vim import strategy if self._strategy: - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update_type return False, reason self._strategy = \ diff --git a/nfv/nfv-vim/nfv_vim/objects/_sw_patch.py b/nfv/nfv-vim/nfv_vim/objects/_sw_patch.py index fc9ec171..c6469b9a 100755 --- a/nfv/nfv-vim/nfv_vim/objects/_sw_patch.py +++ b/nfv/nfv-vim/nfv_vim/objects/_sw_patch.py @@ -43,7 +43,7 @@ class SwPatch(SwUpdate): from nfv_vim import strategy if self._strategy: - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update_type return False, reason self._strategy = strategy.SwPatchStrategy( diff --git a/nfv/nfv-vim/nfv_vim/objects/_sw_upgrade.py b/nfv/nfv-vim/nfv_vim/objects/_sw_upgrade.py index 015fc595..751e5f6e 100755 --- a/nfv/nfv-vim/nfv_vim/objects/_sw_upgrade.py +++ b/nfv/nfv-vim/nfv_vim/objects/_sw_upgrade.py @@ -39,7 +39,7 @@ class SwUpgrade(SwUpdate): from nfv_vim import strategy if self._strategy: - reason = "strategy already exists" + reason = "strategy already exists of type:%s" % self._sw_update_type return False, reason self._strategy = strategy.SwUpgradeStrategy( diff --git a/nfv/nfv-vim/nfv_vim/rpc/_rpc_message_sw_update.py b/nfv/nfv-vim/nfv_vim/rpc/_rpc_message_sw_update.py index 75a816bd..36f83e05 100755 --- a/nfv/nfv-vim/nfv_vim/rpc/_rpc_message_sw_update.py +++ b/nfv/nfv-vim/nfv_vim/rpc/_rpc_message_sw_update.py @@ -143,6 +143,7 @@ class APIResponseCreateSwUpdateStrategy(RPCMessage): RPC API Response Message - Create Software Update Strategy """ strategy = None + error_string = None def __init__(self, msg_version=RPC_MSG_VERSION.VERSION_1_0, msg_type=RPC_MSG_TYPE.CREATE_SW_UPDATE_STRATEGY_RESPONSE, @@ -152,9 +153,11 @@ class APIResponseCreateSwUpdateStrategy(RPCMessage): def serialize_payload(self, msg): msg['strategy'] = self.strategy + msg['error_string'] = self.error_string def deserialize_payload(self, msg): self.strategy = msg.get('strategy', None) + self.error_string = msg.get('error_string', None) def __str__(self): return "create-sw-update-strategy response: %s" % self.strategy