Skip to content

Conversation

@sallyom
Copy link
Contributor

@sallyom sallyom commented Aug 14, 2025

Adds EPP servicemonitor

@kalantar
Copy link
Collaborator

Is the intent of this to add monitoring to the epp node itself?

@kalantar
Copy link
Collaborator

I notice the creation of a ClusterRole and a ClusterRoleBinding. I wonder if this is specific to monitoring the epp pod or if this is a general issue. I notice that in the upstream GAIE inferencepool template, they split the permissions between a Role and a ClusterRole.

@kalantar
Copy link
Collaborator

With a push to move function to the upstream GAIE, I wonder if this should be there instead?

@sallyom sallyom force-pushed the servicemonitor branch 5 times, most recently from a24687c to 705ade9 Compare August 21, 2025 03:35
@sallyom
Copy link
Contributor Author

sallyom commented Aug 21, 2025

@kalantar servicemonitor with rbac/bearertoken is required to scrape metrics from EPP - the equivalent changes are proposed in the GAIE charts (kubernetes-sigs/gateway-api-inference-extension#1425) - yes if charts are moved upstream, this won't be required. This change will enable metrics collection from EPP pod until the charts are consolidated into 1 upstream chart. Clusterrole & Clusterrolebinding are required to scrape metrics from EPP. Same as in GAIE charts.

Copy link
Collaborator

@jgchn jgchn left a comment

Choose a reason for hiding this comment

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

Tracking the upstream GAIE PR. It sounds like once that is merged, this PR against ms won't be necessary.

Signed-off-by: sallyom <[email protected]>
@sallyom
Copy link
Contributor Author

sallyom commented Sep 12, 2025

@kalantar @jgchn upstream PR merged - the wide-ep guide deploys epp with modelservice charts, so we should add the servicemonitor to modelservice charts as well since these will remain through the next release.

PR to add servicemonitor to guides

/cc @Gregory-Pereira

@sallyom
Copy link
Contributor Author

sallyom commented Sep 16, 2025

closing this, will look at new format for kustomize

@sallyom sallyom closed this Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants