Broken support for applications with charts spanning multiple namespaces

Bug #2071380 reported by David Barbosa Bastos
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
StarlingX
Fix Released
Medium
David Barbosa Bastos

Bug Description

Brief Description
-----------------

A function was introduced to check if charts are enabled when applying apps. However, it is not finding any charts for Kubevirt because it is looking at the top-level namespace defined on the root kustomization.yaml file. We cannot rely on the top level namespace because 1) it is not mandatory by FluxCD, thus it may not be present, 2) charts spanning multiple namespaces required that not to be set, as defining a namespace on the root kustomization file results in all underlying resources having that same namespace.

Note: This is not happening to other apps because their top-level namespace matches the namespace defined in the app plugins.

A suggestion here is to generate is to generated the resulting manifests with kustomize and parse them, as they will always provide the namespace for the corresponding helm releases, regardless if that namespace was set on the root kustomization file or on the release definition itself. The release namespace is the one that should be passed to the is_chart_enabled method.

Severity
--------
Critical: System/Feature is not usable after the defect

Steps to Reproduce
------------------
Upload and apply Kubevirt app.

Add an override. For example:
system helm-override-update --set replicas=<NEW_VALUE> kubevirt-app kubevirt-app kubevirt
Reapply the app.

Replica numbers will not be changed.

Expected Behavior
------------------
Kubevirt applies and Helm overrides are respected

Actual Behavior
----------------
Kubevirt applies but Helm overrides do not work

Reproducibility
---------------
100% Reproducible

System Configuration
--------------------
SW_VERSION="24.09"
BUILD_TARGET="Unknown"
BUILD_TYPE="Informal"
BUILD_ID="n/a"

JOB="n/a"
BUILD_BY="igor-soares"
BUILD_NUMBER="n/a"

Branch/Pull Time/Commit
-----------------------
Branch and the time when code was pulled or git commit or cengn load info

Last Pass
---------
Before https://review.opendev.org/c/starlingx/config/+/915377 was merged.

Timestamp/Logs
--------------
sysinv 2024-06-01 11:54:15.493 13106 WARNING sysinv.common.utils [-] is_chart_enabled: kubevirt-app/kubevirt-app/kube-system overrides missing: sysinv.common.exception.HelmOverrideNotFound: No helm override with name kubevirt-app and namespace kube-system
sysinv 2024-06-01 11:54:15.493 13106 ERROR sysinv.conductor.kube_app [-] Application metadata key 'apply_progress_adjust'has an invalid value 0 (too few charts)
sysinv 2024-06-01 11:54:15.494 13106 INFO sysinv.conductor.kube_app [-] lifecycle hook for application kubevirt-app (23.09-21) started {'lifecycle_type': 'fluxcd-request', 'relative_timing': 'post', 'operation': 'apply', 'extra': {'rc': True}}.
sysinv 2024-06-01 11:54:15.972 13106 INFO sysinv.conductor.kube_app [-] Application kubevirt-app (23.09-21) apply completed.

Test Activity
-------------
Developer Testing

Workaround
----------
n/a

Changed in starlingx:
assignee: nobody → David Barbosa Bastos (dbarbosa-wr)
Changed in starlingx:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to config (master)

Reviewed: https://review.opendev.org/c/starlingx/config/+/922932
Committed: https://opendev.org/starlingx/config/commit/674e38adf8ed55e1daff176af65e7a415c4591b1
Submitter: "Zuul (22348)"
Branch: master

