gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

conformance: test gateway when an unsupported Route kind is specified by the Listener.

Open mlavacca opened this issue 2 years ago • 3 comments

What would you like to be added:

A new conformance test case testing the presence of the status ResolvedRefs condition failed with the reason ListenerReasonInvalidRouteKinds should be added to the conformance test suite.

To do so, the test case should create a gateway with a listener specifying an unsupported Route Kind and check the ResolvedRefs condition is added to the corresponding listener status with the reason ListenerReasonInvalidRouteKinds.

Why this is needed:

This is needed to ensure that when a listener references an unsupported Route kind, the Listener status is updated accordingly.

mlavacca avatar Aug 12 '22 13:08 mlavacca

How does this differ from the HTTPRouteDisallowedKind test? Maybe we just need to expand that one?

robscott avatar Aug 12 '22 23:08 robscott

That test is somewhat specular to the one I'm suggesting here:

  • The HTTPRouteDisallowedKind test creates an HTTPRoute with a ParentRefs that doesn't accept that kind of route (in that test the gateway accepts only TLSRoutes)
  • In the test described by this issue we should create a Gateway that accepts a kind of route not compatible with the Listener.Protocol field. As it is stated in the docs https://github.com/kubernetes-sigs/gateway-api/blob/e8842f21edae20c7e35369fea42da05df62ac308/apis/v1beta1/gateway_types.go#L380-L394 we should ensure that the ResolvedRefs condition fails with Reason ListenerReasonInvalidRouteKinds.

Even if these tests could be grouped, I think it would be clearer and more beneficial for those who want to make their own implementation conformant, to have two different test cases.

mlavacca avatar Aug 13 '22 10:08 mlavacca

/assign

mlavacca avatar Aug 16 '22 14:08 mlavacca

I'm having an issue passing this test as the test is requiring the Gateway ListenerStatus SupportedKinds array to be empty. The CRD does not permit an empty array to be passed in, nor does it allow a single element array with a 0 Kind length. Am I missing something here? Thanks!

rperper avatar Oct 07 '22 13:10 rperper

I'm having an issue passing this test as the test is requiring the Gateway ListenerStatus SupportedKinds array to be empty. The CRD does not permit an empty array to be passed in, nor does it allow a single element array with a 0 Kind length. Am I missing something here? Thanks!

Hi @rperper, the field SupportedKinds is mandatory, therefore you cannot leave it as nil, but you need to provide an array of length 0. With such a value, it should work as expected.

mlavacca avatar Oct 07 '22 15:10 mlavacca

Thanks for responding so quickly! When I tried that I got an error: Gateway.gateway.networking.k8s.io "gateway-only-unsupported-route-kind" is invalid: status.listeners[0].supportedKinds: Required value. This little bit of code:

	klog.V(4).Infof("Gateway UpdateStatus: %v", gwItem.gw)
	_, err := lbc.gw.clientset.Gateways(gwItem.gw.ObjectMeta.Namespace).UpdateStatus(context.Background(), gwItem.gw, updateOptions)
	if err != nil {
		klog.Errorf("Error updating Gateway status for %v: %v", gwItem.gw.Name, err)
		return false

Resulted in this output in the trace:

I1007 18:09:12.215071       1 gw.go:813] Gateway UpdateStatus: &{{ } {gateway-only-unsupported-route-kind  gateway-conformance-infra  6a5b8e74-729d-4716-9b5f-05676a76d4c4 6099089 1 2022-10-07 18:09:12 +0000 UTC <nil> <nil> map[] map[] [] []  [{conformance.test Update gateway.networking.k8s.io/v1beta1 2022-10-07 18:09:12 +0000 UTC FieldsV1 {"f:spec":{".":{},"f:gatewayClassName":{},"f:listeners":{".":{},"k:{\"name\":\"https\"}":{".":{},"f:allowedRoutes":{".":{},"f:kinds":{},"f:namespaces":{".":{},"f:from":{}}},"f:name":{},"f:port":{},"f:protocol":{}}}}} }]} {lslbd [{https <nil> 443 HTTPS <nil> 0xc000b0bba0}] []} {[{0xc0003b5ae0 143.244.201.38}] [{Ready True 0 2022-10-07 18:09:12.215065565 +0000 UTC m=+169.553486098 Ready }] [{https [] 0 [{ResolvedRefs False 0 2022-10-07 18:09:12.200713611 +0000 UTC m=+169.539134094 InvalidRouteKinds Only HTTPRoute kind is currently supported and you specified: UDPRoute}]}]}}
E1007 18:09:12.221759       1 gw.go:816] Error updating Gateway status for gateway-only-unsupported-route-kind: Gateway.gateway.networking.k8s.io "gateway-only-unsupported-route-kind" is invalid: status.listeners[0].supportedKinds: Required value

