Skip to content

align kserve web app helm parity#3436

Closed
danish9039 wants to merge 13 commits into
kubeflow:masterfrom
danish9039:fix/kserve-models-web-application-pr163
Closed

align kserve web app helm parity#3436
danish9039 wants to merge 13 commits into
kubeflow:masterfrom
danish9039:fix/kserve-models-web-application-pr163

Conversation

@danish9039

Copy link
Copy Markdown
Member

This is a follow-up to #3393.

The KServe Models Web App Kustomize sync moved the downstream manifests to the new kserve-models-web-application identity, but the downstream Helm chart and Helm-vs-Kustomize parity checks were still rendering the older kserve-models-web-app names and behavior. In practice that left the experimental chart out of sync with the repo source of truth and kept the parity comparison special-cased for KServe.

The root cause was a mix of stale Helm metadata and compare-tool assumptions. The chart still advertised the older app version and image tag, helper-derived labels still rendered the pre-rename component identity, the RBAC rules were missing the newer inferencegraphs permissions, and the parity script still handled KServe with inline --set flags instead of chart-local scenario values.

This change aligns the chart with the synced manifests by updating the rendered identity to kserve-models-web-application, bumping the chart app version and image tag to v0.16.1 and 0.16.1, and adding the missing RBAC resources. It also adds chart-local CI values files for the base and Kubeflow scenarios and updates the comparison tooling to render KServe through those files with the new release identity.

With these changes the KServe Helm parity checks now pass for both supported scenarios without keeping an extra expected Helm-only resource for the base case.

Verification used:

  • bash -n tests/helm_kustomize_compare.sh
  • tests/helm_kustomize_compare.sh kserve-models-web-app base
  • tests/helm_kustomize_compare.sh kserve-models-web-app kubeflow

danish9039 and others added 13 commits March 26, 2026 05:03
…ture

Prepares kubeflow/manifests for kserve/models-web-app PR kubeflow#163 which:
- Moves manifests from config/ to manifests/kustomize/
- Adds components/ layer for Istio and common labels
- Renames deployment: kserve-models-web-app -> kserve-models-web-application

Changes:
- Sync new manifests/kustomize/ structure into applications/kserve/models-web-app/
- Rename all resources, labels, selectors to kserve-models-web-application
- Update tests/kserve_install.sh deployment wait
- Update tests/kserve_test.sh Test 3: port-forward to new service,
  kubeflow-userid auth headers, retry bootstrap loop
- Update .github/workflows/kserve_models_web_application_test.yaml
- Update Helm chart parity for the renamed manifests and Kubeflow overlay
- Update Chart.yaml appVersion and values.yaml imageTag to 0.16.0
- Update tests/helm_kustomize_compare.py expectations
- Update sync script SOURCE_MANIFESTS_PATH and COMMIT placeholder

NOTE: COMMIT=195cabdf is a placeholder for PR kubeflow#163 HEAD SHA.
Update to real release tag once kserve/models-web-app PR kubeflow#163 merges.

Related: kserve/models-web-app#163
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
tests/kserve_test.sh: revert the port-forward, auth headers, retry loop,
and extra XSRF validation so the file returns to upstream behavior with only
the deployment rename. This matches the green fork validation where Test 3
passed through the standard gateway path.

experimental/helm/charts/kserve-models-web-app:
- Chart.yaml: rename chart name to kserve-models-web-application
- templates/_helpers.tpl: restore standard name/fullname/chart helpers that
  derive from .Chart.Name

To keep the repo's parity gate consistent with the new chart name,
 tests/helm_kustomize_compare.sh now renders the KServe chart with Helm
 release name kserve-models-web-application for both scenarios.

Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Copilot AI review requested due to automatic review settings April 3, 2026 19:37
@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kimwnasptd for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

github-actions Bot commented Apr 3, 2026

Copy link
Copy Markdown

Welcome to the Kubeflow Manifests Repository

Thanks for opening your first PR. Your contribution means a lot to the Kubeflow community.

Before making more PRs:
Please ensure your PR follows our Contributing Guide.
Please also be aware that many components are synchronizes from upstream via the scripts in /scripts.
So in some cases you have to fix the problem in the upstream repositories first, but you can use a PR against kubeflow/manifests to test the platform integration.

Community Resources:

Thanks again for helping to improve Kubeflow.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the KServe Models Web App Helm chart with the synchronized Kustomize manifests, addressing inconsistencies between the experimental chart and the repository's source of truth.

Changes:

  • Renamed all resources from kserve-models-web-app to kserve-models-web-application across Kustomize manifests, Helm charts, and test scripts
  • Updated the Helm chart metadata (Chart.yaml) with the new chart name and updated app/image versions to v0.16.1
  • Added missing RBAC permissions for inferencegraphs resources in the cluster role
  • Reorganized Kustomize structure by moving Istio resources (VirtualService and AuthorizationPolicy) to a dedicated components/istio component
  • Created chart-local CI values files (base-values.yaml and kubeflow-values.yaml) for the Helm chart comparison tests
  • Updated the helm_kustomize_compare.sh script to use the new CI values files instead of inline --set flags
  • Removed the obsolete applications/kserve/Makefile
  • Updated test scripts and GitHub Actions workflow to reference the new deployment name

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/kserve_test.sh Updated deployment name to kserve-models-web-application and changed timeout from 300s to 10s
tests/kserve_install.sh Simplified retry logic and updated deployment name reference
tests/helm_kustomize_compare.sh Changed to use chart-local CI values files for KServe instead of inline --set flags
tests/helm_kustomize_compare.py Removed special case handling for KServe base scenario
experimental/helm/charts/kserve-models-web-app/ Updated Chart.yaml (name, appVersion), values.yaml (comment, imageTag), clusterrole.yaml (added inferencegraphs), _helpers.tpl (updated selector labels), and added new CI values files
applications/kserve/models-web-app/ Renamed all resources to kserve-models-web-application, reorganized Istio resources into components, and updated all references
common/kubeflow-namespace/base/kubeflow/ Renamed network policy to match new identity
.github/workflows/kserve_models_web_application_test.yaml Updated deployment name references
scripts/synchronize-kserve-web-application-manifests.sh Updated commit hash and source manifest path
README.md Updated KServe Models Web Application source reference to the new commit hash and path

Comment thread tests/kserve_test.sh
# Test 3: KServe Models Web Application API
# ============================================================
kubectl wait --for=condition=Available --timeout=300s -n kubeflow deployment/kserve-models-web-app
kubectl wait --for=condition=Available --timeout=10s -n kubeflow deployment/kserve-models-web-application

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout for waiting on the kserve-models-web-application deployment has been reduced from 300s to 10s. Although the install script already waits for pods to be ready with a 600s timeout before this point, a 10s timeout for the deployment's Available condition could be too strict and cause test flakiness. Consider using a higher timeout like 30-60 seconds to provide a more reasonable safety margin.

Suggested change
kubectl wait --for=condition=Available --timeout=10s -n kubeflow deployment/kserve-models-web-application
kubectl wait --for=condition=Available --timeout=60s -n kubeflow deployment/kserve-models-web-application

Copilot uses AI. Check for mistakes.
@danish9039

Copy link
Copy Markdown
Member Author

Closing this replacement in favor of the clean follow-up branch PR with only the Helm parity changes.

@danish9039 danish9039 closed this Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants