Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services #1843

robscott · 2023-03-17T23:24:53Z

Recommend reviewing deploy preview so examples are inlined: https://deploy-preview-1843--kubernetes-sigs-gateway-api.netlify.app/geps/gep-1748/.

What type of PR is this?
/kind gep

What this PR does / why we need it:
This formally defines the relationship between Gateway API and Multi-Cluster Services as part of #1748.

Does this PR introduce a user-facing change?:
I think the changelog notes should be saved for actual implementation PR, but open to ideas:

NONE

/hold for consensus
Want at least one LGTM each from SIG-MC, GW API maintainers, and GAMMA leads before removing hold. Feedback from everyone (including people not directly mentioned below) is very appreciated.

SIG-MC:
/cc @lauralorenz @JeremyOT @pmorie

GW API:
/cc @bowei @shaneutt @youngnick

GAMMA
/cc @howardjohn @keithmattix @mikemorris

geps/gep-1748.md

howardjohn · 2023-03-20T15:14:37Z

geps/gep-1748.md

+
+### Services vs ServiceImports
+
+It is important that all implementations provide a consistent experience. That


This is the crux of the issue for me, IMO. Everything else is (to me) obviously correct.

However, this is saying that if you use Gateway API (which, at this point, is the ~entire community), you MUST use MCS for multicluster semantics, and MCS is Alpha and has a some problems blocking adoption.

@howardjohn Could you expand a bit more on "problems blocking adoption"?

From Consul's perspective we do want a broader scope to enable routing to off-cluster destinations without a guarantee of "sameness" (sometimes described as "cross-clusterset"), but we're willing to start with an impl-specific approach for that and work towards expanding the MCS spec to hopefully support that in the future.

geps/gep-1748.md

youngnick · 2023-03-27T07:16:22Z

I will have comments on this, but I haven't had the bandwidth to review properly yet (requires me spending some more time on Cilium's existing Clustermesh to be able to talk about what it can do more intelligibly). Could we give me some time on this one please?

lauralorenz

Little nits suggested.

Representing SIG-Multicluster:
/lgtm

geps/gep-1748.md

examples/standard/multicluster/httproute-gamma.yaml

lauralorenz · 2023-04-04T21:05:32Z

Sorry I did the github-native approve thing and it seems to mean something nowadays 😅 but the hold stands!

shaneutt

On first review I don't have any specific immediate concerns with the contents of the GEP overall.

What I am concerned about is the launch straight to experimental, considering that we consider experimental to be something more like "next", and we don't even have like, conformance tests in mind yet?

geps/gep-1748.md

liwenwu-amazon · 2023-04-05T21:05:15Z

The GEP looks good!. In AWS VPC Lattice controller gateway API/multicluster implementation, VPC Lattice is the federation.

For serviceexport, the lattice controller register all of its endpoints in local cluster to VPC lattice

To reference a serviceimport as a backendRef of a HTTPRoute, the lattice controller will configure VPC lattice resource accordingly. So that the traffic will get routed to endpoints in that remote cluster(VPC) by VPC lattice dataplane.

In addition to K8S endpoints, serviceimport can references targets like EC2, lambda function and ECS container(s).

youngnick · 2023-04-05T22:22:23Z

Because this is extended, this looks fine to me. I don't think we'll be doing this in Cilium for a while though.

/lgtm

zhiying-lin · 2023-04-06T05:48:34Z

geps/gep-1748.md

+endpoints for the local Service within the same cluster.
+
+```yaml
+{% include 'standard/multicluster/httproute-gamma.yaml' %}


just wonder if it's more suitable to define the serviceImport or service as the parentRef in the tcpRoute or udpRoute. To me, both of them are more like l4 resource, instead of l7.

Service as a parentRef for TCPRoute is a viable use case, and is included in GEP-1426.

Supporting HTTPRoute is important for some more advanced configuration, such as redirecting only a specific path (like /admin) to a different Service backendRef when doing something like splitting up a monolith application.

zhiying-lin · 2023-04-06T06:10:26Z

geps/gep-1748.md

+### Mesh: ServiceImport as Parent
+
+In some cases, you may want to override traffic destined for a Multi-Cluster
+Service with a mesh. As part of the broader GAMMA initiative, ServiceImport can


do we have any doc to describe how mesh integrates with gateway api?

Yes, a short high-level summary is at https://gateway-api.sigs.k8s.io/contributing/gamma/, with more detail in GEP-1324 (describing the intent for expanding Gateway API to E/W and service mesh use cases) and GEP-1426 (describing a proposed concrete implementation).

mikemorris

Proposed two alternative suggestions for GAMMA examples, but generally supportive of this with the change to

Services SHOULD always be interpreted as references to endpoints within the same cluster

Further comments on specific behavior can be found in #1843 (comment)

/lgtm

geps/gep-1748.md

mikemorris · 2023-04-06T17:26:40Z

examples/standard/multicluster/httproute-gamma.yaml

+spec:
+  parentRefs:
+  - group: multicluster.x-k8s.io
+    kind: ServiceImport
+    name: store
+  rules:
+  - backendRefs:
+    - kind: Service
+      name: store
+      port: 8080


Suggested change

spec:

parentRefs:

- group: multicluster.x-k8s.io

kind: ServiceImport

name: store

rules:

- backendRefs:

- kind: Service

name: store

port: 8080

spec:

parentRefs:

- group: multicluster.x-k8s.io

kind: ServiceImport

name: store

rules:

- matches:

- path:

value: "/cart"

backendRefs:

- group: multicluster.x-k8s.io

kind: ServiceImport

name: cart

After reading through the updated content, I think splitting off one path from a ServiceImport to a different ServiceImport would be a better example for illustrating ServiceImport as a parentRef than redirecting ClusterSetIP traffic to stay cluster-local.

Configuring traffic to a ServiceImport ClusterSetIP to instead stay cluster-local feels a bit odd (and possibly more natural to configure by either not exporting the service in the first place or blocking store.svc.clusterset.local with egress policy).

Would we additionally want to add an example for redirecting ClusterIP traffic to a Multi-Cluster Service, or should we wait on explicitly endorsing that?

parentRefs: - kind: Service name: store rules: - backendRefs: - group: multicluster.x-k8s.io kind: ServiceImport name: store

Updated, thanks! Opted to leave out the additional example since this is about to merge, and don't want to increase scope for others that have already approved. Probably worth a follow up though.

robscott · 2023-04-06T17:57:10Z

Thanks for all the feedback everyone. I think we've got consensus here, going to remove the hold so the next LGTM merges.

/hold cancel

shaneutt

/lgtm

k8s-ci-robot · 2023-04-06T18:51:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: keithmattix, lauralorenz, mikemorris, robscott, shaneutt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [robscott,shaneutt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested a review from bowei March 17, 2023 23:24

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 17, 2023

k8s-ci-robot requested a review from howardjohn March 17, 2023 23:24

k8s-ci-robot added the kind/gep PRs related to Gateway Enhancement Proposal(GEP) label Mar 17, 2023

k8s-ci-robot requested review from JeremyOT, keithmattix, lauralorenz, mikemorris, pmorie, shaneutt and youngnick March 17, 2023 23:24

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 17, 2023

robscott force-pushed the gep-1748 branch 3 times, most recently from c64f10a to 2cc1153 Compare March 17, 2023 23:59

shaneutt added this to the v1.0.0 milestone Mar 20, 2023

howardjohn reviewed Mar 20, 2023

View reviewed changes

keithmattix reviewed Mar 20, 2023

View reviewed changes

geps/gep-1748.md Outdated Show resolved Hide resolved

mikemorris reviewed Mar 20, 2023

View reviewed changes

geps/gep-1748.md Outdated Show resolved Hide resolved

geps/gep-1748.md Outdated Show resolved Hide resolved

robscott force-pushed the gep-1748 branch from 2cc1153 to 597411a Compare March 24, 2023 23:47

lauralorenz reviewed Apr 4, 2023

View reviewed changes

geps/gep-1748.md Outdated Show resolved Hide resolved

examples/standard/multicluster/httproute-gamma.yaml Outdated Show resolved Hide resolved

k8s-ci-robot assigned lauralorenz Apr 4, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2023

lauralorenz approved these changes Apr 4, 2023

View reviewed changes

keithmattix approved these changes Apr 4, 2023

View reviewed changes

robscott force-pushed the gep-1748 branch from 597411a to ad486d2 Compare April 5, 2023 00:49

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 5, 2023

shaneutt requested changes Apr 5, 2023

View reviewed changes

geps/gep-1748.md Show resolved Hide resolved

shaneutt approved these changes Apr 5, 2023

View reviewed changes

k8s-ci-robot assigned youngnick Apr 5, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 5, 2023

zhiying-lin reviewed Apr 6, 2023

View reviewed changes

mikemorris approved these changes Apr 6, 2023

View reviewed changes

k8s-ci-robot assigned mikemorris Apr 6, 2023

robscott force-pushed the gep-1748 branch from ad486d2 to 42133cc Compare April 6, 2023 17:55

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 6, 2023

robscott mentioned this pull request Apr 6, 2023

Marking ServiceImport as Extended support #1922

Merged

Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services

7c042ed

robscott force-pushed the gep-1748 branch from 42133cc to 7c042ed Compare April 6, 2023 18:14

shaneutt approved these changes Apr 6, 2023

View reviewed changes

k8s-ci-robot assigned shaneutt Apr 6, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2023

shaneutt modified the milestones: v1.0.0, v0.7.0 Apr 6, 2023

k8s-ci-robot merged commit b1db26f into kubernetes-sigs:main Apr 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services #1843

Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services #1843

robscott commented Mar 17, 2023 •

edited

Loading

howardjohn Mar 20, 2023

mikemorris Mar 20, 2023

youngnick commented Mar 27, 2023

lauralorenz left a comment

lauralorenz commented Apr 4, 2023

shaneutt left a comment •

edited

Loading

liwenwu-amazon commented Apr 5, 2023

youngnick commented Apr 5, 2023

zhiying-lin Apr 6, 2023

mikemorris Apr 6, 2023 •

edited

Loading

zhiying-lin Apr 6, 2023

mikemorris Apr 6, 2023 •

edited

Loading

mikemorris left a comment •

edited

Loading

mikemorris Apr 6, 2023

mikemorris Apr 6, 2023

robscott Apr 6, 2023

robscott commented Apr 6, 2023

shaneutt left a comment

k8s-ci-robot commented Apr 6, 2023


		### Services vs ServiceImports

		It is important that all implementations provide a consistent experience. That

Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services #1843

Adding GEP-1748: Gateway API Interaction with Multi-Cluster Services #1843

Conversation

robscott commented Mar 17, 2023 • edited Loading

howardjohn Mar 20, 2023

Choose a reason for hiding this comment

mikemorris Mar 20, 2023

Choose a reason for hiding this comment

youngnick commented Mar 27, 2023

lauralorenz left a comment

Choose a reason for hiding this comment

lauralorenz commented Apr 4, 2023

shaneutt left a comment • edited Loading

Choose a reason for hiding this comment

liwenwu-amazon commented Apr 5, 2023

youngnick commented Apr 5, 2023

zhiying-lin Apr 6, 2023

Choose a reason for hiding this comment

mikemorris Apr 6, 2023 • edited Loading

Choose a reason for hiding this comment

zhiying-lin Apr 6, 2023

Choose a reason for hiding this comment

mikemorris Apr 6, 2023 • edited Loading

Choose a reason for hiding this comment

mikemorris left a comment • edited Loading

Choose a reason for hiding this comment

mikemorris Apr 6, 2023

Choose a reason for hiding this comment

mikemorris Apr 6, 2023

Choose a reason for hiding this comment

robscott Apr 6, 2023

Choose a reason for hiding this comment

robscott commented Apr 6, 2023

shaneutt left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 6, 2023

robscott commented Mar 17, 2023 •

edited

Loading

shaneutt left a comment •

edited

Loading

mikemorris Apr 6, 2023 •

edited

Loading

mikemorris Apr 6, 2023 •

edited

Loading

mikemorris left a comment •

edited

Loading