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.

Sign up for GitHub

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

Jump to bottom

frontend: add support for gateway-api #2504

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

frontend: add support for gateway-api #2504

wants to merge 3 commits into from

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented Oct 30, 2024 •
edited
Loading

image

How to test

Testing with Minikube and Envoy

Testing done

  • smoke tested with minikube and envoy gateway
  • tested it with a remote cluster that doesn't have any gateways used (and it reports not gateways successfully)
  • checked fields against API specs
  • reviewed component states in storybook

Future work

This is stuff out of scope of this PR. (We should probably add an issue for that)

  • experimental features
  • Testing with a service mesh, and mesh related gateway functionality
  • add better http route rule UX (filters, backends,...)
  • add tls UX to gateway
  • add gateway api to map feature
  • ...

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from d09a4e8 to 4fde392 Compare October 30, 2024 21:24
@farodin91
Copy link
Contributor Author

@joaquimrocha I'm not sure, if it's better to move this into a plugin or make gateway more present. This only adds stable resources.

What do you think? Views can be improved.

@illume
Copy link
Collaborator

illume commented Nov 4, 2024

Hi @farodin91

@joaquimrocha (and rest of team) has been busy the last days with very many release related things, and also some holidays.

To plugin or not? The gateway API is part of the Kubernetes project, so in that sense maybe it should be in Headlamp... but yeah... it's not a core Kubernetes thing. The other question to ask, is how commonly it's used? To me it's not entirely clear where it should be. Maybe you could provide more context?

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from 4fde392 to d2fd515 Compare November 6, 2024 20:55
@farodin91
Copy link
Contributor Author

@illume i think Kubernetes is pushing gateway API for the last month quiet hard. I would say. It's now stable.

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch 3 times, most recently from 1e18382 to 05c4bee Compare November 7, 2024 22:14
@illume
Copy link
Collaborator

illume commented Nov 12, 2024

@joaquimrocha Yeah, I agree it should go in headlamp rather than a plugin. This week I was learning about gateway API. Basically it’s the successor to Ingress. My understanding is gateway API is cleaning and standardizing things, and addressing missing spots (L4-L7 vs only http L7). As of the 1.2 release in October, quite a lot of the API is stable. There's several implementations of the http/TLS routers, less of the GRPC routers, and even fewer of the mesh routers (which is the main part left with experimental APIs AFAIK).

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch 3 times, most recently from e232b95 to 35481cd Compare November 16, 2024 14:01
@farodin91 farodin91 marked this pull request as ready for review November 16, 2024 14:07
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Nov 16, 2024
@farodin91
Copy link
Contributor Author

@illume Would you like to review this PR? I think the ui can be improved or extend over time. Their is also a magic route called gateway, so i had to change route to k8sgateway. Do you have an idea why their is a magic route?

illume
illume reviewed Nov 18, 2024
View reviewed changes
frontend/src/components/Sidebar/prepareRoutes.ts Outdated
@@ -166,6 +166,29 @@ function prepareRoutes(
},
],
},
{
name: 'gateway',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about putting these in Network?

  • Gateways
  • Gateway Classes
  • Gateway HTTP Routes
  • Gateway GRPC Routes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but gateway api are multiple resource. it would increase network too much and another idea could be to hide gateway when not installed.

illume
illume reviewed Nov 18, 2024
View reviewed changes
Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some notes about API fields, and also a note about apiVersion should be v1 or both v1 and v1betav1?

frontend/src/lib/k8s/gateway.ts Outdated
import { KubeObject, KubeObjectInterface } from './KubeObject';

export interface GatewayParentReference {
group: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should have documentation.

https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.ParentReference

frontend/src/lib/k8s/gateway.ts Outdated
group: string;
kind: string;
namespace: string;
sectionName: string | null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sectionName is listed as optional. I guess it also needs ?

frontend/src/lib/k8s/gateway.ts Outdated
namespace: string;
sectionName: string | null;
name: string;
[key: string]: any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think port: portNumber is missing here?
https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.ParentReference

frontend/src/lib/k8s/gateway.ts
[key: string]: any;
}
export interface GatewayListenerStatus {
name: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.ListenerStatus

These fields are missing: supportedKinds, attachedRoutes

frontend/src/lib/k8s/gateway.ts Outdated
[key: string]: any;
}
export interface GatewayAddress {
type: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is optional according to here:

https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.GatewayAddress

The AddressType type seems like it can have many implementation specific different strings, and some specified ones. Probably string type is ok, but a comment about the field would be good. https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.AddressType

frontend/src/lib/k8s/grpcRoute.ts Outdated Show resolved Hide resolved
@joaquimrocha
Copy link
Collaborator

@joaquimrocha Yeah, I agree it should go in headlamp rather than a plugin. This week I was learning about gateway API. Basically it’s the successor to Ingress.

Yeah, let's add it to the core.

@illume
Copy link
Collaborator

illume commented Nov 18, 2024 •
edited
Loading

It takes quite a long time spinning on a cluster when there's no gateways. I don't think that should block this being merged, but something to look at at some point.

Same on a cluster with a gateway, but when there is no GRPC routes... the spinner takes quite a while before saying there are no routes.

@illume
Copy link
Collaborator

illume commented Nov 18, 2024 •
edited
Loading

  • I tested it with minikube and Envoy locally. ✅
  • Also tested it with a remote cluster that doesn't have any gateways used. ✅

@illume
Copy link
Collaborator

illume commented Nov 18, 2024

Their is also a magic route called gateway, so i had to change route to k8sgateway. Do you have an idea why their is a magic route?

I can't find such a route in the Headlamp source. Also couldn't see something called 'gateway' in k8s outside of gateway API stuff. What about 'gatewayapi' as an alternative name to k8sgateway? Will have another look in the morning... maybe it's just too late.

@illume
Copy link
Collaborator

illume commented Nov 18, 2024 •
edited
Loading

I think the ui can be improved or extend over time

I haven't tested with a mesh yet. Just from browsing the source, it looks like Mesh Profile isn't done yet? If it's not done, should we add that in future PRs?

It might be good to list anything missing you know of in a future work section of the PR description? Or we can start an issue I guess.

@illume
Copy link
Collaborator

illume commented Nov 18, 2024

btw, I presented Headlamp to some folks at Kubecon and asked them some questions about gateway API. They showed me a policy visualizer. There I also found out about gwctl https://github.com/kubernetes-sigs/gwctl?tab=readme-ov-file#visualizing-relationships-with-dot-graphs-using-gwctl-get--o-graph Also they told me about a gateway api sig meeting where we could take this to show them and get feedback if we want.

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from 35481cd to 62bb37d Compare November 19, 2024 17:25
@farodin91
Copy link
Contributor Author

Their is also a magic route called gateway, so i had to change route to k8sgateway. Do you have an idea why their is a magic route?

I can't find such a route in the Headlamp source. Also couldn't see something called 'gateway' in k8s outside of gateway API stuff. What about 'gatewayapi' as an alternative name to k8sgateway? Will have another look in the morning... maybe it's just too late.

is fixed now. it was the sidebar path.

@farodin91
Copy link
Contributor Author

@illume i have updated the resource. What do you think?

@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch 4 times, most recently from 7f9e7dc to 57d728c Compare November 27, 2024 20:55
@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from 57d728c to fa3ce80 Compare December 7, 2024 19:05
@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from fa3ce80 to 61975b7 Compare December 12, 2024 07:57
@farodin91
fix a small issue
a1a6c60 
Signed-off-by: farodin91 <[email protected]>
@farodin91 farodin91 force-pushed the add-support-for-gateway-api branch from 61975b7 to a1a6c60 Compare December 12, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@illume illume illume left review comments

Assignees
No one assigned
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@farodin91 @illume @joaquimrocha