As you can see, the field: f:kinds":{} which is an empty array. But I may indeed have missed something. So much appreciate your help!

rperper avatar Oct 07 '22 18:10 rperper

The kinds field you pointed out above is in the listener spec, while the supportedKinds is in the listener status. In this case, no kinds specified in the allowedRoutes matches the ProtocolType. Therefore the field supportedKinds in the listener status must be set to an empty (non-nil) slice. Indeed, the error is in status.listeners[0].supportedKinds, not in spec.listeners[0].allowedRoutes.kinds.

From the output above, it looks like you are changing the kinds in the spec, instead of the supportedKinds in the status. If it doesn't help, can you please paste here the gateway that triggers the update error?

mlavacca avatar Oct 07 '22 21:10 mlavacca

Hi @mlavacca, I don't think I was stomping on the listener's allowedRoutes.kinds. So I added additional tracing code right before the gateway UpdateStatus:

I1011 14:58:26.728681       1 gw.go:812] Gateway UpdateStatus: &{{ } {gateway-only-unsupported-route-kind  gateway-conformance-infra  7114166b-9d33-4829-b24b-8e674b707c35 7453682 1 2022-10-11 14:58:26 +0000 UTC <nil> <nil> map[] map[] [] []  [{conformance.test Update gateway.networking.k8s.io/v1beta1 2022-10-11 14:58:26 +0000 UTC FieldsV1 {"f:spec":{".":{},"f:gatewayClassName":{},"f:listeners":{".":{},"k:{\"name\":\"https\"}":{".":{},"f:allowedRoutes":{".":{},"f:kinds":{},"f:namespaces":{".":{},"f:from":{}}},"f:name":{},"f:port":{},"f:protocol":{}}}}} }]} {lslbd [{https <nil> 443 HTTPS <nil> 0xc00097af20}] []} {[{0xc0009b2bd0 143.244.213.73}] [{Ready True 0 2022-10-11 14:58:26.728677538 +0000 UTC m=+186.795908833 Ready }] [{https [] 0 [{ResolvedRefs False 0 2022-10-11 14:58:26.728321414 +0000 UTC m=+186.795552521 InvalidRouteKinds Only HTTPRoute kind is currently supported and you specified: UDPRoute}]}]}}
I1011 14:58:26.728808       1 gw.go:817]     spec.listeners[0].AllowedRoutes: {0xc0009b2440 [{0xc0009b2430 UDPRoute}]}
I1011 14:58:26.728839       1 gw.go:820]     status.listeners: [{https [] 0 [{ResolvedRefs False 0 2022-10-11 14:58:26.728321414 +0000 UTC m=+186.795552521 InvalidRouteKinds Only HTTPRoute kind is currently supported and you specified: UDPRoute}]}]
E1011 14:58:26.739156       1 gw.go:823] Error updating Gateway status for gateway-only-unsupported-route-kind: Gateway.gateway.networking.k8s.io "gateway-only-unsupported-route-kind" is invalid: status.listeners[0].supportedKinds: Required value

I'm using the conformance tests; this is the gateway-unsupported-route-kind test. The full gateway (from kubectl) is:

Name:         gateway-only-unsupported-route-kind
Namespace:    gateway-conformance-infra
Labels:       <none>
Annotations:  <none>
API Version:  gateway.networking.k8s.io/v1beta1
Kind:         Gateway
Metadata:
  Creation Timestamp:  2022-10-11T13:36:33Z
  Generation:          1
  Managed Fields:
    API Version:  gateway.networking.k8s.io/v1beta1
    Fields Type:  FieldsV1
    fieldsV1:
      f:spec:
        .:
        f:gatewayClassName:
        f:listeners:
          .:
          k:{"name":"https"}:
            .:
            f:allowedRoutes:
              .:
              f:kinds:
              f:namespaces:
                .:
                f:from:
            f:name:
            f:port:
            f:protocol:
    Manager:         conformance.test
    Operation:       Update
    Time:            2022-10-11T13:36:33Z
  Resource Version:  7431969
  UID:               e096f695-3d87-459f-9d94-94aa67f5c48c
Spec:
  Gateway Class Name:  lslbd
  Listeners:
    Allowed Routes:
      Kinds:
        Group:  gateway.networking.k8s.io
        Kind:   UDPRoute
      Namespaces:
        From:  All
    Name:      https
    Port:      443
    Protocol:  HTTPS

As a load balancer which doesn't support UDPRoute (and that's what the test is written for), I'm supposed to say that I support [] kinds. But that's where I'm in trouble. I'd think everyone would be failing this test, so that's why I'm thinking I'm doing something wrong. But indeed Gateway is still early and this is a relatively new test.
Thanks so much!

rperper avatar Oct 11 '22 15:10 rperper

@rperper I actually was looking for the Gateway Status that triggers the error, can you add the Status section in the snippet above? Reading from the log, status.listeners[0].supportedKinds looks like a nil slice, not an empty slice

mlavacca avatar Oct 11 '22 15:10 mlavacca

@mlavacca I don't affect the status. This is the Gateway.UpdateStatus() function and it fails so the status doesn't change.
But what it ends up with is this:

Name:         gateway-only-unsupported-route-kind
Namespace:    gateway-conformance-infra
Labels:       <none>
Annotations:  <none>
API Version:  gateway.networking.k8s.io/v1beta1
Kind:         Gateway
Metadata:
  Creation Timestamp:  2022-10-11T16:31:06Z
  Generation:          1
  Managed Fields:
    API Version:  gateway.networking.k8s.io/v1beta1
    Fields Type:  FieldsV1
    fieldsV1:
      f:spec:
        .:
        f:gatewayClassName:
        f:listeners:
          .:
          k:{"name":"https"}:
            .:
            f:allowedRoutes:
              .:
              f:kinds:
              f:namespaces:
                .:
                f:from:
            f:name:
            f:port:
            f:protocol:
    Manager:         conformance.test
    Operation:       Update
    Time:            2022-10-11T16:31:06Z
  Resource Version:  7478798
  UID:               bea07004-a2ef-4f47-95ec-0fad5a90edd7
Spec:
  Gateway Class Name:  lslbd
  Listeners:
    Allowed Routes:
      Kinds:
        Group:  gateway.networking.k8s.io
        Kind:   UDPRoute
      Namespaces:
        From:  All
    Name:      https
    Port:      443
    Protocol:  HTTPS
Status:
  Conditions:
    Last Transition Time:  1970-01-01T00:00:00Z
    Message:               Waiting for controller
    Reason:                NotReconciled
    Status:                Unknown
    Type:                  Scheduled
Events:                    <none>

Since it fails the UpdateStatus() it doesn't even update the NotReconciled reason. Sigh.

rperper avatar Oct 11 '22 16:10 rperper

For reference, Contour does the following if there are no supported kinds, which seems to work fine: https://github.com/projectcontour/contour/blob/main/internal/status/gatewaystatus.go#L204-L207

skriss avatar Oct 11 '22 16:10 skriss

That did it! Thanks so much!

rperper avatar Oct 11 '22 18:10 rperper