few updates in datastore #713

nirrozenbaum · 2025-04-19T18:53:14Z

this PR does the following changes:

in podutil package, changed IsPodReady function to check if pod is marked for deletion. up until today storing pods logic in datastore was inconsistent in a way that pod_reconciler checked this condition, but if there was a change in InfPool that triggered podResync (i.e label selector change), the deletion timestamp was not checked, which may lead to adding pods that are marked for deletion to the datastore. IsPodReady function in GIE serves for the purpose of answering the question - "can this pod serve requests" and the functionality should be consistent whether pod is updated or pool selector is updated.
remove unused pool arg from PodUpdateOrAddIfNotExist function in datastore. updated all calls accordingly.
Upon setting the pool in datastore there is a need to resync the pods in case the label selector has changed. this is a common behavior that should be checked every time PoolSet is called (not specific behavior to inf pool reconciler file, but can be used also in tests). for this purpose, the functionality was moved from InfPool reconciler to datastore. Additionally if PoolSet is called with nil pointer, this triggers clearing the datastore.
according to the above point, podResync has changed to be internal function and removed from ds interface.

Signed-off-by: Nir Rozenbaum <[email protected]>

netlify · 2025-04-19T18:53:36Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`57b0b91`
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/6804005b46d33e0008ffa61b
😎 Deploy Preview	https://deploy-preview-713--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum · 2025-04-19T18:56:48Z

cc @kfswain

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum · 2025-04-19T19:00:35Z

pkg/epp/datastore/datastore.go

-		if podutil.IsPodReady(&pod) {
-			namespacedName := types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}
-			activePods[pod.Name] = true
-			if ds.PodUpdateOrAddIfNotExist(&pod, pool) {
-				logger.V(logutil.DEFAULT).Info("Pod added", "name", namespacedName)
-			} else {
-				logger.V(logutil.DEFAULT).Info("Pod already exists", "name", namespacedName)
-			}
+		if !podutil.IsPodReady(&pod) {
+			continue
+		}
+		namespacedName := types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}
+		activePods[pod.Name] = true
+		if ds.PodUpdateOrAddIfNotExist(&pod, ds.pool) {
+			logger.V(logutil.DEFAULT).Info("Pod added", "name", namespacedName)
+		} else {
+			logger.V(logutil.DEFAULT).Info("Pod already exists", "name", namespacedName)
 		}


functionality hasn't changed, just checking !podutil.IsPodReady(&pod) instead of podutil.IsPodReady(&pod) to reduce identation.

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum · 2025-04-19T19:37:35Z

pkg/epp/datastore/datastore.go

@@ -60,10 +64,9 @@ type Datastore interface {
 	// PodGetAll returns all pods and metrics, including fresh and stale.
 	PodGetAll() []backendmetrics.PodMetrics
 	// PodList lists pods matching the given predicate.
-	PodList(func(backendmetrics.PodMetrics) bool) []backendmetrics.PodMetrics
-	PodUpdateOrAddIfNotExist(pod *corev1.Pod, pool *v1alpha2.InferencePool) bool
+	PodList(predicate func(backendmetrics.PodMetrics) bool) []backendmetrics.PodMetrics


added arg name to be consistent with the other functions in the interface

Signed-off-by: Nir Rozenbaum <[email protected]>

kfswain · 2025-04-23T01:12:56Z

/lgtm
/approve

k8s-ci-robot · 2025-04-23T01:13:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kfswain, nirrozenbaum

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kfswain]

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

* few updates in datastore Signed-off-by: Nir Rozenbaum <[email protected]> * PoolSet documentation Signed-off-by: Nir Rozenbaum <[email protected]> * error phrasing Signed-off-by: Nir Rozenbaum <[email protected]> * removed unused pool arg from PodUpdateOrAddIfNotExist Signed-off-by: Nir Rozenbaum <[email protected]> * linter Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]>

* Add unit test coverage for pod APIs under datastore/pkg * Add unit test coverage for pod APIs under datastore/pkg * Add unit test coverage for pod APIs under datastore/pkg * Add unit test coverage for pod APIs under datastore/pkg * EPP Architecture proposal (#683) * initial changes * Adding to proposal to give a quick barebones definition to refactor * feedback changes * more feedback addressing * removed unused Fake struct (#723) Signed-off-by: Nir Rozenbaum <[email protected]> * epp: return correct response for trailers (#726) This looks like a copy paste error. * Refactor scheduler to run plugins (#677) * Refactor scheduler to run plugins * Add scheduler plugin latency metric * Address comments * Address comments * Complete the InferencePool documentation (#673) * Initial guide for inference pool * Add extensionReference to the InferencePool spec * Fix list formatting * Remove unused labels * Autogenerate the spec * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Update site-src/api-types/inferencepool.md Co-authored-by: Rob Scott <[email protected]> * Rename llm-pool names in rollout example * Add use cases for replacing an inference pool * Rewording the background section * Create replacing-inference-pool.md * Replace instructions with a link for how to replace an inference pool * Update replacing-inference-pool.md * Update mkdocs.yml * Update replacing-inference-pool.md * Update inferencemodel_types.go * Update inferencepool.md * Update site-src/guides/replacing-inference-pool.md Co-authored-by: Rob Scott <[email protected]> --------- Co-authored-by: Rob Scott <[email protected]> * reduce log level in metrics logger not to trash the log (#708) * reduce log level in metrics logger not to trash the log Signed-off-by: Nir Rozenbaum <[email protected]> * rename flush metrics to refresh metrics Signed-off-by: Nir Rozenbaum <[email protected]> * revert log level Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> * few updates in datastore (#713) * few updates in datastore Signed-off-by: Nir Rozenbaum <[email protected]> * PoolSet documentation Signed-off-by: Nir Rozenbaum <[email protected]> * error phrasing Signed-off-by: Nir Rozenbaum <[email protected]> * removed unused pool arg from PodUpdateOrAddIfNotExist Signed-off-by: Nir Rozenbaum <[email protected]> * linter Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> * scheduler refactoring (#730) Signed-off-by: Nir Rozenbaum <[email protected]> * filter irrelevant pod in pod_reconciler (#696) * EPP: Update GetRandomPod() to return nil if no pods exist (#731) Signed-off-by: Daneyon Hansen <[email protected]> * Move filter and scorer plugins registration to a separate file (#729) * Move filters and scorers registration to filter/scorer specific files * Default scheduler config contains empty list of scorers Signed-off-by: Maya Barnea <[email protected]> * Default plugin is not a scorer any more Signed-off-by: Maya Barnea <[email protected]> * fix scheduler test + lint comments Signed-off-by: Maya Barnea <[email protected]> --------- Signed-off-by: Maya Barnea <[email protected]> * Update issue templates (#738) * Update issue templates * Updates artifacts for v0.3.0-rc.1 release Signed-off-by: Kellen Swain <[email protected]> * Updates bbr chart for v0.3.0-rc.1 release Signed-off-by: Kellen Swain <[email protected]> * Updates artifacts for v0.3.0 release Signed-off-by: Kellen Swain <[email protected]> * Adding blank issue template so that all issues start with label --------- Signed-off-by: Kellen Swain <[email protected]> * Add unit test coverage for pod APIs under datastore/pkg * few updates in datastore (#713) * few updates in datastore Signed-off-by: Nir Rozenbaum <[email protected]> * PoolSet documentation Signed-off-by: Nir Rozenbaum <[email protected]> * error phrasing Signed-off-by: Nir Rozenbaum <[email protected]> * removed unused pool arg from PodUpdateOrAddIfNotExist Signed-off-by: Nir Rozenbaum <[email protected]> * linter Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> * few updates in datastore (#713) * few updates in datastore Signed-off-by: Nir Rozenbaum <[email protected]> * PoolSet documentation Signed-off-by: Nir Rozenbaum <[email protected]> * error phrasing Signed-off-by: Nir Rozenbaum <[email protected]> * removed unused pool arg from PodUpdateOrAddIfNotExist Signed-off-by: Nir Rozenbaum <[email protected]> * linter Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> * Add unit test coverage for pod APIs under datastore/pkg --------- Signed-off-by: Nir Rozenbaum <[email protected]> Signed-off-by: Daneyon Hansen <[email protected]> Signed-off-by: Maya Barnea <[email protected]> Signed-off-by: Kellen Swain <[email protected]> Co-authored-by: Kellen Swain <[email protected]> Co-authored-by: Nir Rozenbaum <[email protected]> Co-authored-by: John Howard <[email protected]> Co-authored-by: Cong Liu <[email protected]> Co-authored-by: Nicole Xin <[email protected]> Co-authored-by: Rob Scott <[email protected]> Co-authored-by: nayihz <[email protected]> Co-authored-by: Daneyon Hansen <[email protected]> Co-authored-by: Maya Barnea <[email protected]>

few updates in datastore

c5f664a

Signed-off-by: Nir Rozenbaum <[email protected]>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 19, 2025

k8s-ci-robot requested review from ahg-g and Jeffwan April 19, 2025 18:53

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 19, 2025

PoolSet documentation

01a9368

Signed-off-by: Nir Rozenbaum <[email protected]>

error phrasing

00bfa90

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum commented Apr 19, 2025

View reviewed changes

removed unused pool arg from PodUpdateOrAddIfNotExist

2af39a4

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum commented Apr 19, 2025

View reviewed changes

linter

57b0b91

Signed-off-by: Nir Rozenbaum <[email protected]>

nirrozenbaum mentioned this pull request Apr 19, 2025

Add unit tests for pod APIs under pkg/datastore #712

Merged

k8s-ci-robot assigned kfswain Apr 23, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 23, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2025

k8s-ci-robot merged commit d935a7c into kubernetes-sigs:main Apr 23, 2025
8 checks passed

nirrozenbaum deleted the podresync branch April 23, 2025 03:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

few updates in datastore #713

few updates in datastore #713

nirrozenbaum commented Apr 19, 2025 •

edited

Loading

netlify bot commented Apr 19, 2025 •

edited

Loading

nirrozenbaum commented Apr 19, 2025

nirrozenbaum Apr 19, 2025

nirrozenbaum Apr 19, 2025

kfswain commented Apr 23, 2025

k8s-ci-robot commented Apr 23, 2025

few updates in datastore #713

few updates in datastore #713

Conversation

nirrozenbaum commented Apr 19, 2025 • edited Loading

netlify bot commented Apr 19, 2025 • edited Loading

✅ Deploy Preview for gateway-api-inference-extension ready!

nirrozenbaum commented Apr 19, 2025

nirrozenbaum Apr 19, 2025

Choose a reason for hiding this comment

nirrozenbaum Apr 19, 2025

Choose a reason for hiding this comment

kfswain commented Apr 23, 2025

k8s-ci-robot commented Apr 23, 2025

nirrozenbaum commented Apr 19, 2025 •

edited

Loading

netlify bot commented Apr 19, 2025 •

edited

Loading