controller: create HTTPRoute for each backend #537
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: yweng14 <yweng14@bloomberg.net>
Signed-off-by: yweng14 <yweng14@bloomberg.net>
Signed-off-by: yweng14 <yweng14@bloomberg.net>
yuzisun
reviewed
Mar 31, 2025
| Namespace: aiGatewayRoute.Namespace, | ||
| }, | ||
| Spec: gwapiv1.HTTPRouteSpec{}, | ||
| // List all HTTPRoutes in the same namespace as this aiGatewayRoute. |
Contributor
There was a problem hiding this comment.
We can move to a separate function reconcileHTTPRoute as it is getting bigger
| if err = c.newHTTPRoute(ctx, &httpRoute, aiGatewayRoute); err != nil { | ||
| return fmt.Errorf("failed to construct a new HTTPRoute: %w", err) | ||
| // The naming convention of HTTRoute is <backendName>-<aiGatewayRouteName>, | ||
| // It is impossible that a HTTRRoute belongs to two AIGatewayRoute. |
Contributor
There was a problem hiding this comment.
Can you have two AIGatewayRoute in the same namespace?
Contributor
Author
There was a problem hiding this comment.
Yes, it is possible that there are two AIGatewayRoute in the same namespace, and they have some same backends. In this implementation we create HTTPRoute per backend for each AIGatewayRoute.
Comment on lines
+246
to
+248
| // Choose default backend whose name is alphabetically smallest. | ||
| if defaultBackend == nil || defaultBackend.Name > backend.Name { | ||
| defaultBackend = backend |
Contributor
There was a problem hiding this comment.
What's the defaultBackend for?
xiaolin593
reviewed
Apr 4, 2025
| }, | ||
| Spec: gwapiv1.HTTPRouteSpec{}, | ||
| // List all HTTPRoutes in the same namespace as this aiGatewayRoute. | ||
| var existingHTTPRoutList gwapiv1.HTTPRouteList |
Member
|
i believe we agreed that instead of making HTTPRoute for each Backend, we should support multiple AIGWR attached to one GW that will solve the limitation that this was initially trying to solve |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message
In this PR, I create HTTPRoute for each backend in the AIServiceBackend rules. When AIGatewayRouteController reconciles a AIGatewayRoute, it lists all the HTTPRoutes belongs to that AIGatewayRoute. It also constructs a de-duplicated backends from the rules of that AIGatewayRoute.
The controller creates new HTTRoutes for each backend if they do not exist before, and updates the HTTRoutes if they are already existed. We delete HTTPRoutes if they are not in the backends and only belong to that AIGatewayRoute.
The HTTRoute is created per backend and per AIGatewayRoute, naming convention is
<backendName>-<aiGatewayRouteName>, it makes the logic of HTTRoute lifecycle much clearer. Otherwise, I need merge owner references during update.Related Issues/PRs (if applicable)
Fix issue #509
Special notes for reviewers (if applicable)
N.A.