Helm checks are mandatory for Manifests update, making PR fail.

[tarilabs]

Validation Checklist

• I confirm that this is a Kubeflow-related issue.
• I am reporting this in the appropriate repository.
• I have followed the Kubeflow installation guidelines.
• The issue report is detailed and includes version numbers where applicable.
• I have considered adding my company to the adopters page to support Kubeflow and help the community, since I expect help from the community for my issue (see 1. and 2.).
• This issue pertains to Kubeflow development.
• I am available to work on this issue.
• You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

Version

master

Detailed Description

When I use any Synchronization script on /scripts as requested by Manifest, these do not update the helm packages but only the Manifest, as intended

On the PR, Helm checks are MANDATORY

This makes periodic update FAIL.

Kindly consider either:

• making the Helm gi/GHA OPTIONAL
• updating WG leads instruction on how to update Manifests in this repo

Steps to Reproduce

1. ./scripts/synchronize-model-registry-manifests.sh
2. open PR

running the required instructions to update Manifests, as above, make the PR fail. This is the second time so I can confirm the behaviour is reproducible and present.

Screenshots or Videos (Optional)

Thank you for your consideration.

[tarilabs]

I think we have to make the Helm charts non-blocking or Kunal needs to extend the scripts for helm.

from: #3216 (comment)

to which I concur.

[juliusvonkohout]

@kunal-511 please let's switch the helm test to continue on error to not block the ci/cd until the helm charts are up streamed.

[kunal-511]

@juliusvonkohout Should I make the PR against the #3231 branch or a fresh PR?

[juliusvonkohout]

@juliusvonkohout Should I make the PR against the #3231 branch or a fresh PR?

Both. A separate one to unblock in the future if there are failing helm tests and a second PR to update the helm according to Matteo's customize changes.

[tarilabs]

A separate one to unblock in the future if there are failing helm tests

I'm happy to rebase #3231 on top of #3234 when merged 👍

[tarilabs]

btw @juliusvonkohout I'm curious as to why the ci/GHA workflow on master :
https://github.com/kubeflow/manifests/blob/253043e9c7745058eead4d92405dfaa9802b5c9e/.github/workflows/helm-kustomize-comparison.yml#L46

is specifically only testing "model-registry" 🤔

[kunal-511]

btw @juliusvonkohout I'm curious as to why the ci/GHA workflow on master :

manifests/.github/workflows/helm-kustomize-comparison.yml

Line 46 in 253043e

     ./tests/helm_kustomize_compare_all.sh model-registry 

is specifically only testing "model-registry" 🤔

It was suppose to test none of the kustomize manifests

[kunal-511]

A separate one to unblock in the future if there are failing helm tests

I'm happy to rebase #3231 on top of #3234 when merged 👍

I have raised the PR #3 against the branch of #3231
which updates the helm charts according to the model registry v0.3.0