kserve: move platform resources to kserve namespace - #3515
Conversation
|
Welcome to the Kubeflow Community Distribution Repository Thanks for opening your first PR. Your contribution means a lot to the Kubeflow community. Before making more PRs: Community Resources:
Thanks again for helping to improve Kubeflow. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This pull request moves KServe control-plane resources from the kubeflow namespace to the upstream kserve namespace while preserving Kubeflow-facing behavior (notably keeping the KServe Models Web Application in kubeflow).
Changes:
- Add and manage
Namespace/kserve, and update the KServe installation kustomization to install control-plane resources there (including webhook NetworkPolicy coverage). - Update installation and continuous integration scripts to wait for and collect diagnostics from the
kservenamespace. - Document upgrade cleanup steps to remove legacy
kubeflow-scoped KServe control-plane resources.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/PSS_enable.sh |
Includes kserve in Pod Security Standards enforcement labeling. |
tests/kserve_install.sh |
Updates waits and diagnostics to reflect KServe control-plane deployment in kserve. |
README.md |
Documents the new namespace split and adds upgrade cleanup instructions. |
common/kubeflow-namespace/base/kubeflow/kustomization.yaml |
Stops applying the old kubeflow-scoped KServe NetworkPolicy. |
common/kubeflow-namespace/base/kubeflow/kserve.yaml |
Removes the old kubeflow-scoped KServe webhook NetworkPolicy manifest. |
applications/kserve/UPGRADE.md |
Adds upgrade guidance for cleaning up legacy kubeflow-scoped KServe control-plane resources. |
applications/kserve/kserve/namespace.yaml |
Introduces managed Namespace/kserve with labels for control-plane and Pod Security Standards. |
applications/kserve/kserve/kustomization.yaml |
Installs upstream KServe resources into kserve, adds aggregated roles and kserve-scoped webhook NetworkPolicy, and updates local patches accordingly. |
applications/kserve/kserve/kserve-networkpolicy.yaml |
Adds kserve-scoped NetworkPolicy allowing admission webhook ingress on port 9443. |
applications/kserve/kserve/aggregated-roles.yaml |
Expands aggregated Kubeflow KServe roles to include additional KServe resources. |
.github/workflows/kserve_test.yaml |
Adds common/kubeflow-namespace/** trigger and includes kserve namespace in diagnostics capture. |
.github/workflows/kserve_models_web_application_test.yaml |
Adds common/kubeflow-namespace/** trigger. |
.github/workflows/full_kubeflow_integration_test.yaml |
Includes kserve namespace in diagnostics capture. |
a41eace to
f635136
Compare
6961ac0 to
48761fc
Compare
48761fc to
6b61122
Compare
6b61122 to
fb1b194
Compare
|
/retest |
1 similar comment
|
/retest |
|
@juliusvonkohout for review |
|
/retest |
Move the KServe control plane and Models Web Application into the kserve namespace while preserving user InferenceService resources in Profile namespaces and routing through the Kubeflow gateway. Align namespace foundation ownership, workload-specific Istio injection, NetworkPolicies, the existing Models Web Application chart, synchronization behavior, installation tests, and upgrade cleanup guidance. Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
The manifest test asserted properties that the live KinD run in tests/kserve_test.sh already proves, and asserted documentation prose and shell source text that no rendered resource depends on. Remove the assertions covered by the cluster run: the Models Web Application route, which Test 3 proves by requesting /kserve-endpoints, and the inferenceservice-config gateway and path template, which Test 2a proves by path-based routing. Remove the assertions on prose and source text. The root README upgrade steps are version-specific and are rewritten each release, so pinning their wording fails an unrelated release-notes edit. The kustomization comment assertions only prevent removed comments from being retyped. Keep the checks a green cluster run cannot make: resources left behind in kubeflow, sidecar injection selecting the Models Web Application through a pod label rather than an annotation, the storage initializer security context that no restricted namespace exercises in CI, namespace fields on cluster-scoped resources that the API server silently drops, the disabled local model node agent whose rejected pods create no pods to wait for, and the webhook certificate and authority references for the llmisvc and localmodel chains that no test calls. Document on each remaining test why the cluster run cannot catch it. Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
…n test The installation section still told operators the Models Web Application remains in the kubeflow namespace, which contradicts both the rendering and the upgrade guidance in the same document. State that the control plane and the Models Web Application are both in kserve, that user InferenceService resources stay in the profile namespaces, and that the Models Web Application is still reached through the Kubeflow gateway. Assert the exact namespace of every namespaced resource rather than the absence of the old one. A resource that declares no namespace is applied into whichever namespace the caller is using, which is default for the installation scripts, so a check for "not kubeflow" stayed green for a resource that had silently lost its namespace. The check now covers the control plane and the Models Web Application through one shared rule for cluster scope, which the cluster scope test reuses. Watch applications/kserve/kserve in the comparison workflow. That rendering became an input to the namespace parity check, but the workflow did not watch it, so a change to it alone would skip the comparison. Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
d43a56d to
7e95d7d
Compare
|
/retest |
|
@juliusvonkohout Addressed the Copilot review comments |
|
@juliusvonkohout ready to merge |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/kserve_manifest_test.py:229
- This filtering only validates resources that already have
cert-manager.io/inject-ca-from;self.assertTrue(annotated_resources)proves only that one annotation remains. Removing the annotation from any individual conversion CRD or webhook configuration therefore leaves this test green, although the test documentation states that it guards every certificate chain. Compare the annotated resource identities with the expected set, or assert the annotation while traversing every webhook-backed resource.
tests/kserve_manifest_test.py:223 all(...)is true for an emptydnsNameslist. If a future synchronization removes the DNS names from the local-model or LLM certificate, this regression test remains green even though the certificate cannot match its webhook service. Assert that the list is nonempty before checking every suffix.
This issue also appears on line 225 of the same file.
tests/kserve_manifest_test.py:151
- The Profile controller Pod Security overlay currently enforces
baseline, notrestricted(applications/dashboard/upstream/profile-controller/overlays/kubeflow-pss/namespace-labels-kubeflow-pss.yaml:19-20). This security-policy statement is therefore inaccurate; describe restricted enforcement as an installation choice rather than the repository default.
| drop: | ||
| - ALL | ||
| runAsNonRoot: true | ||
| runAsUser: 1000 |
There was a problem hiding this comment.
| runAsUser: 1000 |
| ## Upgrade Cleanup | ||
|
|
||
| When upgrading from a version that installed KServe control-plane resources in | ||
| the `kubeflow` namespace, plan for a short control-plane interruption. Do not | ||
| delete KServe CRDs, user `InferenceService` objects, or profile namespaces. | ||
|
|
||
| The standard upstream bundle also uses the current upstream workload labels. | ||
| Update custom monitoring and policies that select the legacy `app: kserve` pod | ||
| label to select the `control-plane` label instead. | ||
|
|
||
| First remove the unused cluster-scoped authorization for the disabled local | ||
| model node agent: | ||
|
|
||
| ```sh | ||
| kubectl delete --ignore-not-found \ | ||
| clusterrolebinding/kserve-localmodelnode-agent-rolebinding \ | ||
| clusterrole/kserve-localmodelnode-agent-role | ||
| ``` | ||
|
|
||
| Before applying the new manifests, scale the old controllers to zero. | ||
| Controllers in different namespaces use different leader-election Leases and | ||
| must not run concurrently: | ||
|
|
||
| ```sh | ||
| kubectl scale -n kubeflow --replicas=0 \ | ||
| deployment/kserve-controller-manager \ | ||
| deployment/kserve-localmodel-controller-manager \ | ||
| deployment/llmisvc-controller-manager | ||
| ``` | ||
|
|
||
| Remove the old Models Web Application route before applying the new manifests | ||
| so the old and new VirtualServices do not claim `/kserve-endpoints/` | ||
| concurrently. Then remove the remaining old namespaced resources: | ||
|
|
||
| ```sh | ||
| kubectl delete -n kubeflow --ignore-not-found \ | ||
| virtualservice.networking.istio.io/kserve-models-web-application | ||
| kubectl delete -n kubeflow --ignore-not-found \ | ||
| serviceaccount/kserve-models-web-application \ | ||
| configmap/kserve-models-web-application-config \ | ||
| service/kserve-models-web-application \ | ||
| deployment/kserve-models-web-application \ | ||
| authorizationpolicy.security.istio.io/kserve-models-web-application \ | ||
| networkpolicy.networking.k8s.io/kserve-models-web-application | ||
| ``` | ||
|
|
||
| Apply the new release manifests using the normal upgrade procedure. Then verify | ||
| the new KServe controllers, Models Web Application, certificates, and webhook | ||
| endpoints in the `kserve` namespace: | ||
|
|
||
| ```sh | ||
| kubectl rollout status -n kserve --timeout=120s \ | ||
| deployment/kserve-controller-manager \ | ||
| deployment/kserve-localmodel-controller-manager \ | ||
| deployment/llmisvc-controller-manager \ | ||
| deployment/kserve-models-web-application | ||
| kubectl wait -n kserve --for=condition=Ready --timeout=120s \ | ||
| certificate.cert-manager.io/serving-cert \ | ||
| certificate.cert-manager.io/llmisvc-serving-cert \ | ||
| certificate.cert-manager.io/localmodel-serving-cert | ||
| kubectl get endpoints -n kserve \ | ||
| kserve-webhook-server-service \ | ||
| llmisvc-webhook-server-service \ | ||
| localmodel-webhook-server-service | ||
| ``` | ||
|
|
||
| After the new control plane is ready, delete the old `kubeflow`-scoped | ||
| resources: | ||
|
|
||
| ```sh | ||
| kubectl delete -n kubeflow --ignore-not-found \ | ||
| serviceaccount/kserve-controller-manager \ | ||
| serviceaccount/kserve-localmodel-controller-manager \ | ||
| serviceaccount/kserve-localmodelnode-agent \ | ||
| serviceaccount/llmisvc-controller-manager \ | ||
| networkpolicy.networking.k8s.io/kserve \ | ||
| role.rbac.authorization.k8s.io/kserve-leader-election-role \ | ||
| role.rbac.authorization.k8s.io/llmisvc-leader-election-role \ | ||
| rolebinding.rbac.authorization.k8s.io/kserve-leader-election-rolebinding \ | ||
| rolebinding.rbac.authorization.k8s.io/llmisvc-leader-election-rolebinding \ | ||
| configmap/inferenceservice-config \ | ||
| secret/kserve-webhook-server-secret \ | ||
| secret/kserve-webhook-server-cert \ | ||
| secret/llmisvc-webhook-server-cert \ | ||
| secret/localmodel-webhook-server-cert \ | ||
| service/kserve-controller-manager-metrics-service \ | ||
| service/kserve-controller-manager-service \ | ||
| service/kserve-webhook-server-service \ | ||
| service/llmisvc-controller-manager-service \ | ||
| service/llmisvc-webhook-server-service \ | ||
| service/localmodel-webhook-server-service \ | ||
| deployment.apps/kserve-controller-manager \ | ||
| deployment.apps/kserve-localmodel-controller-manager \ | ||
| deployment.apps/llmisvc-controller-manager \ | ||
| daemonset.apps/kserve-localmodelnode-agent \ | ||
| lease.coordination.k8s.io/kserve-controller-manager-leader-lock \ | ||
| lease.coordination.k8s.io/llminferenceservice-kserve-controller-manager \ | ||
| certificate.cert-manager.io/serving-cert \ | ||
| certificate.cert-manager.io/llmisvc-serving-cert \ | ||
| certificate.cert-manager.io/localmodel-serving-cert \ | ||
| issuer.cert-manager.io/selfsigned-issuer | ||
| ``` |
There was a problem hiding this comment.
Why are these notes duplicated here from the main README.md? 🤔 I think they can be dropped here.
Also I think such notes should always be versioned, otherwise it's unsure on which upgrades they apply 😅
There was a problem hiding this comment.
Yes please remove duplicates
There was a problem hiding this comment.
Where are those roles coming from and why do we need to introduce them? 🤔 AFAIU the upstream overlay already creates rules like this in the kubeflow overlay 😅
There was a problem hiding this comment.
The kubeflow overlay is not that well maintained. This is why we wanted to switch to the regular one, also in light of the helm efforts.
|
Please check #3515 (review) |
|
Please rebase after #3576 |
Summary of Changes
This follow-up moves KServe-owned platform resources from
kubeflowto theupstream
kservenamespace.kserve, includingcontrollers, webhooks, certificates, namespaced RBAC, and
inferenceservice-config.kservewhile preserving routing through
kubeflow/kubeflow-gateway.InferenceServiceresources and serving workloads in Profilenamespaces.
Namespace/kserve;add equivalent namespace ownership and restricted Pod Security labels to the
kubeflow-namespacesfoundation chart.while retaining explicit injection for the Models Web Application.
incompatible local model node agent.
version handling, and upgrade cleanup guidance.
Ownership Boundaries
The component Kustomization owns
Namespace/kservefor direct Kustomizeinstallation. Helm component charts do not create namespaces; the
kubeflow-namespacesfoundation chart ownsNamespace/kservefor Helminstallation.
This pull request only aligns the existing Models Web Application chart with
the relocation. Generated KServe control-plane charts and their synchronization
pipeline remain separate future Helm work.
Upgrade Notes
Controllers in the old and new namespaces use different leader-election
Leases, so the old
kubeflowcontrollers must be scaled to zero before the newcontrollers start. The old Models Web Application route and namespaced
resources must also be removed to prevent duplicate routing and stale
resources.
The documented cleanup does not delete KServe CRDs, user
InferenceServiceobjects, Profile namespaces, or model-serving workloads. Existing Helm
installations must uninstall the Models Web Application release from
kubeflowand install a new release inkserve, because Helm cannot move arelease between namespaces.
Related Issues
Follow-up to #3505 and the namespace recommendation in the pull request
discussion.
Contributor Checklist
Verification
python3 tests/kserve_manifest_test.pygit diff --check upstream/master...HEADshared namespaces, and complete example
kubeconformvalidation of both KServe renderingstests/helm_kustomize_compare_all.sh allparity matrixNetworkPolicies, Pod Security, Istio injection, upgrade cleanup, and Helm
ownership
Not run locally: live-cluster admission, runtime, and in-place migration tests
because no active Kubernetes context was available.
Related Issues
Tracking issue:
#3561