-
Notifications
You must be signed in to change notification settings - Fork 150
Truncate long resource names when provisioning #4387
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.
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4387 +/- ##
=======================================
Coverage 86.15% 86.15%
=======================================
Files 132 132
Lines 14359 14376 +17
Branches 35 35
=======================================
+ Hits 12371 12386 +15
+ Misses 1785 1781 -4
- Partials 203 209 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where Services could fail to be created when the Gateway or InferencePool names were very long, exceeding the Kubernetes 63-character limit for Service names. The solution implements name truncation with hash-based uniqueness preservation.
Key Changes:
- Implements
truncateAndHashNamefunction that truncates long resource names and adds an 8-character hash for uniqueness while preserving required suffixes - Replaces
GetInferencePoolNamefunction withInferencePoolNamefield inRouteBackendRefstruct to avoid recreating pool names from service names - Adds comprehensive tests for name truncation logic
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/framework/controller/resource.go | Implements new truncation logic with hash generation for resource names exceeding 63 characters |
| internal/framework/controller/resource_test.go | Adds comprehensive unit tests for name truncation scenarios |
| internal/controller/state/graph/route_common.go | Adds InferencePoolName field to RouteBackendRef struct |
| internal/controller/state/graph/httproute.go | Updates to populate and use new InferencePoolName field |
| internal/controller/state/graph/inferencepools.go | Uses InferencePoolName field instead of deriving from service name |
| internal/controller/state/graph/backend_refs.go | Uses InferencePoolName field instead of deriving from service name |
| internal/controller/state/graph/*_test.go | Updates test data to include InferencePoolName field |
| internal/controller/provisioner/objects.go | Truncates gateway name when used as label value |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b874551 to
279bea5
Compare
Problem: When provisioning the NGINX Service, if the Gateway name was very long, it could result in a failure to create the Service. A Service name must be 63 characters or less. This also could affect the shadow Service that we create for InferencePools if the InferencePool name is long. Solution: If the name of the resource that we create is going to be over the max, create a hash of the name and truncate to ensure that it is less than the max. The suffix is still included, since we rely on that for certain resources. Just the prefix is shortened as necessary to include the hash.
279bea5 to
82767ae
Compare
Problem: When provisioning the NGINX Service, if the Gateway name was very long, it could result in a failure to create the Service. A Service name must be 63 characters or less. This also could affect the shadow Service that we create for InferencePools if the InferencePool name is long.
Solution: If the name of the resource that we create is going to be over the max, create a hash of the name and truncate to ensure that it is less than the max. The suffix is still included, since we rely on that for certain resources. Just the prefix is shortened as necessary to include the hash.
Testing: Deployed a Gateway with a very long name and verified that the Service was still created properly. Also did the same test with an InferencePool.
Closes #4177
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.