commit 674e38adf8ed55e1daff176af65e7a415c4591b1
Author: David Bastos <email address hidden>
Date: Thu Jun 27 09:46:12 2024 -0300

    Fix support for apps with multiple namespaces

    Helm allows charts to span multiple namespaces. However, the
    behavior implemented in the Application Framework was using only
    namespaces available on kustomization.yaml files to look for enabled
    charts. This strategy breaks the multiple namespace support because
    kustomize requires each HelmRelease resource to have a different
    namespace to support this scenario. Using a namespace on the a
    kustomization.yaml file means that a single namespace would be used
    across all underlying resources, including Helm releases.

    As a result, the "is_chart_enabled" function, would always return no
    charts for the multiple namespaces scenario, since the release
    namespace and the namespace used by the framework would not match,
    causing Helm overrides not to work correctly.

    The solution consists of using the output of the "kubectl kustomize
    <fluxcd_directory>" command and checking the namespace present
    within each HelmRelease. That way we ensure that the HelmRelease
    manifest always has a namespace, whether it is inherited by a
    kustomization.yaml file or explicitly defined on the release manifest.

    Test Plan:
    PASS: Upload, apply, remove and delete platform-integ-app
    PASS: Upload, apply, remove and delete metrics-server
    PASS: Upload, apply, remove and delete Kubevirt
    PASS: Update user overrides to metrics-server and platform-integ-app
          via "system helm-override-update" and when reapplying the app,
          the changes were made successfully
    PASS: Update the user overrides of the kubevirt app for the kubevirt
          namespace: "system helm-override-update kubevirt-app
          kubevirt-app kubevirt --set replicas=3". After running the
          command and reapplying the app, the changes were made
          successfully.
    PASS: Update the user overrides of the kubevirt app for the cdi
          namespace: "system helm-override-update kubevirt-app
          kubevirt-app cdi --set replicas=3". After running the command
          and reapplying the app, the changes were made successfully.

    Closes-bug: 2071380

    Change-Id: I8a7535a4303f3a64d99cc8cc3db36bd2d6d6ba5d
    Signed-off-by: David Bastos <email address hidden>

Changed in starlingx:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to app-kubevirt (master)

Reviewed: https://review.opendev.org/c/starlingx/app-kubevirt/+/922968
Committed: https://opendev.org/starlingx/app-kubevirt/commit/8dfe02713b44f6e9ce225b23555cd5c020519222
Submitter: "Zuul (22348)"
Branch: master

commit 8dfe02713b44f6e9ce225b23555cd5c020519222
Author: David Bastos <email address hidden>
Date: Thu Jun 27 14:58:28 2024 -0300

    Fix support for multiple namespaces in Kubevirt

    Helm allows charts to span multiple namespaces. However, the
    behavior implemented in the Application Framework was using only
    namespaces available on kustomization.yaml files to look for enabled
    charts. This strategy breaks the multiple namespace support because
    kustomize requires each HelmRelease resource to have a different
    namespace to support this scenario.

    This behavior was resolved in this review:
    https://review.opendev.org/c/starlingx/config/+/922932

    The kubevirt application needs to adapt as it makes use of namespace
    in the root kustomization.yaml which means that a single namespace
    will be used across all underlying resources, including Helm
    versions.

    The solution consists of removing the kube-system namespace from the
    root kustomization.yaml and adding the kubevirt namespace to the
    kustomization.yaml that is inside the chart.

    The "meta.helm.sh/release-namespace" annotation of the
    namespaces also needs to be changed to reference the kubevirt
    namespace.

    Test Plan:
    PASS: build-pkgs -c -p python3-k8sapp-kubevirt
    PASS: build-pkgs -c -p stx-kubevirt-app-helm
    PASS: Upload, apply, remove and delete Kubevirt
    PASS: Update the kubevirt app user overrides for the kubevirt
          namespace: "system helm-override-update kubevirt-app
          kubevirt-app kubevirt --set replicas=3". After running the
          command and reapplying the app, the changes were made
          successfully.
    PASS: Update the kubevirt app user overrides for the cdi
          namespace: "system helm-override-update kubevirt-app
          kubevirt-app cdi --set replicas=3". After running the command
          and reapplying the app, the changes were made successfully.

    Depends-on: https://review.opendev.org/c/starlingx/config/+/922932

    Closes-bug:2071380

    Change-Id: I7416e80e071b18d31b2f7d0968246365f0e3e9d1
    Signed-off-by: David Bastos <email address hidden>

Ghada Khalil (gkhalil)
Changed in starlingx:
importance: Undecided → Medium
tags: added: stx.10.0 stx.apps
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.