Skip to content

Conversation

carlory
Copy link
Contributor

@carlory carlory commented Aug 13, 2025

image

@carlory
Copy link
Contributor Author

carlory commented Aug 13, 2025

I'm not sure whether I should get the git information of llm-d or the git information of GIE which is from go mod

RUN go build -a -o bin/epp -ldflags="-extldflags '-L$(pwd)/lib'" cmd/epp/main.go
ARG COMMIT_SHA=unknown
ARG BUILD_REF
RUN go build -a -o bin/epp -ldflags="-extldflags '-L$(pwd)/lib' -X sigs.k8s.io/gateway-api-inference-extension/pkg/epp/metrics.CommitSHA=${COMMIT_SHA} -X sigs.k8s.io/gateway-api-inference-extension/pkg/epp/metrics.BuildRef=${BUILD_REF}" cmd/epp/main.go
Copy link
Collaborator

Choose a reason for hiding this comment

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

The correct fields to set are sigs.k8s.io/gateway-api-inference-extension/version.CommitSHA and sigs.k8s.io/gateway-api-inference-extension/version.BuildRef.

Thus the command should be:

RUN go build -a -o bin/epp -ldflags="-extldflags '-L$(pwd)/lib' -X sigs.k8s.io/gateway-api-inference-extension/version.CommitSHA=${COMMIT_SHA} -X sigs.k8s.io/gateway-api-inference-extension/version.BuildRef=${BUILD_REF}" cmd/epp/main.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shmuelk v0.5.1 doesn't have the version package.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

right. this change (moving the fields to package version) was done post v0.5.1 release 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry. My bad.

Copy link
Collaborator

@shmuelk shmuelk Aug 14, 2025

Choose a reason for hiding this comment

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

Please rebase and sign your commit and then I'll happily approve and merge this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shmuelk rebased.

@shmuelk
Copy link
Collaborator

shmuelk commented Aug 14, 2025

Thanks for the rebase.

Please go to your github settings and verify your signature. While you have signed your commits, they are not verified by github. We require verified signatures.

@carlory
Copy link
Contributor Author

carlory commented Aug 14, 2025

@shmuelk thanks for your guide.

Copy link
Collaborator

@shmuelk shmuelk left a comment

Choose a reason for hiding this comment

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

/lgtm

/approve

@nirrozenbaum nirrozenbaum merged commit f23a6f5 into llm-d:main Aug 14, 2025
4 checks passed
@carlory carlory deleted the fix-image-build branch August 15, 2025 02:09
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