gateway icon indicating copy to clipboard operation
gateway copied to clipboard

Align gateway class supported features with upstream changes

Open levikobi opened this issue 1 year ago • 2 comments
trafficstars

Description:

Describe the desired behavior, what scenario it enables and how it would be used.

https://github.com/kubernetes-sigs/gateway-api/pull/3200 introduced a breaking change to SupportedFeatures experimental API. We need to align the controller with the new API structure.

[optional Relevant Links:]

Any extra documentation required to understand the issue.

levikobi avatar Aug 15 '24 16:08 levikobi

In https://github.com/envoyproxy/gateway/pull/3888 the feature was turned off. I will not turn it on (unless @arkodg wants me to), but I still think it's worth updating the controller with this change.

/assign

levikobi avatar Aug 15 '24 16:08 levikobi

my vote is to turn it on when the field becomes stable (else it introduces a lot of breaking changes) or when we have many users asking for this

arkodg avatar Aug 15 '24 16:08 arkodg

@arkodg I aligned how we update supported features in the gateway class status. To do so, I used the following version of gateway-api: v1.1.1-0.20240819173812-8f5f9d1f515b.

While testing it out, I saw another breaking change (as the code could no longer compile):

internal/gatewayapi/route.go:610:34: cannot use headerMatch.Type (variable of type *"sigs.k8s.io/gateway-api/apis/v1".GRPCHeaderMatchType) as *"sigs.k8s.io/gateway-api/apis/v1".HeaderMatchType value in argument to HeaderMatchTypeDerefOr
make[1]: *** [tools/make/golang.mk:31: go.build.linux_amd64.envoy-gateway] Error 1

Is this known and addressed in another issue?

If not, I think we have two options:

  1. I will continue with the original intent of this issue and create a PR that only contains the fix for it. This PR will probably be cherry-picked into a different PR that will update the gateway-api version with all the breaking changes fixes.
  2. I can fix the new error and open one PR, which will update the gateway-api version and contain the fixes for the breaking changes.

I think the 2nd way is the way to go, but please let me know what you prefer.

levikobi avatar Sep 02 '24 05:09 levikobi

hey @levikobi, we discussed this in the community meeting this week and decided that we should put this on hold until the feature is stable since we feel that this feature doesnt add any extra value yet

arkodg avatar Sep 05 '24 22:09 arkodg

No problem Thanks for the update @arkodg

levikobi avatar Sep 06 '24 05:09 levikobi

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

github-actions[bot] avatar Oct 06 '24 08:10 github-actions[bot]

hmm @levikobi @arkodg what is status of this?

I deployed eg 1.2.1 and now I am seeing following in logs

2024-11-13T11:56:50.969Z        ERROR   provider        cache/reflector.go:158  Unhandled Error {"runner": "provider", "logger": "UnhandledError", "error": "pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:243: Failed to watch *v1.GatewayClass: failed to list *v1.GatewayClass: json: cannot unmarshal string into Go struct field GatewayClassStatus.items.status.supportedFeatures of type v1.SupportedFeature"}

my gatewayclass

apiVersion: gateway.networking.k8s.io/v1
kind: GatewayClass
metadata:
  finalizers:
  - gateway-exists-finalizer.gateway.networking.k8s.io
  generation: 1
  name: eg-external
  resourceVersion: "579344721"
  uid: a076f9ad-7949-453d-9708-7cfd350ff06d
spec:
  controllerName: gateway.envoyproxy.io/gatewayclass-controller
  parametersRef:
    group: gateway.envoyproxy.io
    kind: EnvoyProxy
    name: custom-proxy-config-external
    namespace: envoy-gateway-system
status:
  conditions:
  - lastTransitionTime: "2024-05-30T08:56:51Z"
    message: Valid GatewayClass
    observedGeneration: 1
    reason: Accepted
    status: "True"
    type: Accepted
  supportedFeatures:
  - GRPCRoute
  - Gateway
  - GatewayPort8080
  - HTTPRoute
  - HTTPRouteBackendProtocolH2C
  - HTTPRouteBackendProtocolWebSocket
  - HTTPRouteBackendTimeout
  - HTTPRouteDestinationPortMatching
  - HTTPRouteHostRewrite
  - HTTPRouteMethodMatching
  - HTTPRouteParentRefPort
  - HTTPRoutePathRedirect
  - HTTPRoutePathRewrite
  - HTTPRoutePortRedirect
  - HTTPRouteQueryParamMatching
  - HTTPRouteRequestMirror
  - HTTPRouteRequestMultipleMirrors
  - HTTPRouteRequestTimeout
  - HTTPRouteResponseHeaderModification
  - HTTPRouteSchemeRedirect
  - ReferenceGrant
  - TLSRoute

so basically eg should now convert status section somehow to new spec?

zetaab avatar Nov 13 '24 12:11 zetaab

2024-11-13T13:02:31.699Z        ERROR   provider        runner/runner.go:65     unable to start provider        {"runner": "provider", "error": "[failed to wait for gatewayapi-1731502801 caches to sync: timed out waiting for cache to be synced for Kind *v1.GatewayClass, failed waiting for all runnables to end within grace period of 30s: context deadline exceeded]", "errorCauses": [{"error": "failed to wait for gatewayapi-1731502801 caches to sync: timed out waiting for cache to be synced for Kind *v1.GatewayClass"}, {"error": "failed waiting for all runnables to end within grace period of 30s: context deadline exceeded"}]}

now the issue is that our envoy gateway is crashloopbackoff because of this. Of course one option is to recreate gatewayclasses.. but what is the instruction here

zetaab avatar Nov 13 '24 13:11 zetaab

% kubectl delete gatewayclass eg-internal --force --grace-period=0
Warning: Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely.
gatewayclass.gateway.networking.k8s.io "eg-internal" force deleted

looks like I cannot remove it either.. the command is just stuck and does not remove anything. I cannot patch the old object either, it returns errors.

Trying to remove whole status section:

% kubectl replace -f out.yaml
The GatewayClass "eg-internal" is invalid: 
* status.supportedFeatures[0]: Invalid value: "string": status.supportedFeatures[0] in body must be of type object: "string"
* status.supportedFeatures[1]: Invalid value: "string": status.supportedFeatures[1] in body must be of type object: "string"
* status.supportedFeatures[2]: Invalid value: "string": status.supportedFeatures[2] in body must be of type object: "string"
* status.supportedFeatures[3]: Invalid value: "string": status.supportedFeatures[3] in body must be of type object: "string"
* status.supportedFeatures[4]: Invalid value: "string": status.supportedFeatures[4] in body must be of type object: "string"
* status.supportedFeatures[5]: Invalid value: "string": status.supportedFeatures[5] in body must be of type object: "string"
* status.supportedFeatures[6]: Invalid value: "string": status.supportedFeatures[6] in body must be of type object: "string"
* status.supportedFeatures[7]: Invalid value: "string": status.supportedFeatures[7] in body must be of type object: "string"
* status.supportedFeatures[8]: Invalid value: "string": status.supportedFeatures[8] in body must be of type object: "string"
* status.supportedFeatures[9]: Invalid value: "string": status.supportedFeatures[9] in body must be of type object: "string"
* status.supportedFeatures[10]: Invalid value: "string": status.supportedFeatures[10] in body must be of type object: "string"
* status.supportedFeatures[11]: Invalid value: "string": status.supportedFeatures[11] in body must be of type object: "string"
* status.supportedFeatures[12]: Invalid value: "string": status.supportedFeatures[12] in body must be of type object: "string"
* status.supportedFeatures[13]: Invalid value: "string": status.supportedFeatures[13] in body must be of type object: "string"
* status.supportedFeatures[14]: Invalid value: "string": status.supportedFeatures[14] in body must be of type object: "string"
* status.supportedFeatures[15]: Invalid value: "string": status.supportedFeatures[15] in body must be of type object: "string"
* status.supportedFeatures[16]: Invalid value: "string": status.supportedFeatures[16] in body must be of type object: "string"
* status.supportedFeatures[17]: Invalid value: "string": status.supportedFeatures[17] in body must be of type object: "string"
* status.supportedFeatures[18]: Invalid value: "string": status.supportedFeatures[18] in body must be of type object: "string"
* status.supportedFeatures[19]: Invalid value: "string": status.supportedFeatures[19] in body must be of type object: "string"
* status.supportedFeatures[20]: Invalid value: "string": status.supportedFeatures[20] in body must be of type object: "string"
* status.supportedFeatures[21]: Invalid value: "string": status.supportedFeatures[21] in body must be of type object: "string"
* <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation

zetaab avatar Nov 13 '24 13:11 zetaab

I dont think we have any released versions supporting that feature, it may have been because of using v0.0.0-latest related gateway api discussion https://kubernetes.slack.com/archives/CR0H13KGA/p1731343804784929

arkodg avatar Nov 14 '24 05:11 arkodg