From 229acb985fda966d125b3358ef46eba658f301d0 Mon Sep 17 00:00:00 2001 From: Bin Qian Date: Fri, 1 Mar 2024 18:47:25 +0000 Subject: [PATCH] Add deletion constraint This change adds checks before deleting software releases: 1. software release is available or unavailable 2. When it is on a system controller, the release is not being used by a subcloud This change also update the following: 1. removed the exception handling in controller level, moved to exception hook 2. CLI code to display HTTP error, only handles 500 status code, by displaying message from API, all other 4xx, 5xx status code display HTTP error directly. 3. ensure CLI return 1 for unsuccessful requets (status code 500) 4. fixed some minor issues Story: 2010676 Task: 49657 TCs: passed: observe delection rejected because of release not found, release is not in available or unavailable state. passed: delete an available release passed: on system controller, successfully delete scenarios passed: (simulated) on system controller with subcloud, delete release used by subcloud is rejected Change-Id: I306b1d8604113b92d907384844e8e8107835a463 Signed-off-by: Bin Qian --- software-client/software_client/client.py | 2 +- .../software_client/common/http_errors.py | 49 ++++++++ .../software_client/common/utils.py | 42 +++++++ .../software_client/software_client.py | 9 +- software-client/software_client/v1/release.py | 2 +- .../software_client/v1/release_shell.py | 6 +- .../software/api/controllers/v1/software.py | 68 +++------- software/software/constants.py | 7 +- software/software/dc_utils.py | 118 ++++++++++++++++++ software/software/release_data.py | 23 +++- software/software/software_controller.py | 43 ++++++- software/software/software_entities.py | 6 +- software/software/software_functions.py | 1 - software/software/sysinv_utils.py | 20 ++- software/software/utils.py | 2 +- 15 files changed, 325 insertions(+), 73 deletions(-) create mode 100644 software-client/software_client/common/http_errors.py create mode 100644 software/software/dc_utils.py diff --git a/software-client/software_client/client.py b/software-client/software_client/client.py index 20c8f668..84e74ae5 100644 --- a/software-client/software_client/client.py +++ b/software-client/software_client/client.py @@ -4,7 +4,6 @@ # SPDX-License-Identifier: Apache-2.0 # -from keystoneauth1 import loading from oslo_utils import importutils from software_client import exc @@ -18,6 +17,7 @@ API_ENDPOINT = "http://127.0.0.1:" + API_PORT def _make_session(**kwargs): + from keystoneauth1 import loading """Construct a session based on authentication information :param kwargs: keyword args containing credentials, either: diff --git a/software-client/software_client/common/http_errors.py b/software-client/software_client/common/http_errors.py new file mode 100644 index 00000000..3ea590cf --- /dev/null +++ b/software-client/software_client/common/http_errors.py @@ -0,0 +1,49 @@ +""" +Copyright (c) 2024 Wind River Systems, Inc. + +SPDX-License-Identifier: Apache-2.0 + +""" + +# HTTP ERRORS, display message when corresponding status code +# is received. +# status code 500, will be handled separatedly, as it returns +# API request related information. +HTTP_ERRORS = { + 400: "Bad Request", + 401: "Unauthorized", + 403: "Forbidden", + 404: "Not Found", + 405: "Method Not Allowed", + 406: "Not Acceptable", + 407: "Proxy Authentication Required", + 408: "Request Timeout", + 409: "Conflict", + 410: "Gone", + 411: "Length Required", + 412: "Precondition Failed", + 413: "Content Too Large", + 414: "URI Too Long", + 415: "Unsupported Media Type", + 416: "Range Not Satisfiable", + 417: "Expectation Failed", + 418: "I'm a teapot", + 421: "Misdirected Request", + 422: "Unprocessable Content", + 423: "Locked", + 424: "Failed Dependency", + 425: "Too Early", + 426: "Upgrade Required", + 428: "Precondition Required", + 429: "Too Many Requests", + 501: "Not Implemented", + 502: "Bad Gateway", + 503: "Service Unavailable", + 504: "Gateway Timeout", + 505: "HTTP Version Not Support", + 506: "Variant Also Negotiates", + 507: "Insufficient Storage", + 508: "Loop Detected", + 509: "Not Extended", + 511: "Network Authentication Required" +} diff --git a/software-client/software_client/common/utils.py b/software-client/software_client/common/utils.py index 84c8625c..aa49e9e6 100644 --- a/software-client/software_client/common/utils.py +++ b/software-client/software_client/common/utils.py @@ -25,6 +25,8 @@ import textwrap from oslo_utils import importutils from six.moves import zip +from software_client.common.http_errors import HTTP_ERRORS + TERM_WIDTH = 72 @@ -116,6 +118,46 @@ def check_rc(req, data): return rc +def _display_info(text): + ''' display the basic info json object ''' + try: + data = json.loads(text) + except Exception: + print(f"Invalid response format: {text}") + return + + if "error" in data and data["error"] != "": + print("Error:\n%s" % data["error"]) + elif "warning" in data and data["warning"] != "": + print("Warning:\n%s" % data["warning"]) + elif "info" in data and data["info"] != "": + print(data["info"]) + + +def display_info(resp): + ''' + This function displays basic REST API return, w/ info json object: + { + "info":"", + "warning":"", + "error":"", + } + ''' + + status_code = resp.status_code + text = resp.text + + if resp.status_code == 500: + # all 500 error comes with basic info json object + _display_info(text) + elif resp.status_code in HTTP_ERRORS: + # any 4xx and 5xx errors does not contain API information. + print("Error:\n%s", HTTP_ERRORS[status_code]) + else: + # print out the basic info json object + _display_info(text) + + def print_result_list(header_data_list, data_list, has_error, sort_key=0): """ Print a list of data in a simple table format diff --git a/software-client/software_client/software_client.py b/software-client/software_client/software_client.py index f04e3c53..91dac8ad 100644 --- a/software-client/software_client/software_client.py +++ b/software-client/software_client/software_client.py @@ -392,12 +392,17 @@ class SoftwareClientShell(object): args.os_endpoint_type = endpoint_type client = sclient.get_client(api_version, auth_mode, **(args.__dict__)) + return args.func(client, args) + # TODO(bqian) reenable below once Exception classes are defined + """ try: args.func(client, args) except exc.Unauthorized: raise exc.CommandError("Invalid Identity credentials.") except exc.HTTPForbidden: raise exc.CommandError("Error: Forbidden") + """ + def do_bash_completion(self, args): """Prints all of the commands and options to stdout. @@ -435,7 +440,7 @@ class HelpFormatter(argparse.HelpFormatter): def main(): try: - SoftwareClientShell().main(sys.argv[1:]) + return SoftwareClientShell().main(sys.argv[1:]) except KeyboardInterrupt as e: print(('caught: %r, aborting' % (e)), file=sys.stderr) @@ -451,4 +456,4 @@ def main(): if __name__ == "__main__": - main() + sys.exit(main()) diff --git a/software-client/software_client/v1/release.py b/software-client/software_client/v1/release.py index fdf7bd34..90bbc246 100644 --- a/software-client/software_client/v1/release.py +++ b/software-client/software_client/v1/release.py @@ -83,7 +83,7 @@ class ReleaseManager(base.Manager): path = '/v1/software/upload' if is_local: - to_upload_filenames = json.dumps(valid_files) + to_upload_filenames = valid_files headers = {'Content-Type': 'text/plain'} return self._create(path, body=to_upload_filenames, headers=headers) else: diff --git a/software-client/software_client/v1/release_shell.py b/software-client/software_client/v1/release_shell.py index 435b4138..ed76bce3 100644 --- a/software-client/software_client/v1/release_shell.py +++ b/software-client/software_client/v1/release_shell.py @@ -192,9 +192,5 @@ def do_upload_dir(cc, args): def do_delete(cc, args): """Delete the software release""" resp, body = cc.release.release_delete(args.release) - if args.debug: - utils.print_result_debug(resp, body) - else: - utils.print_software_op_result(resp, body) - + utils.display_info(resp) return utils.check_rc(resp, body) diff --git a/software/software/api/controllers/v1/software.py b/software/software/api/controllers/v1/software.py index 276ef39b..e34ffc86 100644 --- a/software/software/api/controllers/v1/software.py +++ b/software/software/api/controllers/v1/software.py @@ -14,6 +14,7 @@ from pecan import Response import shutil from software.exceptions import SoftwareError +from software.exceptions import SoftwareServiceError from software.software_controller import sc import software.utils as utils import software.constants as constants @@ -26,32 +27,20 @@ class SoftwareAPIController(object): @expose('json') def commit_patch(self, *args): - try: - result = sc.patch_commit(list(args)) - except SoftwareError as e: - return dict(error=str(e)) - + result = sc.patch_commit(list(args)) sc.software_sync() return result @expose('json') def commit_dry_run(self, *args): - try: - result = sc.patch_commit(list(args), dry_run=True) - except SoftwareError as e: - return dict(error=str(e)) - + result = sc.patch_commit(list(args), dry_run=True) return result @expose('json') @expose('query.xml', content_type='application/xml') def delete(self, *args): - try: - result = sc.software_release_delete_api(list(args)) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) - + result = sc.software_release_delete_api(list(args)) sc.software_sync() return result @@ -60,13 +49,9 @@ class SoftwareAPIController(object): @expose('query.xml', content_type='application/xml') def deploy_activate(self, *args): if sc.any_patch_host_installing(): - return dict(error="Rejected: One or more nodes are installing a release.") - - try: - result = sc.software_deploy_activate_api(list(args)[0]) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + raise SoftwareServiceError(error="Rejected: One or more nodes are installing a release.") + result = sc.software_deploy_activate_api(list(args)[0]) sc.software_sync() return result @@ -74,12 +59,9 @@ class SoftwareAPIController(object): @expose('query.xml', content_type='application/xml') def deploy_complete(self, *args): if sc.any_patch_host_installing(): - return dict(error="Rejected: One or more nodes are installing a release.") + raise SoftwareServiceError(error="Rejected: One or more nodes are installing a release.") - try: - result = sc.software_deploy_complete_api(list(args)[0]) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_deploy_complete_api(list(args)[0]) sc.software_sync() return result @@ -93,10 +75,7 @@ class SoftwareAPIController(object): if len(list(args)) > 1 and 'force' in list(args)[1:]: force = True - try: - result = sc.software_deploy_host_api(list(args)[0], force, async_req=True) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_deploy_host_api(list(args)[0], force, async_req=True) return result @@ -107,10 +86,7 @@ class SoftwareAPIController(object): if 'force' in list(args): force = True - try: - result = sc.software_deploy_precheck_api(list(args)[0], force, **kwargs) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_deploy_precheck_api(list(args)[0], force, **kwargs) return result @@ -121,12 +97,9 @@ class SoftwareAPIController(object): force = 'force' in list(args) if sc.any_patch_host_installing(): - return dict(error="Rejected: One or more nodes are installing releases.") + raise SoftwareServiceError(error="Rejected: One or more nodes are installing a release.") - try: - result = sc.software_deploy_start_api(list(args)[0], force, **kwargs) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_deploy_start_api(list(args)[0], force, **kwargs) sc.send_latest_feed_commit_to_agent() sc.software_sync() @@ -144,10 +117,7 @@ class SoftwareAPIController(object): @expose('json') @expose('query.xml', content_type='application/xml') def install_local(self): - try: - result = sc.software_install_local_api() - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_install_local_api() return result @@ -166,10 +136,7 @@ class SoftwareAPIController(object): @expose('json') @expose('show.xml', content_type='application/xml') def show(self, *args): - try: - result = sc.software_release_query_specific_cached(list(args)) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + result = sc.software_release_query_specific_cached(list(args)) return result @@ -212,8 +179,6 @@ class SoftwareAPIController(object): # Process uploaded files return sc.software_release_upload(uploaded_files) - except Exception as e: - return dict(error=str(e)) finally: # Remove all uploaded files from /scratch dir sc.software_sync() @@ -223,10 +188,7 @@ class SoftwareAPIController(object): @expose('json') @expose('query.xml', content_type='application/xml') def query(self, **kwargs): - try: - sd = sc.software_release_query_cached(**kwargs) - except SoftwareError as e: - return dict(error="Error: %s" % str(e)) + sd = sc.software_release_query_cached(**kwargs) return dict(sd=sd) diff --git a/software/software/constants.py b/software/software/constants.py index 73b945a3..f358e027 100644 --- a/software/software/constants.py +++ b/software/software/constants.py @@ -21,6 +21,9 @@ ADDRESS_VERSION_IPV4 = 4 ADDRESS_VERSION_IPV6 = 6 CONTROLLER_FLOATING_HOSTNAME = "controller" +DISTRIBUTED_CLOUD_ROLE_SYSTEMCONTROLLER = 'systemcontroller' +SYSTEM_CONTROLLER_REGION = 'SystemController' + SOFTWARE_STORAGE_DIR = "/opt/software" SOFTWARE_CONFIG_FILE_LOCAL = "/etc/software/software.conf" @@ -64,7 +67,8 @@ UNAVAILABLE = 'unavailable' DEPLOYING = 'deploying' DEPLOYED = 'deployed' REMOVING = 'removing' -UNKNOWN = 'n/a' + +DELETABLE_STATE = [AVAILABLE, UNAVAILABLE] # TODO(bqian) states to be removed once current references are removed ABORTING = 'aborting' @@ -182,6 +186,7 @@ class DEPLOY_STATES(Enum): HOST_DONE = 'host-done' HOST_FAILED = 'host-failed' + class DEPLOY_HOST_STATES(Enum): DEPLOYED = 'deployed' DEPLOYING = 'deploying' diff --git a/software/software/dc_utils.py b/software/software/dc_utils.py new file mode 100644 index 00000000..48732c6b --- /dev/null +++ b/software/software/dc_utils.py @@ -0,0 +1,118 @@ +""" +Copyright (c) 2024 Wind River Systems, Inc. + +SPDX-License-Identifier: Apache-2.0 + +""" +import json +import logging +from keystoneauth1 import exceptions +from keystoneauth1 import identity +from keystoneauth1 import session + +from oslo_config import cfg +from oslo_utils import encodeutils +from six.moves.urllib.request import Request +from six.moves.urllib.request import urlopen + +from software import utils +from software.constants import SYSTEM_CONTROLLER_REGION + + +LOG = logging.getLogger('main_logger') +CONF = cfg.CONF + + +def get_token_endpoint(service_type, region_name=None, interface="internal"): + config = CONF.get('keystone_authtoken') + if region_name is None: + region_name = config.region_name + + try: + auth = identity.Password( + auth_url=config.auth_url, + username=config.username, + password=config.password, + project_name=config.project_name, + user_domain_name=config.user_domain_name, + project_domain_name=config.project_domain_name + ) + sess = session.Session(auth=auth) + token = sess.get_token() + endpoint = sess.get_endpoint(service_type=service_type, + region_name=region_name, + interface=interface) + except exceptions.http.Unauthorized: + raise Exception("Failed to authenticate to Keystone. Request unauthorized") + except Exception as e: + msg = "Failed to get token and endpoint. Error: %s", str(e) + raise Exception(msg) + return token, endpoint + + +def rest_api_request(token, method, api_cmd, + api_cmd_payload=None, timeout=45): + """ + Make a rest-api request + Returns: response as a dictionary + """ + api_cmd_headers = dict() + api_cmd_headers['Content-type'] = "application/json" + api_cmd_headers['User-Agent'] = "usm/1.0" + + request_info = Request(api_cmd) + request_info.get_method = lambda: method + if token: + request_info.add_header("X-Auth-Token", token) + request_info.add_header("Accept", "application/json") + + if api_cmd_headers is not None: + for header_type, header_value in api_cmd_headers.items(): + request_info.add_header(header_type, header_value) + + if api_cmd_payload is not None: + request_info.data = encodeutils.safe_encode(api_cmd_payload) + + request = None + try: + request = urlopen(request_info, timeout=timeout) + response = request.read() + finally: + if request: + request.close() + + if response == "": + response = json.loads("{}") + else: + response = json.loads(response) + + return response + + +def get_subclouds_from_dcmanager(): + token, api_url = get_token_endpoint("dcmanager", region_name=SYSTEM_CONTROLLER_REGION) + + api_cmd = api_url + '/subclouds' + LOG.debug('api_cmd %s' % api_cmd) + data = rest_api_request(token, "GET", api_cmd) + if 'subclouds' in data: + return data['subclouds'] + raise Exception(f"Incorrect response from dcmanager for querying subclouds {data}") + + +def get_subcloud_groupby_version(): + subclouds = get_subclouds_from_dcmanager() + grouped_subclouds = {} + for subcloud in subclouds: + major_ver = utils.get_major_release_version(subcloud['software_version']) + if major_ver not in grouped_subclouds: + grouped_subclouds[major_ver] = [subcloud] + else: + grouped_subclouds[major_ver].append(subcloud) + + msg = "total %s subclouds." % len(subclouds) + for ver in grouped_subclouds: + msg = msg + " %s: %s subclouds." % (ver, len(grouped_subclouds[ver])) + + LOG.info(msg) + return grouped_subclouds diff --git a/software/software/release_data.py b/software/software/release_data.py index afd9a29e..beeef3e1 100644 --- a/software/software/release_data.py +++ b/software/software/release_data.py @@ -5,11 +5,13 @@ # import os +from packaging import version import shutil from software import constants from software.exceptions import FileSystemError from software.exceptions import InternalError from software.software_functions import LOG +from software import utils class SWRelease(object): @@ -19,6 +21,7 @@ class SWRelease(object): self._id = rel_id self._metadata = metadata self._contents = contents + self._sw_version = None @property def metadata(self): @@ -83,9 +86,17 @@ class SWRelease(object): raise InternalError(error) @property - def sw_version(self): + def sw_release(self): + '''3 sections MM.mm.pp release version''' return self.metadata['sw_version'] + @property + def sw_version(self): + '''2 sections MM.mm software version''' + if self._sw_version is None: + self._sw_version = utils.get_major_release_version(self.sw_release) + return self._sw_version + def _get_latest_commit(self): num_commits = self.contents['number_of_commits'] if int(num_commits) > 0: @@ -156,6 +167,16 @@ class SWRelease(object): # latest commit return None + @property + def is_ga_release(self): + ver = version.parse(self.sw_release) + _, _, pp = ver.release + return pp == 0 + + @property + def is_deletable(self): + return self.state in constants.DELETABLE_STATE + class SWReleaseCollection(object): '''SWReleaseCollection encapsulates aggregated software release collection diff --git a/software/software/software_controller.py b/software/software/software_controller.py index c29b6f0b..edd39b8a 100644 --- a/software/software/software_controller.py +++ b/software/software/software_controller.py @@ -31,6 +31,7 @@ from software.api import app from software.authapi import app as auth_app from software.constants import DEPLOY_STATES from software.base import PatchService +from software.dc_utils import get_subcloud_groupby_version from software.exceptions import APTOSTreeCommandFail from software.exceptions import InternalError from software.exceptions import MetadataFail @@ -66,6 +67,7 @@ from software.release_verify import verify_files import software.config as cfg import software.utils as utils from software.sysinv_utils import get_k8s_ver +from software.sysinv_utils import is_system_controller from software.db.api import get_instance @@ -1148,7 +1150,7 @@ class PatchController(PatchService): max_major_releases = 2 major_releases = [] for rel in self.release_collection.iterate_releases(): - major_rel = utils.get_major_release_version(rel.sw_version) + major_rel = rel.sw_version if major_rel not in major_releases: major_releases.append(major_rel) @@ -1470,7 +1472,43 @@ class PatchController(PatchService): msg_error = "" # Protect against duplications - release_list = sorted(list(set(release_ids))) + full_list = sorted(list(set(release_ids))) + + not_founds = [] + cannot_del = [] + used_by_subcloud = [] + release_list = [] + for rel_id in full_list: + rel = self.release_collection.get_release_by_id(rel_id) + if rel is None: + not_founds.append(rel_id) + else: + if not rel.is_deletable: + cannot_del.append(rel_id) + elif rel.is_ga_release and is_system_controller(): + subcloud_by_sw_version = get_subcloud_groupby_version() + if rel.sw_version in subcloud_by_sw_version: + used_by_subcloud.append(rel_id) + else: + release_list.append(rel_id) + else: + release_list.append(rel_id) + + err_msg = "" + if len(not_founds) > 0: + list_str = ','.join(not_founds) + err_msg = f"Releases {list_str} can not be found\n" + + if len(cannot_del) > 0: + list_str = ','.join(cannot_del) + err_msg = err_msg + f"Releases {list_str} are not ready to delete\n" + + if len(used_by_subcloud) > 0: + list_str = ','.join(used_by_subcloud) + err_msg = err_msg + f"Releases {list_str} are still used by subcloud(s)" + + if len(err_msg) > 0: + raise SoftwareServiceError(error=err_msg) msg = "Deleting releases: %s" % ",".join(release_list) LOG.info(msg) @@ -2915,7 +2953,6 @@ class PatchController(PatchService): return None deploy = deploy[0] - deploy_host_list = [] for host in deploy_hosts: state = host.get("state") diff --git a/software/software/software_entities.py b/software/software/software_entities.py index fc4f110f..2a87bd19 100644 --- a/software/software/software_entities.py +++ b/software/software/software_entities.py @@ -276,8 +276,8 @@ class DeployHandler(Deploy): """ super().query(from_release, to_release) for deploy in self.data.get("deploy", []): - if (deploy.get("from_release") == from_release - and deploy.get("to_release") == to_release): + if (deploy.get("from_release") == from_release and + deploy.get("to_release") == to_release): return deploy return [] @@ -325,7 +325,7 @@ class DeployHostHandler(DeployHosts): super().__init__() self.data = get_software_filesystem_data() - def create(self, hostname, state:DEPLOY_HOST_STATES=None): + def create(self, hostname, state: DEPLOY_HOST_STATES = None): super().create(hostname, state) deploy = self.query(hostname) if deploy: diff --git a/software/software/software_functions.py b/software/software/software_functions.py index 0b26e132..fbaa4a47 100644 --- a/software/software/software_functions.py +++ b/software/software/software_functions.py @@ -1178,7 +1178,6 @@ def create_deploy_hosts(): raise err - def collect_current_load_for_hosts(): load_data = { "current_loads": [] diff --git a/software/software/sysinv_utils.py b/software/software/sysinv_utils.py index d72ffee6..fe49dbbd 100644 --- a/software/software/sysinv_utils.py +++ b/software/software/sysinv_utils.py @@ -5,8 +5,10 @@ SPDX-License-Identifier: Apache-2.0 """ import logging -import software.utils as utils + from software.exceptions import SysinvClientNotInitialized +from software import constants +from software import utils LOG = logging.getLogger('main_logger') @@ -41,6 +43,7 @@ def get_k8s_ver(): return k8s_ver.version raise Exception("Failed to get current k8s version") + def get_ihost_list(): try: token, endpoint = utils.get_endpoints_token() @@ -49,3 +52,18 @@ def get_ihost_list(): except Exception as err: LOG.error("Error getting ihost list: %s", err) raise + + +def get_dc_role(): + try: + token, endpoint = utils.get_endpoints_token() + sysinv_client = get_sysinv_client(token=token, endpoint=endpoint) + system = sysinv_client.isystem.list()[0] + return system.distributed_cloud_role + except Exception as err: + LOG.error("Error getting DC role: %s", err) + raise + + +def is_system_controller(): + return get_dc_role() == constants.DISTRIBUTED_CLOUD_ROLE_SYSTEMCONTROLLER diff --git a/software/software/utils.py b/software/software/utils.py index 96346bcb..ffe3aa25 100644 --- a/software/software/utils.py +++ b/software/software/utils.py @@ -42,10 +42,10 @@ class ExceptionHook(hooks.PecanHook): if isinstance(e, SoftwareServiceError): LOG.warning("An issue is detected. Signature [%s]" % signature) + # TODO(bqian) remove the logging after it is stable LOG.exception(e) data = dict(info=e.info, warning=e.warning, error=e.error) - data['error'] = data['error'] + " Error signature [%s]" % signature else: err_msg = "Internal error occurred. Error signature [%s]" % signature LOG.error(err_msg)