Create IR struct to hold specs for extension features. #185

sawsa307 · 2024-08-21T16:38:21Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Add structures needed to support translation of extension features.

Which issue(s) this PR fixes:

Fixes #125

Does this PR introduce a user-facing change?:

None

sawsa307 · 2024-08-21T16:38:56Z

/assign @LiorLieberman

sawsa307 · 2024-08-21T16:40:43Z

/assign @arkodg

sawsa307 · 2024-08-21T16:41:08Z

/assign @robscott

pkg/i2gw/provider.go

arkodg · 2024-08-21T17:27:28Z

pkg/i2gw/provider.go

+	GatewayExtensions map[types.NamespacedName]unstructured.Unstructured
+}
+
+// IR contains all Gateway-API objects, and Intermediate Representation(IR) to


curious what's going to happen to the GatewayResources struct

shouldnt the IR replace it ?

IR and Gateway Resources are for different translation purpuse. What I have in mind is:

A pool of resources
-------> Ingress + Ingress extension (By input provider reader)
-------> IR, or Gateway/HTTPRoute context(By input provider converter)
-------> Gateway + Gateway extension(By output provider extension converter)

Let's say we are doing translation of enableHTTPSRedirect feature, we will have:

type GceGatewayIR struct {
EnableHTTPSRedirect bool
}
type GceHTTPRouteIR struct {
EnableHTTPSRedirect bool
}
since HTTPS Redirect is supported by modifying the Gateway with an additional HTTPRoute to redirect HTTP traffics.

And during translation, what would happen is:
Ingress+Ingress extension
--->IR.GceGatewayIR.EnableHTTPSRedirect=true, IR.GceHTTPRouteIR.EnableHTTPSRedirect=true(by reading corresponding Ingress extension)
--->Gateway with additional HTTPRoute

Gateway extensions will be translated in a similar manner.

pkg/i2gw/provider.go

LiorLieberman · 2024-08-21T18:09:39Z

/cc

LiorLieberman

Can we see one feature that will work with the IR, ideally in this PR as a different commit, so we can look at these independently (and we wont squash the PR when we merge, to keep the separation)?

pkg/i2gw/provider.go

pkg/i2gw/ingress2gateway.go

LiorLieberman · 2024-09-04T09:02:26Z

pkg/i2gw/ingress2gateway.go

 }

-func mergeGateways(gatewaResources []GatewayResources) (map[types.NamespacedName]gatewayv1.Gateway, field.ErrorList) {
-	newGateways := map[types.NamespacedName]gatewayv1.Gateway{}
+func mergeGateways(irs []IR) (map[types.NamespacedName]GatewayContext, field.ErrorList) {


Since you move to merge GatewayContext and not just gateways, do we need to change this function to also merge the IR fields within the gatewayContexts?

Similar comment for HTTPRouteContext

Updated for GatewayContext. We don't currently perform any operations on HTTPRouteContext other than putting them together. There is no merge on HTTPRoutes.

before, we had MergeGatewayResources, right?
This would have merged anything under GatewayResources, including Gateways, GatewayRoutes, HTTPRoutes, etc. (we can see this here on the left panel)

My question is how maps.Copy(mergedIRs.HTTPRoutes, gr.HTTPRoutes) would work given that HTTPRouteContext has nested HTTPRouteIR field with its own nested fields.
Maybe maps.copy is enough for us, idk.. but something to check, I am not sure it will work out of the box

They should work. I've tried to run tests on functions using this helper, and they passed with expected merged results.

Well thats because you have nothing on the http IR yet. similarly to the logic you employ for the GatewayIR merging in db03324 you would have to add logic to merge the HTTPIRs and ServiceIR, maps.copy() would not cover that.

pkg/i2gw/ingress2gateway.go

pkg/i2gw/provider.go

robscott

Thanks for all the great work on this @sawsa307! Mostly focused on GCP section with this review

robscott · 2024-09-04T22:56:12Z

pkg/i2gw/providers/gce/gce_extensions.go

+	backendPolicy := gkegatewayv1.GCPBackendPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: serviceNamespacedName.Namespace,
+			Name:      serviceNamespacedName.Name + "-GCPBackendPolicy",


Not quite sure if we have prior art here, but since this could at least theoretically overwrite existing config in a cluster, it might be safer to use GenerateName instead of Name. On the other hand, that would mean that every time this manifest was applied, you'd get a new GCPBackendPolicy.

cc @LiorLieberman @levikobi for any additional perspective on which is the least bad approach here.

I see we are hand-crafting names for translated Gateway and HTTPRoute based on the ingress name and port, which is why I went ahead and generate a name by appending a suffix, given we can only attach one GCPBackendPolicy to a k8s Service. Definitely would like more suggestions here on the naming schema.

That's fair, maybe just remove the -GCPBackendPolicy suffix as that's already implied by the type of resource it is.

pkg/i2gw/providers/gce/gce_extensions.go

pkg/i2gw/providers/gce/gce_extensions_test.go

pkg/i2gw/providers/gce/storage.go

pkg/i2gw/ir/provider_istio.go

pkg/i2gw/provider.go

LiorLieberman · 2024-09-05T08:11:57Z

pkg/i2gw/ingress2gateway.go

 }

-func mergeGateways(gatewaResources []GatewayResources) (map[types.NamespacedName]gatewayv1.Gateway, field.ErrorList) {
-	newGateways := map[types.NamespacedName]gatewayv1.Gateway{}
+func mergeGateways(irs []IR) (map[types.NamespacedName]GatewayContext, field.ErrorList) {


before, we had MergeGatewayResources, right?
This would have merged anything under GatewayResources, including Gateways, GatewayRoutes, HTTPRoutes, etc. (we can see this here on the left panel)

My question is how maps.Copy(mergedIRs.HTTPRoutes, gr.HTTPRoutes) would work given that HTTPRouteContext has nested HTTPRouteIR field with its own nested fields.
Maybe maps.copy is enough for us, idk.. but something to check, I am not sure it will work out of the box

pkg/i2gw/ingress2gateway.go

LiorLieberman · 2024-09-05T08:21:22Z

pkg/i2gw/ingress2gateway.go

-		providerGatewayResources, conversionErrs := provider.ToGatewayAPI()
+		ir, conversionErrs := provider.ToIR()
+		errs = append(errs, conversionErrs...)
+		providerGatewayResources, conversionErrs := provider.ToGatewayResources(ir)


sawsa307 · 2024-09-05T09:15:17Z

Discussed with Lior, since this PR is getting too big, will separate the logic into individual PRs. #188 will be the new PR to include definitions of IRs.

robscott

Thanks @sawsa307! This all LGTM, left a few nits but nothing blocking.

pkg/i2gw/providers/gce/gce.go

pkg/i2gw/providers/gce/gce_extensions.go

pkg/i2gw/providers/gce/gce_extensions_test.go

pkg/i2gw/providers/gce/storage.go

k8s-ci-robot · 2024-09-10T05:13:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: robscott, sawsa307

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]

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

* Create IR struct on Gateway, HTTPRoute, and Service to hold extensions. * Add merge functions for GceGatewayIR. * Add TODO and links to existing ingress2gateway issue.

* Add BackendConfig to resource_reader. * go.mod requires go 1.22.4, while golangci-lint 1.55.2 is built on go 1.21. This would cause an issue to load go package. "go.mod requires go >= 1.22.1 (running go 1.22.0; GOTOOLCHAIN=local)" * Thus, we need to update to the latest goclint-ci version.

* Providers now should implement ResourcesToIRConverter interface and IRToGatewayAPIConverter interface.

sawsa307 · 2024-09-10T16:02:40Z

Closing in favor of #188.

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 21, 2024

k8s-ci-robot requested review from levikobi and Xunzhuo August 21, 2024 16:38

sawsa307 changed the title Ir Create IR struct to hold specs for extension features. Aug 21, 2024

sawsa307 temporarily deployed to release August 21, 2024 16:38 — with GitHub Actions Inactive

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 21, 2024

k8s-ci-robot assigned LiorLieberman Aug 21, 2024

k8s-ci-robot assigned arkodg Aug 21, 2024

k8s-ci-robot assigned robscott Aug 21, 2024

arkodg reviewed Aug 21, 2024

View reviewed changes

pkg/i2gw/provider.go Outdated Show resolved Hide resolved

arkodg reviewed Aug 21, 2024

View reviewed changes

pkg/i2gw/provider.go Outdated Show resolved Hide resolved

k8s-ci-robot requested a review from LiorLieberman August 21, 2024 18:09

sawsa307 force-pushed the ir branch from bb813b5 to a900076 Compare August 21, 2024 18:33

sawsa307 temporarily deployed to release August 21, 2024 18:33 — with GitHub Actions Inactive

LiorLieberman reviewed Aug 22, 2024

View reviewed changes

pkg/i2gw/provider.go Outdated Show resolved Hide resolved

sawsa307 force-pushed the ir branch from a900076 to 3b4ca20 Compare August 23, 2024 07:59

sawsa307 temporarily deployed to release August 23, 2024 07:59 — with GitHub Actions Inactive

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 23, 2024

sawsa307 requested a review from LiorLieberman August 23, 2024 07:59

sawsa307 force-pushed the ir branch from 3b4ca20 to ec555df Compare August 23, 2024 17:26

sawsa307 temporarily deployed to release August 23, 2024 17:26 — with GitHub Actions Inactive

sawsa307 force-pushed the ir branch from ec555df to 7f1ee8d Compare August 23, 2024 20:45

sawsa307 temporarily deployed to release August 23, 2024 20:45 — with GitHub Actions Inactive

sawsa307 requested a review from robscott August 29, 2024 16:19

sawsa307 force-pushed the ir branch from bbf81b7 to 053ddfe Compare September 3, 2024 15:38

sawsa307 temporarily deployed to release September 3, 2024 15:38 — with GitHub Actions Inactive

LiorLieberman reviewed Sep 4, 2024

View reviewed changes

sawsa307 force-pushed the ir branch from 053ddfe to dbd750a Compare September 4, 2024 18:14

sawsa307 temporarily deployed to release September 4, 2024 18:14 — with GitHub Actions Inactive

sawsa307 force-pushed the ir branch from dbd750a to 1d4d98e Compare September 4, 2024 18:28

sawsa307 temporarily deployed to release September 4, 2024 18:28 — with GitHub Actions Inactive

sawsa307 force-pushed the ir branch from 1d4d98e to 2774404 Compare September 4, 2024 22:29

sawsa307 temporarily deployed to release September 4, 2024 22:29 — with GitHub Actions Inactive

robscott reviewed Sep 4, 2024

View reviewed changes

sawsa307 force-pushed the ir branch from 2774404 to af84597 Compare September 5, 2024 04:05

sawsa307 temporarily deployed to release September 5, 2024 04:05 — with GitHub Actions Inactive

LiorLieberman reviewed Sep 5, 2024

View reviewed changes

LiorLieberman mentioned this pull request Sep 5, 2024

Create IR structs, and merge functions for IRs. #188

Merged

robscott approved these changes Sep 10, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2024

sawsa307 force-pushed the ir branch from af84597 to 5364bc8 Compare September 10, 2024 06:37

sawsa307 temporarily deployed to release September 10, 2024 06:37 — with GitHub Actions Inactive

sawsa307 added 4 commits September 10, 2024 15:58

Create IR struct to hold specs for extension features.

3c8e76b

* Create IR struct on Gateway, HTTPRoute, and Service to hold extensions. * Add merge functions for GceGatewayIR. * Add TODO and links to existing ingress2gateway issue.

Add GCE session affinity translation

0777d27

Migrate all providers to the new interface.

82c9d8e

* Providers now should implement ResourcesToIRConverter interface and IRToGatewayAPIConverter interface.

sawsa307 force-pushed the ir branch from 5364bc8 to 82c9d8e Compare September 10, 2024 16:00

sawsa307 temporarily deployed to release September 10, 2024 16:00 — with GitHub Actions Inactive

sawsa307 closed this Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create IR struct to hold specs for extension features. #185

Create IR struct to hold specs for extension features. #185

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

arkodg Aug 21, 2024

arkodg Aug 21, 2024

sawsa307 Aug 21, 2024 •

edited

Loading

sawsa307 Aug 21, 2024 •

edited

Loading

LiorLieberman commented Aug 21, 2024

LiorLieberman left a comment

LiorLieberman Sep 4, 2024

sawsa307 Sep 4, 2024

LiorLieberman Sep 5, 2024

sawsa307 Sep 5, 2024

LiorLieberman Sep 5, 2024

robscott left a comment

robscott Sep 4, 2024

sawsa307 Sep 5, 2024 •

edited

Loading

robscott Sep 10, 2024

LiorLieberman Sep 5, 2024

LiorLieberman Sep 5, 2024

sawsa307 commented Sep 5, 2024

robscott left a comment

k8s-ci-robot commented Sep 10, 2024

sawsa307 commented Sep 10, 2024

Create IR struct to hold specs for extension features. #185

Create IR struct to hold specs for extension features. #185

Conversation

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

sawsa307 commented Aug 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawsa307 Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

sawsa307 Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

LiorLieberman commented Aug 21, 2024

LiorLieberman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robscott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawsa307 Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawsa307 commented Sep 5, 2024

robscott left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 10, 2024

sawsa307 commented Sep 10, 2024

sawsa307 Aug 21, 2024 •

edited

Loading

sawsa307 Aug 21, 2024 •

edited

Loading

sawsa307 Sep 5, 2024 •

edited

Loading