From 4a0521d70bbfb18fcde7471c69959b37f66d8332 Mon Sep 17 00:00:00 2001 From: Igor Soares Date: Fri, 30 Jun 2023 18:15:49 -0300 Subject: [PATCH] Improve AppFwk namespaces handling This commit improves the application framework namespaces handling by: 1) Removing the requirement for application developers to set a mandatory namespace on the root level kustomization.yaml. This aims to uncouple Flux resources namespaces and application specific namespaces. Establishing a mandatory top level namespace tells kustomize to put all underlying resources inside that same namespace. However, Flux resources such as HelmRepository can be on a different namespace than others resources such as the Chart definition namespace. An example from Flux's documentation can be seen at [1]. If no namespace is explicitly set for a given resource then the "default" namespace is assumed following the Kubernetes standard. 2) Allowing plugins to span multiple namespaces within a chart. Even though the code to generate Helm overrides files for multiple namespaces within the same chart was already in place, only the override file named after the same chart's namespace was being copied over to the system overrides. This behavior is fixed by creating a single intermediate override file per chart, containing content for all its namespaces. Single namespaced charts did not have their behavior changed, which guarantees compatibility with all current apps. No action is required from application developers unless they want to start using multiple namespaces per chart. This allows work done on kubevirt to proceed, since it now requires overrides that span two namespaces [2]. [1] https://fluxcd.io/flux/faq/#can-i-use-flux-helmreleases-without-gitops [2] https://review.opendev.org/c/starlingx/app-kubevirt/+/884431/2/python3-k8sapp-kubevirt/k8sapp_kubevirt/k8sapp_kubevirt/helm/kubevirt.py Test plan: PASS: build-pkgs -a & build-image PASS: Full AIO-SX install PASS: Edit kubevirt manifests to remove the root level kustomization namespace. Place the Helm repository and chart definitions on flux-helm and kubevirt namespaces respectively. Add the CDI namespace to the get_overrides method on the app plugin so that two application-specific namespaces are available for its single chart. Rebuild the app. Upload/apply kubevirt Check if the number of pod replicas correctly reflects the system overrides. Remove/delete kubevirt PASS: Add user overrides to both kubevirt namespaces changing the number of replicas. Check if the number of pod replicas correctly reflects the user overrides. PASS: Apply a single namespaced version of kubevirt and update to another single namespaced version. PASS: Apply a single namespaced version of kubevirt and update to a multiple namespaced version. PASS: Apply a multiple namespaced version of kubevirt and update to a single namespaced version. PASS: Apply a multiple namespaced version of kubevirt and update to another multiple namespaced version. PASS: Upload/apply platform-integ-apps PASS: Upload/apply/remove oidc-auth-apps using user overrides PASS: Upload/apply/remove/delete snmp PASS: Upload/apply/remove/delete metrics-server PASS: Upload/apply/remove/delete vault PASS: Upload/apply/remove/delete auditd Closes-Bug: 2025511 Co-Authored-By: David Barbosa Bastos Change-Id: I7b66d19c9cd977e6e1a82a613907e794de026cfb Signed-off-by: Igor Soares --- .../sysinv/sysinv/sysinv/common/constants.py | 1 + .../sysinv/sysinv/conductor/kube_app.py | 20 ++++++----- sysinv/sysinv/sysinv/sysinv/helm/helm.py | 33 ++++++++++++++----- .../sysinv/sysinv/helm/kustomize_base.py | 7 ++-- sysinv/sysinv/sysinv/sysinv/helm/utils.py | 15 +++++++++ 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/common/constants.py b/sysinv/sysinv/sysinv/sysinv/common/constants.py index e1c96dd94d..06950aa9ec 100644 --- a/sysinv/sysinv/sysinv/sysinv/common/constants.py +++ b/sysinv/sysinv/sysinv/sysinv/common/constants.py @@ -1818,6 +1818,7 @@ FLUXCD_RECOVERABLE_HELM_RELEASE_STATUS = [ FLUXCD_NAMESPACE = "flux-helm" FLUXCD_HELM_CONTROLLER_LABEL = "helm-controller" FLUXCD_SOURCE_CONTROLLER_LABEL = "source-controller" +FLUXCD_K8S_FALLBACK_NAMESPACE = "default" # State constants APP_NOT_PRESENT = 'missing' diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py index 5cff9552c8..c9612abbc9 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py @@ -536,6 +536,11 @@ class AppOperator(object): app.sync_fluxcd_manifest, app.sync_overrides_dir) + @staticmethod + def get_global_namespace(root_kustomization_yaml): + """ Retrieve the namespace of a top level kustomization """ + return root_kustomization_yaml.get("namespace", constants.FLUXCD_K8S_FALLBACK_NAMESPACE) + def _get_image_tags_by_charts_fluxcd(self, app_images_file, manifest, overrides_dir): app_imgs = [] images_file = None @@ -556,7 +561,7 @@ class AppOperator(object): # get namespace with io.open(root_kustomization_path, 'r', encoding='utf-8') as f: root_kustomization_yaml = next(yaml.safe_load_all(f)) - global_namespace = root_kustomization_yaml["namespace"] + global_namespace = AppOperator.get_global_namespace(root_kustomization_yaml) charts_groups = root_kustomization_yaml["resources"] for chart_group in charts_groups: @@ -1133,7 +1138,7 @@ class AppOperator(object): # get global namespace with io.open(root_kustomization_path, 'r', encoding='utf-8') as f: root_kustomization_yaml = next(yaml.safe_load_all(f)) - global_namespace = root_kustomization_yaml["namespace"] + global_namespace = AppOperator.get_global_namespace(root_kustomization_yaml) charts_groups = root_kustomization_yaml["resources"] # get the helm repo base url @@ -1188,7 +1193,7 @@ class AppOperator(object): available_helm_overrides = [] for chart in charts: - overrides = chart.namespace + '-' + chart.name + '.yaml' + overrides = helm_utils.build_overrides_filename(chart.name) overrides_file = os.path.join(overrides_dir, overrides) if not os.path.exists(overrides_file): missing_helm_overrides.append(overrides_file) @@ -1204,7 +1209,7 @@ class AppOperator(object): def _write_fluxcd_overrides(self, charts, helm_files): for chart in charts: - override_file = chart.namespace + '-' + chart.name + '.yaml' + override_file = chart.name + '.yaml' for f in os.listdir(chart.chart_os_path): if f.endswith("system-overrides.yaml"): @@ -2503,11 +2508,10 @@ class AppOperator(object): raise exception.KubeAppAbort() LOG.info("Generating application overrides...") - self._helm.generate_helm_application_overrides( - app.sync_overrides_dir, app.name, mode, cnamespace=None, - chart_info=app.charts, combined=True) + helm_files = self._helm.generate_helm_application_overrides( + app.sync_overrides_dir, app.name, mode, cnamespace=None, + chart_info=app.charts, combined=True) - helm_files = self._get_overrides_files(app) if helm_files: LOG.info("Application overrides generated.") # put the helm_overrides in the chart's system-overrides.yaml diff --git a/sysinv/sysinv/sysinv/sysinv/helm/helm.py b/sysinv/sysinv/sysinv/sysinv/helm/helm.py index 9d1e931c96..7c8c62a573 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/helm.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/helm.py @@ -679,6 +679,7 @@ class HelmOperator(object): system overrides """ + generated_files = [] app, plugin_name = self._find_kube_app_and_app_plugin_name(app_name) # Get a kustomize operator to provide a single point of @@ -735,7 +736,9 @@ class HelmOperator(object): Loader=yaml.FullLoader ) - self._write_chart_overrides(path, chart_name, cnamespace, overrides) + override_file = self._write_chart_overrides( + path, chart_name, cnamespace, overrides) + generated_files.append(override_file) # Update manifest docs based on the plugin directives. If the # application does not provide a manifest operator, the @@ -783,13 +786,16 @@ class HelmOperator(object): Loader=yaml.FullLoader ) - self._write_chart_overrides(path, chart.name, - cnamespace, user_overrides) + override_file = self._write_chart_overrides( + path, chart.name, cnamespace, user_overrides) + generated_files.append(override_file) # Write the kustomization doc overrides and a unified manifest for deletion. kustomize_op.save_kustomization_updates() kustomize_op.save_release_cleanup_data() + return generated_files + def _find_kube_app_and_app_plugin_name(self, app_name): return utils.find_kube_app(self.dbapi, app_name), \ utils.find_app_plugin_name(app_name) @@ -821,17 +827,26 @@ class HelmOperator(object): """Write a one or more overrides files for a chart. """ def _write_file(filename, values): + filepath = "" try: - self._write_overrides(path, filename, values) + filepath = self._write_overrides(path, filename, values) except Exception as e: LOG.exception("failed to write %s overrides: %s: %s" % ( chart_name, filename, e)) - if cnamespace: - _write_file("%s-%s.yaml" % (cnamespace, chart_name), overrides) + return filepath + + filename = helm_utils.build_overrides_filename(chart_name, cnamespace) + + # If the chart has just one namespace there is no need to add + # the top level reference to it. + if len(overrides) == 1: + filepath = _write_file(filename, + overrides[next(iter(overrides))]) else: - for ns in overrides.keys(): - _write_file("%s-%s.yaml" % (ns, chart_name), overrides[ns]) + filepath = _write_file(filename, overrides) + + return filepath def _write_overrides(self, path, filename, overrides): """Write a single overrides file. """ @@ -854,6 +869,8 @@ class HelmOperator(object): LOG.exception("failed to write overrides file: %s" % filepath) raise + return filepath + def _remove_overrides(self, path, filename): """Remove a single overrides file. """ diff --git a/sysinv/sysinv/sysinv/sysinv/helm/kustomize_base.py b/sysinv/sysinv/sysinv/sysinv/helm/kustomize_base.py index ed3ab38b8f..50e1cd840c 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/kustomize_base.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/kustomize_base.py @@ -116,7 +116,7 @@ class FluxCDKustomizeOperator(object): # Grab the global namespace self.kustomization_namespace = deepcopy( - self.kustomization_content[0]['namespace']) + self.kustomization_content[0].get("namespace")) # For these resources, find the HelmRelease info and build a resource # map @@ -175,9 +175,12 @@ class FluxCDKustomizeOperator(object): elif resource_helmrelease_namespace: LOG.debug("Using namespace defined on the helmrelease.yaml") namespace = resource_helmrelease_namespace - else: + elif self.kustomization_namespace: LOG.debug("Using namespace defined on the top level kustomization.yaml") namespace = self.kustomization_namespace + else: + LOG.debug("Using fallback namespace") + namespace = constants.FLUXCD_K8S_FALLBACK_NAMESPACE # Save pertinent data for disabling chart resources and cleaning # up existing helm releases after being disabled diff --git a/sysinv/sysinv/sysinv/sysinv/helm/utils.py b/sysinv/sysinv/sysinv/sysinv/helm/utils.py index da7fac1907..3b1fee5fff 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/utils.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/utils.py @@ -269,3 +269,18 @@ def compress_helm_release_data(release_data): release_data = release_data.decode('utf-8') return release_data + + +def build_overrides_filename(chart_name, namespace=None): + """ Build a standardized overrides filename + + :param chart_name: Name of the Helm chart + :param namespace: Chart base namespace + :return: string + Standardized overrides filename + """ + filename = chart_name + ".yaml" + if namespace: + filename = namespace + "-" + filename + + return filename