refactor telemetry backendRefs #3293

zirain · 2024-04-29T02:49:39Z

fix: #3161

Signed-off-by: zirain <zirain2009@gmail.com>

internal/gatewayapi/listener.go

internal/ir/xds.go

internal/provider/kubernetes/controller.go

internal/gatewayapi/listener.go

internal/gatewayapi/route.go

Signed-off-by: zirain <zirain2009@gmail.com>

shawnh2

LGTM! no questions from my side.

internal/ir/xds.go

arkodg · 2024-05-16T23:27:50Z

internal/provider/kubernetes/controller.go

@@ -226,18 +239,6 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques
 			gwcResource.Namespaces = append(gwcResource.Namespaces, namespace)
 		}

-		// Process the parametersRef of the accepted GatewayClass.


curious why we moved this away from L241

need update allAssociatedBackendRefs first.

arkodg · 2024-05-16T23:39:56Z

internal/provider/kubernetes/controller.go

@@ -1465,6 +1466,60 @@ func (r *gatewayAPIReconciler) processParamsRef(ctx context.Context, gc *gwapiv1
 				validationErr = fmt.Errorf("invalid envoyproxy: %w", err)
 				continue
 			}
+


we probably also need to enhance

gateway/internal/provider/kubernetes/predicates.go

Line 217 in e5f8630

