Skip to content

fix: align envoy pod template labels with controller selector #1439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jeffwan merged 1 commit into from
Aug 15, 2025
Merged

fix: align envoy pod template labels with controller selector #1439

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions dist/chart/templates/gateway-instance/gateway.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ spec:
template:
metadata:
labels:
{{- include "chart.labels" . | nindent 20 }}
app.kubernetes.io/component: aibrix-gateway
{{- include "chart.selectorLabels" . | nindent 20 }}
Copy link
Collaborator

@googs1025 googs1025 Aug 13, 2025

Choose a reason for hiding this comment

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

some info in: https://github.com/vllm-project/aibrix/tree/main/dist/chart#limitations

Is this a related?

Copy link
Collaborator

@Jeffwan Jeffwan Aug 14, 2025

Choose a reason for hiding this comment

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

not directly related. The problem is envoy managed it's labels by themselves.

Copy link
Collaborator

@Jeffwan Jeffwan Aug 14, 2025

Choose a reason for hiding this comment

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

I thought I fixed related issues here https://github.com/vllm-project/aibrix/pull/1367/files but seems not.

@omerap12 did you meet problems following the current guidance? Why our helm CI chart testing can not detect the problem?

If this is a problem on the main branch, then it's a very critical problem..

Copy link
Contributor Author

@omerap12 omerap12 Aug 14, 2025

Choose a reason for hiding this comment

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

Yes I followed the official instructions.
The CI has no way of knowing that something like that has happened since it's checking that chart was installed successfully and all pods are in Ready state.
Since all of that has happened it passed.

Copy link
Collaborator

@Jeffwan Jeffwan Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@omerap12 BTW, the error logs in the issue is from which component? envoy gateway? so the aibrix envoy instance is not created, right? technically, we should be able to detect it. I will help reproduce from my side.

Copy link
Contributor Author

@omerap12 omerap12 Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes it's from the envoy gateway ( sorry I didn't mention this in the description )

Copy link
Collaborator

@Jeffwan Jeffwan Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This makes sense. I can reproduce from my side.

# latest labels
                template:
                  metadata:
                    labels:
                      app.kubernetes.io/component: proxy
                      app.kubernetes.io/instance: aibrix
                      app.kubernetes.io/name: envoy

@omerap12 nice catch! I will merge this one and could you help send a cherry-pick PR against release-0.4? We want to cut v0.4.1 and this should be included in the patch release.

Copy link
Contributor Author

@omerap12 omerap12 Aug 15, 2025

Choose a reason for hiding this comment

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

No problem, but I’m on vacation. I can do it tomorrow.

app.kubernetes.io/name: envoy
app.kubernetes.io/component: proxy
spec:
containers:
- name: envoy
Expand Down