From c9b71ebd65c698ff8fa6fb28b477a84c823b8203 Mon Sep 17 00:00:00 2001 From: David Bastos Date: Thu, 29 Feb 2024 21:42:30 -0300 Subject: [PATCH] Fix delete process to apps that have charts disabled When deleting an application that has one chart or more disabled, the app framework was not able to correctly delete the disabled charts from the helm repository. If, after deleting an app, an attempt was made to upload that same app, a failure would occur, informing that the charts were already in the helm repository. The correction consists of using the kustomization-orig.yaml file instead of kustomization.yaml in the deletion process to list the enabled and disabled charts. Another fix was made in case an application has the status of "upload failed" and an attempt is made to delete another app. This caused a Python runtime error because the get_chart_tarball_path function tried to access the dictionary key and it wasn't there. The solution was to check if the key for that chart exists and only then try to access it. New logs are added to alert the user if the chart does not exist. Test Plan: PASS: Build-pkgs PASS: Upload, apply, remove and delete dell-storage PASS: Upload, apply, remove and delete oidc-auth-apps PASS: upload, apply, remove and delete metrics-server PASS: Deletes app that has charts disabled and all charts are deleted from the helm repository correctly. PASS: After deleting and trying to upload the same app, no error occurs and the upload and apply process is completed successfully. PASS: Deleting an app with another app with "upload failed" status and no Python runtime error occurs Closes-Bug: 2055697 Change-Id: I22de414e8780fe3691d06bdd015e4c927dcc10f0 Signed-off-by: David Bastos --- .../sysinv/sysinv/sysinv/common/constants.py | 1 + .../sysinv/sysinv/conductor/kube_app.py | 21 +++++++++++++------ sysinv/sysinv/sysinv/sysinv/helm/utils.py | 16 ++++++++------ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/common/constants.py b/sysinv/sysinv/sysinv/sysinv/common/constants.py index bb131c09a1..d676dd8164 100644 --- a/sysinv/sysinv/sysinv/sysinv/common/constants.py +++ b/sysinv/sysinv/sysinv/sysinv/common/constants.py @@ -1829,6 +1829,7 @@ APP_FLUXCD_MANIFEST_DIR = 'fluxcd-manifests' APP_FLUXCD_BASE_PATH = os.path.join(tsc.PLATFORM_PATH, 'fluxcd') APP_FLUXCD_DATA_PATH = os.path.join(APP_FLUXCD_BASE_PATH, tsc.SW_VERSION) APP_ROOT_KUSTOMIZE_FILE = 'kustomization.yaml' +APP_ROOT_KUSTOMIZE_ORIG_FILE = 'kustomization-orig.yaml' APP_HELMREPOSITORY_FILE = "helmrepository.yaml" APP_BASE_HELMREPOSITORY_FILE = os.path.join("base", APP_HELMREPOSITORY_FILE) APP_RELEASE_CLEANUP_FILE = 'helmrelease_cleanup.yaml' diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py index 31d8f625e7..a5af8c3a39 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py @@ -1186,10 +1186,7 @@ class AppOperator(object): LOG.error(e) raise - def _get_list_of_charts(self, app): - return self._get_list_of_charts_fluxcd(app.sync_fluxcd_manifest) - - def _get_list_of_charts_fluxcd(self, manifest): + def _get_list_of_charts(self, app, include_disabled=False): """Get the charts information from the manifest directory The following chart data for each chart in the manifest file @@ -1199,11 +1196,23 @@ class AppOperator(object): - namespace - location - release + + :param app: application + :param include_disabled: boolean value to add disabled charts on function return + + :return: Array with chart object for each chart present in the application """ + manifest = app.sync_fluxcd_manifest helmrepo_path = os.path.join(manifest, "base", "helmrepository.yaml") + + if include_disabled: + app_root_kustomize_file = constants.APP_ROOT_KUSTOMIZE_ORIG_FILE + else: + app_root_kustomize_file = constants.APP_ROOT_KUSTOMIZE_FILE + root_kustomization_path = os.path.join( - manifest, constants.APP_ROOT_KUSTOMIZE_FILE) + manifest, app_root_kustomize_file) for f in (helmrepo_path, root_kustomization_path): if not os.path.isfile(f): raise exception.SysinvException(_( @@ -3153,7 +3162,7 @@ class AppOperator(object): self._plugins.deactivate_plugins(app) self._dbapi.kube_app_destroy(app.name) - app.charts = self._get_list_of_charts(app) + app.charts = self._get_list_of_charts(app, include_disabled=True) self._cleanup(app) self._utils._patch_report_app_dependencies(app.name + '-' + app.version) # One last check of app alarm, should be no-op unless the diff --git a/sysinv/sysinv/sysinv/sysinv/helm/utils.py b/sysinv/sysinv/sysinv/sysinv/helm/utils.py index febeb193c4..e5db29914c 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/utils.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/utils.py @@ -297,19 +297,23 @@ def get_chart_tarball_path(repo_path, chart_name, chart_version): :param repo_path: Filesystem path to the Helm repository :param chart_name: Name of the Helm chart :param chart_version: Version of the Helm chart - :return: string - Full path of the chart tarball in the repository if a + :return: String with full path of the chart tarball in the repository if a matching chart/version is found. Otherwise returns None. """ repo_index_file = os.path.join(repo_path, "index.yaml") with io.open(repo_index_file, "r", encoding="utf-8") as f: root_index_yaml = next(yaml.safe_load_all(f)) - chart_versions = root_index_yaml["entries"][chart_name] + if chart_name in root_index_yaml["entries"]: + chart_versions = root_index_yaml["entries"][chart_name] - for chart in chart_versions: - if chart["version"] == chart_version and len(chart["urls"]) > 0: - return os.path.join(repo_path, chart["urls"][0]) + for chart in chart_versions: + if chart["version"] == chart_version and len(chart["urls"]) > 0: + return os.path.join(repo_path, chart["urls"][0]) + else: + LOG.warning("Chart {} not found in {}." + .format(chart_name, repo_index_file)) + return None def index_repo(repo_path):