func (r *gatewayAPIReconciler) validateServiceForReconcile(obj client.Object) bool {

to make sure we trigger a reconcile when the Service changes

yeah, but want to do that in an seperated PR.

Signed-off-by: zirain <zirain2009@gmail.com>

internal/xds/translator/testdata/out/xds-ir/tracing.clusters.yaml

internal/xds/translator/testdata/out/xds-ir/tracing-endpoint-stats.listeners.yaml

Signed-off-by: zirain <zirain2009@gmail.com>

internal/xds/translator/testdata/out/xds-ir/tracing-endpoint-stats.clusters.yaml

arkodg · 2024-05-20T23:02:48Z

internal/gatewayapi/listener.go

+					return nil, err
+				}
+				al.Destination = ir.RouteDestination{
+					Name:     fmt.Sprintf("accesslog-%d", idx), // TODO: rename this, so that we can share backend with tracing?


+1 to this prefix, once we add cluster level features like circuitBreaking which will live on backendRef level , we won't be able to reuse tracing- here

but we should reuse with tracing provider, normally there's only one collector per cluster.

it may not be possible to reuse if we decide to put features inside backendRef field, if feature live in top level policy, it may be possible

arkodg · 2024-05-20T23:04:03Z

internal/gatewayapi/listener.go

-	var port uint32
-	if tracing.Provider.Host != nil {
-		host, port = *tracing.Provider.Host, uint32(tracing.Provider.Port)
+	// TODO: how to get authority from the backendRefs?


name.namespace ?

what about a service outside of cluster.

yah that we'd need to add an explicit field in the future

codecov · 2024-05-21T00:15:10Z

Codecov Report

Attention: Patch coverage is 69.68085% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 67.26%. Comparing base (926d572) to head (91e990f).
Report is 14 commits behind head on main.

Files	Patch %	Lines
internal/provider/kubernetes/controller.go	4.54%	40 Missing and 2 partials ⚠️
internal/gatewayapi/route.go	79.41%	5 Missing and 2 partials ⚠️
internal/gatewayapi/listener.go	95.23%	2 Missing and 2 partials ⚠️
internal/gatewayapi/status/gateway.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3293      +/-   ##
==========================================
- Coverage   67.37%   67.26%   -0.12%     
==========================================
  Files         166      166              
  Lines       19357    19438      +81     
==========================================
+ Hits        13041    13074      +33     
- Misses       5376     5421      +45     
- Partials      940      943       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: zirain <zirain2009@gmail.com>

arkodg

LGTM, thanks !

zirain requested a review from a team as a code owner April 29, 2024 02:49

zirain force-pushed the telemetry-backend branch from 08ca96d to 0b3d6bb Compare April 29, 2024 21:14

zirain added 5 commits April 30, 2024 05:34

refactor telemetry backendRefs

9731e6b

Signed-off-by: zirain <zirain2009@gmail.com>

update Gateway condition

e70b1e8

Signed-off-by: zirain <zirain2009@gmail.com>

lint

0d2b332

Signed-off-by: zirain <zirain2009@gmail.com>

processParamsRef before processBackendRefs

aa2e267

Signed-off-by: zirain <zirain2009@gmail.com>

fix test

246a49f

Signed-off-by: zirain <zirain2009@gmail.com>

zirain force-pushed the telemetry-backend branch from 0b3d6bb to 246a49f Compare April 29, 2024 21:34

zirain commented Apr 30, 2024

View reviewed changes

internal/gatewayapi/listener.go Outdated Show resolved Hide resolved

zirain added 2 commits April 30, 2024 22:12

Merge branch 'main' into telemetry-backend

df8beb4

Merge branch 'main' into telemetry-backend

ee2032a

zirain changed the title ~~[WIP] refactor telemetry backendRefs~~ refactor telemetry backendRefs May 1, 2024

zirain added 3 commits May 3, 2024 22:25

Merge branch 'main' into telemetry-backend

4729e37

Merge branch 'main' into telemetry-backend

7591e44

Merge branch 'main' into telemetry-backend

ea5d012

shawnh2 reviewed May 8, 2024

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

internal/provider/kubernetes/controller.go Show resolved Hide resolved

internal/gatewayapi/listener.go Outdated Show resolved Hide resolved

internal/gatewayapi/route.go Outdated Show resolved Hide resolved

address comments

208dd91

Signed-off-by: zirain <zirain2009@gmail.com>

zirain requested a review from shawnh2 May 9, 2024 06:10

shawnh2 approved these changes May 9, 2024

View reviewed changes

zirain requested a review from arkodg May 9, 2024 21:11

Merge branch 'main' into telemetry-backend

74535d1

arkodg reviewed May 16, 2024

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

zirain mentioned this pull request May 16, 2024

probably want to use RouteDestination here #3410

Closed

arkodg reviewed May 16, 2024

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

arkodg reviewed May 16, 2024

View reviewed changes

zirain mentioned this pull request May 17, 2024

Trigger a reconcile when the Otel backendRefs / Service changes #3412

Closed

zirain added 2 commits May 17, 2024 13:09

use destinations

3c1d844

Signed-off-by: zirain <zirain2009@gmail.com>

refactor

bc2883e

Signed-off-by: zirain <zirain2009@gmail.com>

zirain force-pushed the telemetry-backend branch from 809c8c5 to bc2883e Compare May 17, 2024 06:28

zirain added 2 commits May 17, 2024 14:40

lint

ebee69b

Signed-off-by: zirain <zirain2009@gmail.com>

yamllint

c92777e

Signed-off-by: zirain <zirain2009@gmail.com>

arkodg reviewed May 17, 2024

View reviewed changes

internal/xds/translator/testdata/out/xds-ir/tracing.clusters.yaml Outdated Show resolved Hide resolved

arkodg reviewed May 17, 2024

View reviewed changes

internal/xds/translator/testdata/out/xds-ir/tracing-endpoint-stats.listeners.yaml Show resolved Hide resolved

zirain added 4 commits May 18, 2024 09:28

fix authority

29886c7

Signed-off-by: zirain <zirain2009@gmail.com>

address TODO

ac7065b

Signed-off-by: zirain <zirain2009@gmail.com>

fix protocol

2cd8738

Signed-off-by: zirain <zirain2009@gmail.com>

fix lint

c451672

Signed-off-by: zirain <zirain2009@gmail.com>

zirain requested a review from arkodg May 20, 2024 01:07

arkodg reviewed May 20, 2024

View reviewed changes

internal/xds/translator/testdata/out/xds-ir/tracing-endpoint-stats.clusters.yaml Outdated Show resolved Hide resolved

arkodg reviewed May 20, 2024

View reviewed changes

zirain added 2 commits May 21, 2024 08:18

update test

0fa64ed

Signed-off-by: zirain <zirain2009@gmail.com>

Merge branch 'main' into telemetry-backend

654997f

zirain force-pushed the telemetry-backend branch from aa582af to 654997f Compare May 21, 2024 00:21

remove status.RemoveGatewayListenersNotValidCondition

91e990f

Signed-off-by: zirain <zirain2009@gmail.com>

zirain requested a review from arkodg May 22, 2024 09:32

arkodg approved these changes May 22, 2024

View reviewed changes

arkodg merged commit 5aded1e into envoyproxy:main May 22, 2024
23 checks passed

zirain deleted the telemetry-backend branch May 23, 2024 00:39

This was referenced May 23, 2024

Pluralize backendRefs for non xRoute BackendRef #3091

Open

feat: Reconcile BackendRefs on EnvoyProxy #3190

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor telemetry backendRefs #3293

refactor telemetry backendRefs #3293

zirain commented Apr 29, 2024

shawnh2 left a comment

arkodg May 16, 2024

zirain May 17, 2024

arkodg May 16, 2024

zirain May 17, 2024

arkodg May 20, 2024

zirain May 21, 2024

arkodg May 21, 2024

arkodg May 20, 2024

zirain May 21, 2024

arkodg May 21, 2024

codecov bot commented May 21, 2024 •

edited

arkodg left a comment

refactor telemetry backendRefs #3293

refactor telemetry backendRefs #3293

Conversation

zirain commented Apr 29, 2024

shawnh2 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

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

codecov bot commented May 21, 2024 • edited

Codecov Report

arkodg left a comment

Choose a reason for hiding this comment

codecov bot commented May 21, 2024 •

edited