helm-upload sha check is not effective plus incorrect logic

[Kevin Smith]

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

There are a couple issues with the changes made below.

1. The change will compare an sha of a chart to be uploaded vs one already in the repo. These are just tarballs being compared, not the files inside. The more important thing to check is that the files inside are the same, not the tarball, as the sha will be different for the same exact chart contents every time a "helm package" is run to produce a tarball (2 tarballs can have identical contents with different shas). In effect, this will erroneously prevent an application-update with helm charts of the same version and same exact helm chart contents, though different helm chart sha, from proceeding correctly.

2. The Try/Except block in function _upload_helm_charts on line 912 is done over a loop of uploading all application helm charts. If the first helm chart being upload hits the ""Chart X already exists in the Y repository. Skipping upload." case for example, the application-update will proceed on its merry way without having loaded all the helm charts. If this one particular error/warning is survivable it should to be checked inside the "for chart in charts" loop.

https://review.opendev.org/c/starlingx/integ/+/896870/9/kubernetes/helm/centos/files/helm-upload#52

https://opendev.org/starlingx/config/commit/120df4573d461ab40b4845bb74178ff6304a429b

Severity
--------

Major: System/Feature is usable but degraded

Steps to Reproduce
------------------
Application-update to a tarball with the same helm chart versions but different shas (same contents) Application-update will fail.

Expected Behavior
------------------
Application-update passes

Actual Behavior
----------------
Application-update fails

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

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

Branch/Pull Time/Commit
-----------------------
See commits mentioned.

Last Pass
---------
Did this test scenario pass previously? If so, please indicate the load/pull time info of the last pass.
Use this section to also indicate if this is a new test scenario.

Timestamp/Logs
--------------
N/A

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

Workaround
----------
Upversion all helm charts of an application for every application-update, though this will lengthen the time taken for the application-update to complete as some helm releases will be upgraded needlessly.

[Kevin Smith (kevin.smith.wrs)]

I guess this is a known issue wrt helm package: https://github.com/helm/helm/issues/3612 Still an issue. Until this is solved, we can't rely on shas to determine whether 2 helm tarballs are the same.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/starlingx/integ/+/912305
Committed: https://opendev.org/starlingx/integ/commit/61f9198a3e19ecb45a421aac28a8b729e954210d
Submitter: "Zuul (22348)"
Branch: master

commit 61f9198a3e19ecb45a421aac28a8b729e954210d
Author: Igor Soares <email address hidden>
Date: Fri Mar 8 17:38:44 2024 -0300

    Use diff tool to differentiate charts

    This commit changes how we differentiate Helm charts when uploading new
    StarlingX applications. The method previously used was based on
    comparing SHA256 digests, which was causing the helm-upload script to
    mistakenly report charts with the same implementation as different
    after rebuilding them with no changes.

    The new implementation uses the diff tool to perform such comparison so
    that charts with the same implementation are reported as equals
    regardless of whether they were rebuilt.

    In addition, two new parameters were added to the helm-upload script:
        * 'check-only': check if charts are valid without uploading them to
                         the given repository.
        * 'upload-only': upload charts to the given repository bypassing the
                         preliminary checks.

    The new parameters aim for more flexibility when integrating with other
    pieces of software such as sysinv.

    Test Plan:
    PASS: build-pkgs -a && build-image
    PASS: AIO-SX fresh install.
    PASS: Update platform-integ-apps containing a rebuilt version of
          ceph-pools-audit with no changes.
          Confirm that the app was successfully updated.
    PASS: Update platform-integ-apps containing a rebuilt version of
          ceph-pools-audit containing changes to values.yaml but keeping the
          same version number.
          Confirm that the app update failed.
    PASS: Run helm-upload with the 'check-only' parameter and confirm that
          no charts were uploaded.
    PASS: Run helm-upload with the 'upload-only' parameter and confirm that
          charts were correctly uploaded.
    PASS: Run helm-upload without the new parameters and confirm that the
          original behavior was preserved.

    Partial-Bug: 2053074

    Change-Id: I45f6482118f5ecf9da1b51f21fbaf0db63eb321c
    Signed-off-by: Igor Soares <email address hidden>

[OpenStack Infra (hudson-openstack)]

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

commit b1b160f48bab21af56b617a911218877cde13550
Author: Igor Soares <email address hidden>
Date: Tue Mar 19 17:29:11 2024 -0300

    Fix charts upload when there are existing ones

    This fixes a bug that prevents StarlingX application charts from
    being uploaded to the helm repository when one or more of them have been
    uploaded before.

    The charts upload logic was changed to check if all charts provided by
    the given application are valid prior to uploading. If a chart is
    invalid then no charts for that application will be uploaded, since the
    upload process cannot proceed in that scenario.

    Test Plan:
    PASS: build-pkgs -a && build-image
    PASS: AIO-SX fresh install
    PASS: Build a platform-integ-apps version containing one existing chart
          and two nonexistent charts in the local Helm repository.
          Update platform-integ-apps to the built version.
          Confirm that the existing chart was not re-uploaded and that the
          nonexistent ones were correctly uploaded to the Helm repository.
    PASS: Apply/remove/delete platform-integ-apps

    Closes-Bug: 2053074
    Depends-on: https://review.opendev.org/c/starlingx/integ/+/912305

    Change-Id: I155d457f58be1986cc6f25178929aedfbe1d0693
    Signed-off-by: Igor Soares <email address hidden>