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

feat(gateway-apis): update to grpc routes v1 + update crds to v1.2.0 #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

larivierec
Copy link
Contributor

This PR bumps some k8s libraries to latest and updates crds to gateway-apis v1.2.0.
GRPC has been updated to v1.

Tests:

---
apiVersion: gateway.networking.k8s.io/v1
kind: GRPCRoute
metadata:
  name: blocky-grpc
  namespace: networking
spec:
  hostnames:
    - blocky-grpc.garb.dev
  parentRefs:
    - group: gateway.networking.k8s.io
      kind: Gateway
      name: cilium-internal
      namespace: networking
  rules:
    - backendRefs:
        - group: ""
          kind: Service
          name: blocky-app
          namespace: networking
          port: 4000
          weight: 1
      matches: []

this drops support for GRPCRoute v1alpha1 as it is no longer supported by GW APIs

debug logs with both tests
command used for testing the above yaml.

dig blocky-grpc.garb.dev @127.0.0.1

[INFO] plugin/k8s_gateway: Building k8s_gateway controller
[INFO] plugin/k8s_gateway: VirtualServer CRDs are not found. Not syncing VirtualServer resources.
[INFO] plugin/k8s_gateway: Starting k8s_gateway controller
[INFO] plugin/k8s_gateway: Waiting for controllers to sync
.:53
[INFO] plugin/reload: Running configuration SHA512 = efa9a3950d8ec51915d7250786e61456e5e280f223ddbce98509d19250659b6da36babb4bd13be78e0b06b1cf3824a1af9015be79510983a7b3d8a82e2e3b10d
CoreDNS-1.11.3+k8s_gateway-0.4.1
darwin/arm64, go1.22.1, 
[DEBUG] plugin/k8s_gateway: Request 4642370252174313057.2047300512947879040. has not matched any zones [garb.dev.]
[INFO] 127.0.0.1:59256 - 13578 "HINFO IN 4642370252174313057.2047300512947879040. udp 57 false 512" - - 0 0.00032275s
[ERROR] plugin/errors: 2 4642370252174313057.2047300512947879040. HINFO: plugin/loop: no next plugin found
[INFO] plugin/k8s_gateway: Synced all required resources
[DEBUG] plugin/k8s_gateway: Adding index blocky-grpc for grpcRoute blocky-grpc.garb.dev
[DEBUG] plugin/k8s_gateway: Computed Index Keys [blocky-grpc.garb.dev blocky-grpc]
[DEBUG] plugin/k8s_gateway: Found 0 matching httpRoute objects
[DEBUG] plugin/k8s_gateway: Found 0 matching tlsRoute objects
[DEBUG] plugin/k8s_gateway: Found 1 matching grpcRoute objects
[DEBUG] plugin/k8s_gateway: Found 1 matching gateway objects
[DEBUG] plugin/k8s_gateway: Computed response addresses [192.168.40.20]
[INFO] 127.0.0.1:63261 - 25330 "A IN blocky-grpc.garb.dev. udp 49 false 4096" NOERROR qr,aa,rd 74 0.00087725s
[DEBUG] plugin/k8s_gateway: Computed Index Keys [blocky-grpc.garb.dev blocky-grpc]
[DEBUG] plugin/k8s_gateway: Found 0 matching httpRoute objects
[DEBUG] plugin/k8s_gateway: Found 0 matching tlsRoute objects
[DEBUG] plugin/k8s_gateway: Found 1 matching grpcRoute objects
[DEBUG] plugin/k8s_gateway: Found 1 matching gateway objects
[DEBUG] plugin/k8s_gateway: Computed response addresses [192.168.40.20]
[INFO] 127.0.0.1:56346 - 53965 "A IN blocky-grpc.garb.dev. udp 49 false 4096" NOERROR qr,aa,rd 74 0.000334583s

I did not re-work the logic as it's pretty much the same as the others.

Signed-off-by: Christopher Larivière <lariviere.c@gmail.com>
@solidDoWant
Copy link
Contributor

Not the maintainer, but I found a couple of somewhat minor issues with this patch:

  • The base image (Go image) tag for the release build does not match the go.mod minimum version
  • This unfortunately still requires the v1alpha2 API (due to TLSRoute suppport), and the plugin will crash without it installed
  • This change will only load gateway v1 resources, and not older v1beta1, v1 alpha1/2 resources, which isn't backwards compatible

I started working on a patch to fix these issues, but unfortunately it looks like it'll require some pretty sweeping changes and I don't have the time to commit to this right now. If anybody is interested in taking on this work, this function needs the following changes for every supported resource:

  • Prior to checking if the resource needs to be loaded, the function should check if the user wants the resource to be loaded. The list of resources that the user has configured to be loaded is defined in the caller as gw.Resources and needs to be plumbed through here.
  • Rather than testing if the gateway APIs collectively exist here, each version of each CRD needs to be individually checked, and each CRD version of each CRD needs a separate controller loaded.
  • The indexer functions starting here need to generate a unique index for each resource in each namespace for each version of each CRD, rather than creating an index based upon namespace/name alone. This could possibly use the UID field in the resource metadata.
  • The lookup functions starting here need to be updated with the corresponding changes for each indexer function.

@solidDoWant
Copy link
Contributor

If anybody is interested in using this PR, I've pushed an image here built on this branch, with the Dockerfile fix included.

@larivierec
Copy link
Contributor Author

larivierec commented Nov 9, 2024

Nice catch and yeah, there's a lot of changes required to properly support this. Thanks for dockerfile fix.
also if you're interested, I started doing work on adding DNSEndpoint but I haven't updated it there's a lot more records that would need supporting -> #305 if you're interested

@solidDoWant
Copy link
Contributor

That's really cool. It'd be great to get better integration with external-dns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants