Skip to content
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

ingestion/gateway-api: Map backend weight to model #22380

Merged
merged 1 commit into from Nov 28, 2022

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Nov 27, 2022

Description

This commit is to make sure the weightage value is propagated to internal model.

Relates: 58c8aff

Reported-by: Nico Vibert nicolas.vibert@isovalent.com
Signed-off-by: Tam Mach tam.mach@cilium.io

Testing

Verification is done by checking envoy config dump, then run-time behavior is confirmed.

HTTP route config
---
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: HTTPRoute
metadata:
  name: example-route-1
spec:
  parentRefs:
  - name: my-example-gateway
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /echo
    backendRefs:
    - kind: Service
      name: echo-1
      port: 8080
      weight: 100
    - kind: Service
      name: echo-2
      port: 8090
      weight: 0
Envoy Config dump
# Before (weight values are always 1)
"route": {
 "weighted_clusters": {
  "clusters": [
   {
    "name": "default/echo-1:8080",
    "weight": 1
   },
   {
    "name": "default/echo-2:8090",
    "weight": 1
   }
  ],
  "total_weight": 2
 },
           
# After
"route": {
 "weighted_clusters": {
  "clusters": [
   {
    "name": "default/echo-1:8080",
    "weight": 100
   },
   {
    "name": "default/echo-2:8090",
    "weight": 0
   }
  ],
  "total_weight": 100
 },        
Runtime behavior
$ curl -s http://$lb/echo | grep "Hostname: echo-"
Hostname: echo-1-7b499f4578-7j9dr

...
$ curl -s http://$lb/echo | grep "Hostname: echo-"
Hostname: echo-1-7b499f4578-7j9dr

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 27, 2022
@sayboras sayboras added the release-note/misc This PR makes changes that have no direct user impact. label Nov 27, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 27, 2022
@sayboras sayboras added the area/servicemesh GH issues or PRs regarding servicemesh label Nov 27, 2022
This commit is to make sure the weightage value is propagated to
internal model.

Relates: 58c8aff

Reported-by: Nico Vibert <nicolas.vibert@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras marked this pull request as ready for review November 27, 2022 21:31
@sayboras sayboras requested a review from a team as a code owner November 27, 2022 21:31
@sayboras sayboras requested a review from nebril November 27, 2022 21:31
@sayboras
Copy link
Member Author

Full CI is not required, the changes are covered by Gateway API conformance test.

@sayboras sayboras added this to the 1.13 milestone Nov 28, 2022
@sayboras
Copy link
Member Author

This changes are only for Gateway API, full CI is not required. Conformance test is passed, in addition to manual verification. Mark this ready to merge.

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 28, 2022
@jrajahalme jrajahalme merged commit f09610e into cilium:master Nov 28, 2022
@sayboras sayboras deleted the tam/service-weight branch November 28, 2022 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants