refactor: update kustomize manifests structure#163
Conversation
0e509fc to
47f7114
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the repository’s Kustomize deployment manifests to a manifests/kustomize base/components/overlays layout (aligned with Kubeflow Notebooks 2.0 patterns), introducing reusable components for common labels and Istio resources while updating docs and CI linting to match the new structure.
Changes:
- Migrated legacy
config/Kustomize manifests intomanifests/kustomize/{base,components,overlays}. - Added Kustomize Components for cross-cutting concerns:
common(standard labels) andistio(VirtualService, AuthorizationPolicy, sidecar injection patch). - Updated
README.mdinstall/config paths and expanded YAML linting coverage to includemanifests/.
Reviewed changes
Copilot reviewed 13 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| manifests/kustomize/overlays/kubeflow/kustomization.yaml | New Kubeflow overlay wiring base + components + overlay-specific patches/config. |
| manifests/kustomize/overlays/kubeflow/params.yaml | Kustomize transformer configuration for label application on AuthorizationPolicy selectors. |
| manifests/kustomize/overlays/kubeflow/patches/web-app-vsvc.yaml | Overlay JSON6902 patch to adapt VirtualService gateways and destination host for Kubeflow. |
| manifests/kustomize/components/istio/kustomization.yaml | Defines the Istio component resources + deployment sidecar injection patch. |
| manifests/kustomize/components/istio/virtual-service.yaml | Standalone-mode VirtualService definition (Knative ingress gateway). |
| manifests/kustomize/components/istio/authorization-policy.yaml | AuthorizationPolicy for ingressgateway access to the app. |
| manifests/kustomize/components/istio/web-app-sidecar.yaml | Strategic merge patch enabling Istio sidecar injection on the Deployment. |
| manifests/kustomize/components/common/kustomization.yaml | Common component to apply standard app.kubernetes.io/* labels broadly. |
| manifests/kustomize/base/kustomization.yaml | New base kustomization for standalone install (resources, image, configmap, labels). |
| manifests/kustomize/base/deployment.yaml | Base Deployment manifest for the models web app. |
| manifests/kustomize/base/service.yaml | Updates Service selector to the standardized component label value and fixes indentation. |
| manifests/kustomize/base/rbac.yaml | Base RBAC resources (ServiceAccount/ClusterRole/ClusterRoleBinding). |
| README.md | Updates install/config instructions to point at manifests/kustomize/.... |
| .github/workflows/linting_bash_python_yaml_files.yaml | Extends YAML lint scope to include manifests/ and reformats steps. |
| config/overlays/kubeflow/kustomization.yaml | Removed legacy overlay (migrated to manifests/kustomize/overlays/kubeflow). |
| config/overlays/kubeflow/web-app-authorization-policy.yaml | Removed legacy AuthPolicy manifest (migrated into Istio component). |
| config/base/kustomization.yaml | Removed legacy base (migrated to manifests/kustomize/base). |
| config/base/deployment.yaml | Removed legacy deployment (migrated to manifests/kustomize/base). |
|
@alokdangre i think you are missing a test. You can just copy and adapt https://github.com/kubeflow/manifests/blob/master/.github/workflows/kserve_test.yaml for now. |
45579b2 to
9d217ca
Compare
a2f285b to
4908f9c
Compare
fec83b9 to
1ee5773
Compare
|
Thanks for the PR @alokdangre! Julius suggested I take a look at this, so I did a review I did a thorough review and validated a cleaner approach to the test setup based on @juliusvonkohout's comment about pulling tests from Currently, there is a problem with the current patch script approach , The patch script mutates The cleaner approach would be to check out so -kubectl wait --for=condition=Available --timeout=300s -n kubeflow deployment/kserve-models-web-app
+kubectl wait --for=condition=Available --timeout=300s -n kubeflow deployment/kserve-models-web-applicationSince I validated this end-to-end at danish9039#1 , all tests green, no patch script required . The branch is built directly on top of your original commits. You can cherry-pick the two CI fix commits onto this branch, or @juliusvonkohout can consider merging the validated branch directly |
|
@danish9039 Thank you so much for these |
|
I will try to merge this after the Kubelfow 26.03 release so rather end of next week |
acd071d to
9f427df
Compare
|
Please rebase to master and make sure that the test triggers. |
Signed-off-by: alokdangre <alokdangre@gmail.com>
Signed-off-by: alokdangre <alokdangre@gmail.com>
…overlays Signed-off-by: alokdangre <alokdangre@gmail.com>
Signed-off-by: alokdangre <alokdangre@gmail.com>
Signed-off-by: alokdangre <alokdangre@gmail.com>
Signed-off-by: alokdangre <alokdangre@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
… 25m Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> Signed-off-by: alokdangre <alokdangre@gmail.com>
2193e69 to
1a64e84
Compare
|
Alright lets try it out and @danish9039 can change the synchronization script in kubeflow/manifests accordingly. |
…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>
…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>
) * fix: update kserve models web application for PR #163 restructure Prepares kubeflow/manifests for kserve/models-web-app PR #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 #163 HEAD SHA. Update to real release tag once kserve/models-web-app PR #163 merges. Related: kserve/models-web-app#163 Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * fix: narrow kserve test changes and restore Helm naming 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> * chore(kserve-web-app): defer helm parity follow-up Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * docs(kserve-web-app): update sync source reference Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * style(kserve-web-app): reduce yaml diff churn Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * test kserve models web app auth flow Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * revert kserve test changes Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * fix models web app test headers Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * allow ingress to kserve models web application Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * revert models web app test headers Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * align kserve downstream helpers Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> * Delete applications/kserve/Makefile Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com> --------- Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com> Co-authored-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
## Summary This revives the SSE update work from #160 and rebases it onto current `master`. The original PR had gone stale and conflicted with current repository conventions, especially the Jest/Cypress-only frontend test stack from #189 and the `manifests/kustomize/**` layout from #163. This PR preserves the SSE feature while adapting the frontend and backend changes to the current codebase. ## Changes - Adds backend SSE endpoints for InferenceServices, Kubernetes events, and logs. - Adds backend watcher/connection-manager support for shared namespace and single-resource watch streams. - Updates the frontend index, server info, and events views to consume SSE updates. - Keeps polling fallback when SSE is unavailable or errors. - Ensures SSE subscriptions are closed before falling back to polling or switching route params. - Adds/updates Jest, Cypress, and backend unit coverage for SSE behavior and cleanup. - Switches the runtime to gevent workers for long-lived streaming connections. - Preserves the current `manifests/kustomize/**` layout and does not restore stale `config/**` paths. - Keeps the current dependency lockfile state from `master`; this branch has no `frontend/package.json` or `frontend/package-lock.json` diff. ## Review follow-up Copilot review comments were addressed in `82fb3a7`: - Shared namespace and single-resource watchers are reused across clients instead of creating duplicate Kubernetes watch streams per client. - Watcher `stop()` now stops the active watch and joins the worker thread with a timeout. - The server-info status getter now handles the unloaded state without calling `getK8sObjectUiStatus` on an empty object. - `loadOwnedObjects()` now uses the correct 2-tuple type and direct destructuring. - The Cypress SSE test no longer suppresses broad `Cannot read properties` errors. - The hard-coded endpoint assertion was removed in favor of behavior-focused coverage. Screenshots were posted in #192 (comment). ## DCO and attribution This PR keeps the original PR author's commits instead of squashing them: - 10 rebased commits are authored by `Prateek Bala <prateekbala28@gmail.com>`. - Follow-up rebase/review-fix commits are authored and signed off by `danish9039 <danishsiddiqui040@gmail.com>`. ## Verification Latest GitHub checks passed on the current head: - DCO - `security/snyk (KServe)` - `license/snyk (KServe)` - Frontend `Code format and lint` - Frontend `Unit Tests` - Frontend `Mock Tests` - Bash, Python, and YAML formatting checks - OCI container publisher `test` Additional local checks: ```bash git diff --check upstream/master..HEAD python3 backend/apps/common/sse/manager_test.py ``` Earlier local verification during the rebase also passed: ```bash cd frontend && npm ci npm run format:check npm run lint-check npm run test:prod npm run test:jest -- --runInBand python3 -m compileall backend/apps/common/sse backend/apps/common ``` Supersedes #160. Closes #152. --------- Signed-off-by: Prateek Bala <prateekbala28@gmail.com> Signed-off-by: danish9039 <danishsiddiqui040@gmail.com> Co-authored-by: Prateek Bala <prateekbala28@gmail.com>
Description
Refactor the Kustomize manifest structure and labeling strategy to align with the modern standards established for Kubeflow Notebooks 2.0.
This PR migrates the legacy [config/] directory to the standard [manifests/kustomize/] hierarchy and introduces Kustomize Components to handle cross-cutting concerns like global labels and Istio configurations.
Issue : #162
Summary of Changes
app.kubernetes.io/*labels (name, part-of, managed-by) consistently.app.kubernetes.io/componentto a standardmodels-web-appvalue.VirtualService,AuthorizationPolicy, and the sidecar injection patch into a dedicated [components/istio] directory.Verification Results
I have verified the rendered output using
kubectl kustomizefor bothbaseand thekubeflowoverlay.Result: Functional resources (Deployments, Services, RBAC) remain identical in configuration. Differences are limited to the addition of standard labels and the removal of the legacy
kustomize.componentlabel.Checklist
base/components/overlayspattern.app.kubernetes.iolabels applied.