Some helm charts can override other helm charts (eg: Ironic & OVS)

Bug #1833746 reported by Frank Miller
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
StarlingX
Fix Released
High
Bob Church

Bug Description

Brief Description
-----------------
The current implementation of using meta overrides with helm charts is not able to handle multiple optional charts. This is leading to scenarios where one chart is overriding another chart. For example if ironic is enabled and OVS enabled with vswitch type None, this will cause Ironic to not be loaded.

Severity
--------
Major.

Steps to Reproduce
------------------
Enable multiple optional helm charts/services

Expected Behavior
------------------
All services are enabled

Actual Behavior
----------------
Some services are not enabled

Reproducibility
---------------
Reproducible

System Configuration
--------------------
All configs

Branch/Pull Time/Commit
-----------------------
Master branch; ISO: 20190619T140326

Last Pass
---------
Never

Timestamp/Logs
--------------
n/a

Test Activity
-------------
Code reviews

Revision history for this message
Ghada Khalil (gkhalil) wrote :

Marking as stx.2.0; framework limitation resulting in incorrect pod creation.

tags: added: stx.2.0 stx.containers
Changed in starlingx:
importance: Undecided → High
status: New → Triaged
assignee: nobody → Bob Church (rchurch)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to config (master)

Fix proposed to branch: master
Review: https://review.opendev.org/668112

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

Reviewed: https://review.opendev.org/668112
Committed: https://git.openstack.org/cgit/starlingx/config/commit/?id=e6b177eb93f85b5a4e53242214060c97728e2048
Submitter: Zuul
Branch: master

commit e6b177eb93f85b5a4e53242214060c97728e2048
Author: Robert Church <email address hidden>
Date: Fri Jun 28 03:09:18 2019 -0400

    Introduce ArmadaManifestOperator

    Add the ArmadaManifestOperator to provide a single point of manipulation
    for an application manifest.

    This operator will load the original manifest file and provide a set of
    utilily methods to update the list of chart_groups, an individual chart
    group, or an individual chart based on platform and plugin conditions.
    Based on the methods that are executed, changes are tracked and
    overrides are generated for those elements that have changed.

    These armada overrides are fed to the Armada command in addition to the
    Helm overrides.

    This also includes updated changes originally proposed from
    I4bf4dcc3900c65f1492a362d31c9b3e7d73b8e31 to allow Barbican and
    Telemetry chart groups to be optional services. This is utilizing a k8s
    label to enable the services. This mechanism will be re-visited in a
    future commit, but for now continue to use the existing mechanism.

    Change-Id: I7e0c5bbe7daa0b0b8365ca1ed5f8fdf11a348c62
    Partial-Bug: #1833746
    Signed-off-by: Robert Church <email address hidden>

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to root (master)

Fix proposed to branch: master
Review: https://review.opendev.org/671949

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to config (master)

Fix proposed to branch: master
Review: https://review.opendev.org/671950

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to root (master)

Reviewed: https://review.opendev.org/671949
Committed: https://git.openstack.org/cgit/starlingx/root/commit/?id=c8e076be39f9daa9804d74ff4d1745253048f377
Submitter: Zuul
Branch: master

commit c8e076be39f9daa9804d74ff4d1745253048f377
Author: Robert Church <email address hidden>
Date: Sun Jul 21 18:48:47 2019 -0400

    Add pre-existing metadata file support when building app tarballs

    If an application RPM contains a metadata file, copy it into the staging
    area and use it as a baseline for further modifications.

    This enables specifying 'disabled_charts' metadata to allow specific
    charts to be disabled automatically on application upload

    Change-Id: I418f0fe4978946a44e512c3025817fb27216c078
    Partial-Bug: #1833746
    Signed-off-by: Robert Church <email address hidden>

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to config (master)

Reviewed: https://review.opendev.org/671950
Committed: https://git.openstack.org/cgit/starlingx/config/commit/?id=3cd4032f55f5f52317ffd9be84c30ffcb9a1da06
Submitter: Zuul
Branch: master

commit 3cd4032f55f5f52317ffd9be84c30ffcb9a1da06
Author: Robert Church <email address hidden>
Date: Sun Jul 21 17:41:24 2019 -0400

    Provide an API to control enabling/disabling application charts

    Extend the helm_charts API to support an enable attribute. This
    attribute is set on application upload and stored in the existing
    system_overrides element of the helm_overrides table.

    Changes include
    - Add application metadata support for disabling charts on application
      upload.
    - Add the system helm-chart-attribute-modify command to allow enabling
      and disabling charts from the command-line. This removes the current
      implementation of adding a faux label via the system host-label-assign
      command to enable and disable charts.
    - Add a --long option to helm-override-list to enable easy viewing of
      what charts are enabled for a given application
    - Enhance the ArmadaManifestOperator to make this a base class for
      application specific operator classes. Introduce classes for the
      stx-openstack and platform-integ-apps manifests with specific
      knowledge of the charts and chart groups within each class.
    - Use stevedore to load the application specific manifest operators.
      This will allow future packaging of manifest operators with new
      application tarballs.
    - Move the helm chart definition from the common/constants.py to
      helm/common.py. This limits helm/armada specific data leakage outside
      of the helm directory, which we may carve out of sysinv in the future.
    - Clean up the code related to the faux labels: LABEL_IRONIC,
      LABEL_BARBICAN, and LABEL_TELEMETRY
    - Rework the manifest update code in the plugins to include checks for
      if the chart for a given application has been disabled.

    Change-Id: If284f622ceac48c4ffd74e7022fdd390971d0fd8
    Closes-Bug: #1833746
    Depends-On: I418f0fe4978946a44e512c3025817fb27216c078
    Signed-off-by: Robert Church <email address hidden>

Changed in starlingx:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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