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

Conflicted listener status is not surfaced for gateways when MergeGateways enabled #2668

Closed
shawnh2 opened this issue Feb 21, 2024 · 4 comments · Fixed by #2672
Closed

Conflicted listener status is not surfaced for gateways when MergeGateways enabled #2668

shawnh2 opened this issue Feb 21, 2024 · 4 comments · Fixed by #2672
Assignees
Labels
kind/bug Something isn't working road-to-ga
Milestone

Comments

@shawnh2
Copy link
Contributor

shawnh2 commented Feb 21, 2024

Description:

Applying these config, the listener of GTW merged-eg-3 is conflicted with the listener GTW merged-eg-2. But there is no status surfaced about these conflicted listener.

apiVersion: gateway.networking.k8s.io/v1
kind: GatewayClass
metadata:
  name: mg
spec:
  controllerName: gateway.envoyproxy.io/gatewayclass-controller
  parametersRef:
    group: gateway.envoyproxy.io
    kind: EnvoyProxy
    name: custom-proxy-config
    namespace: envoy-gateway-system
---
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyProxy
metadata:
  name: custom-proxy-config
  namespace: envoy-gateway-system
spec:
  mergeGateways: true
---
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: merged-eg-1
  namespace: default
spec:
  gatewayClassName: mg
  listeners:
     - name: http
       port: 8080
       protocol: HTTP
---
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: merged-eg-2
  namespace: default
spec:
  gatewayClassName: mg
  listeners:
     - name: http
       port: 8081
       protocol: HTTP
---
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: merged-eg-3
  namespace: default
spec:
  gatewayClassName: mg
  listeners:
     - name: http
       port: 8081
       protocol: HTTP

According to the validation method, the conflicted status should be computed, but somehow is not surfaced.

func (t *Translator) validateConflictedMergedListeners(gateways []*GatewayContext) {

k get gtw -o yaml                                                 ✱

apiVersion: v1
items:
- apiVersion: gateway.networking.k8s.io/v1
  kind: Gateway
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"gateway.networking.k8s.io/v1","kind":"Gateway","metadata":{"annotations":{},"name":"merged-eg-1","namespace":"default"},"spec":{"gatewayClassName":"mg","listeners":[{"name":"http","port":8080,"protocol":"HTTP"}]}}
    creationTimestamp: "2024-02-21T14:43:57Z"
    generation: 1
    name: merged-eg-1
    namespace: default
    resourceVersion: "7869"
    uid: 2285a144-8c92-4ab3-81d4-fd485723f246
  spec:
    gatewayClassName: mg
    listeners:
    - allowedRoutes:
        namespaces:
          from: Same
      name: http
      port: 8080
      protocol: HTTP
  status:
    addresses:
    - type: IPAddress
      value: 172.18.255.200
    conditions:
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: The Gateway has been scheduled by Envoy Gateway
      observedGeneration: 1
      reason: Accepted
      status: "True"
      type: Accepted
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: Address assigned to the Gateway, 1/1 envoy Deployment replicas available
      observedGeneration: 1
      reason: Programmed
      status: "True"
      type: Programmed
- apiVersion: gateway.networking.k8s.io/v1
  kind: Gateway
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"gateway.networking.k8s.io/v1","kind":"Gateway","metadata":{"annotations":{},"name":"merged-eg-2","namespace":"default"},"spec":{"gatewayClassName":"mg","listeners":[{"name":"http","port":8081,"protocol":"HTTP"}]}}
    creationTimestamp: "2024-02-21T14:43:57Z"
    generation: 1
    name: merged-eg-2
    namespace: default
    resourceVersion: "7870"
    uid: 27440145-0932-46dd-aba1-d8dcf53a7f0d
  spec:
    gatewayClassName: mg
    listeners:
    - allowedRoutes:
        namespaces:
          from: Same
      name: http
      port: 8081
      protocol: HTTP
  status:
    addresses:
    - type: IPAddress
      value: 172.18.255.200
    conditions:
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: The Gateway has been scheduled by Envoy Gateway
      observedGeneration: 1
      reason: Accepted
      status: "True"
      type: Accepted
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: Address assigned to the Gateway, 1/1 envoy Deployment replicas available
      observedGeneration: 1
      reason: Programmed
      status: "True"
      type: Programmed
- apiVersion: gateway.networking.k8s.io/v1
  kind: Gateway
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"gateway.networking.k8s.io/v1","kind":"Gateway","metadata":{"annotations":{},"name":"merged-eg-3","namespace":"default"},"spec":{"gatewayClassName":"mg","listeners":[{"name":"http","port":8081,"protocol":"HTTP"}]}}
    creationTimestamp: "2024-02-21T14:43:57Z"
    generation: 1
    name: merged-eg-3
    namespace: default
    resourceVersion: "7871"
    uid: 7bc0dc08-f145-4310-b18d-6dc86a123151
  spec:
    gatewayClassName: mg
    listeners:
    - allowedRoutes:
        namespaces:
          from: Same
      name: http
      port: 8081
      protocol: HTTP
  status:
    addresses:
    - type: IPAddress
      value: 172.18.255.200
    conditions:
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: The Gateway has been scheduled by Envoy Gateway
      observedGeneration: 1
      reason: Accepted
      status: "True"
      type: Accepted
    - lastTransitionTime: "2024-02-21T14:43:58Z"
      message: Address assigned to the Gateway, 1/1 envoy Deployment replicas available
      observedGeneration: 1
      reason: Programmed
      status: "True"
      type: Programmed
kind: List
metadata:
  resourceVersion: ""

Environment:

lattest

@shawnh2 shawnh2 added the kind/bug Something isn't working label Feb 21, 2024
@shawnh2 shawnh2 changed the title Conflicted listener status is not surfaced for gateways with MergeGateways enabled Conflicted listener status is not surfaced for gateways when MergeGateways enabled Feb 21, 2024
@arkodg
Copy link
Contributor

arkodg commented Feb 21, 2024

good find,

for _, gateway := range gateways {
looks fine from a first glance which is being tested here https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/testdata/merge-invalid-multiple-gateways.out.yaml

@arkodg arkodg added the help wanted Extra attention is needed label Feb 21, 2024
@arkodg arkodg added this to the v1.0.0-rc1 milestone Feb 21, 2024
@shawnh2 shawnh2 self-assigned this Feb 22, 2024
@shawnh2 shawnh2 removed the help wanted Extra attention is needed label Feb 22, 2024
@shawnh2
Copy link
Contributor Author

shawnh2 commented Feb 22, 2024

the conflicted listener sure has been found and set to listener's status in this method:

func (t *Translator) validateConflictedMergedListeners(gateways []*GatewayContext) {

but when mergeGateways enabled, predicates for Service and Deployment

func (r *gatewayAPIReconciler) validateDeploymentForReconcile(obj client.Object) bool {

func (r *gatewayAPIReconciler) validateServiceForReconcile(obj client.Object) bool {

will update GTW status again, during this update, GTWs are loaded from GatewayAPIResources which does not contains any computed (translated) status. so will overwrite previous listener's status to empty, casue the missing of listener's status.

r.updateStatusForGateway(ctx, gw)


so the fix here would be:

since we cannot get computed (translated) status in gatewayapi-layer, so directly get the GTW via k8s client instead of loading it from GatewayAPIResources

@cnvergence
Copy link
Member

+1 I think it would solve this issue

@arkodg
Copy link
Contributor

arkodg commented Feb 22, 2024

good find @shawnh2 ! this probably explains a lot of the flakiness in CI :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working road-to-ga
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants