Skip to content

Commit 362a219

Browse files
u-kaiu-kai
committed
Refactor
1 parent d89f80e commit 362a219

File tree

5 files changed

+46
-54
lines changed

5 files changed

+46
-54
lines changed

‎pkg/controllers/route_controller.go‎

Lines changed: 36 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -216,33 +216,38 @@ func (r *routeReconciler) getRoute(ctx context.Context, req ctrl.Request) (core.
216216
}
217217

218218
func updateRouteListenerStatus(ctx context.Context, k8sClient client.Client, route core.Route) error {
219-
gw, err := findFirstParentGw(ctx, k8sClient, route)
220-
if err != nil {
219+
gws, err := findControlledParents(ctx, k8sClient, route)
220+
if len(gws) <= 0 {
221221
return fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
222222
}
223+
// TODO assume one parent for now and point to service network
224+
gw := gws[0]
223225
return UpdateGWListenerStatus(ctx, k8sClient, gw)
226+
224227
}
225228

226229
func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) bool {
227230
if len(route.Spec().ParentRefs()) == 0 {
228231
r.log.Infof(ctx, "Ignore Route which has no ParentRefs gateway %s ", route.Name())
229232
return false
230233
}
231-
// if route has gateway parentRef that is managed by lattice gateway controller,
234+
// if route has gateway parentRef that is controlled by lattice gateway controller,
232235
// then it is relevant
233-
gw, err := findFirstParentGw(ctx, r.client, route)
234-
return gw != nil && err == nil
236+
gws, _ := findControlledParents(ctx, r.client, route)
237+
return len(gws) > 0
235238
}
236239

237-
func findFirstParentGw(
240+
// findControlledParents returns parent gateways that are controlled by lattice gateway controller
241+
func findControlledParents(
238242
ctx context.Context,
239243
client client.Client,
240244
route core.Route,
241-
) (*gwv1.Gateway, error) {
242-
gw := &gwv1.Gateway{}
245+
) ([]*gwv1.Gateway, error) {
246+
var result []*gwv1.Gateway
243247
gwNamespace := route.Namespace()
244-
fails := []string{}
248+
misses := []string{}
245249
for _, parentRef := range route.Spec().ParentRefs() {
250+
gw := &gwv1.Gateway{}
246251
if parentRef.Namespace != nil {
247252
gwNamespace = string(*parentRef.Namespace)
248253
}
@@ -251,14 +256,18 @@ func findFirstParentGw(
251256
Name: string(parentRef.Name),
252257
}
253258
if err := client.Get(ctx, gwName, gw); err != nil {
259+
misses = append(misses, gwName.String())
254260
continue
255261
}
256-
if k8s.IsManagedGateway(ctx, client, gw) {
257-
return gw, nil
262+
if k8s.IsControlledByLatticeGatewayController(ctx, client, gw) {
263+
result = append(result, gw)
258264
}
259-
fails = append(fails, gwName.String())
260265
}
261-
return nil, fmt.Errorf("failed to get gateway, name %s", fails)
266+
var err error
267+
if len(misses) > 0 {
268+
err = fmt.Errorf("failed to get gateway, name %s", misses)
269+
}
270+
return result, err
262271
}
263272

264273
func (r *routeReconciler) buildAndDeployModel(
@@ -298,11 +307,13 @@ func (r *routeReconciler) buildAndDeployModel(
298307
return stack, err
299308
}
300309

301-
func (r *routeReconciler) findFirstParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) {
302-
gw, err := findFirstParentGw(ctx, r.client, route)
303-
if err != nil {
304-
return gwv1.ParentReference{}, err
310+
func (r *routeReconciler) findControlledParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) {
311+
gws, err := findControlledParents(ctx, r.client, route)
312+
if len(gws) <= 0 {
313+
return gwv1.ParentReference{}, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
305314
}
315+
// TODO assume one parent for now and point to service network
316+
gw := gws[0]
306317
for _, parentRef := range route.Spec().ParentRefs() {
307318
if string(parentRef.Name) == gw.Name {
308319
return parentRef, nil
@@ -332,7 +343,7 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
332343

333344
if backendRefIPFamiliesErr != nil {
334345
httpRouteOld := route.DeepCopy()
335-
parentRef, err := r.findFirstParentRef(ctx, route)
346+
parentRef, err := r.findControlledParentRef(ctx, route)
336347
if err != nil {
337348
return err
338349
}
@@ -356,7 +367,7 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
356367
if _, err := r.buildAndDeployModel(ctx, route); err != nil {
357368
if services.IsConflictError(err) {
358369
// Stop reconciliation of this route if the route cannot be owned / has conflict
359-
parentRef, parentRefErr := r.findFirstParentRef(ctx, route)
370+
parentRef, parentRefErr := r.findControlledParentRef(ctx, route)
360371
if parentRefErr != nil {
361372
// if parentRef not found, we cannot update route status
362373
return parentRefErr
@@ -502,24 +513,6 @@ func (r *routeReconciler) hasNotAcceptedCondition(route core.Route) bool {
502513
return false
503514
}
504515

505-
// find Gateway by Route and parentRef, returns nil if not found
506-
func (r *routeReconciler) findRouteParentGw(ctx context.Context, route core.Route, parentRef gwv1.ParentReference) (*gwv1.Gateway, error) {
507-
ns := route.Namespace()
508-
if parentRef.Namespace != nil && *parentRef.Namespace != "" {
509-
ns = string(*parentRef.Namespace)
510-
}
511-
gwName := types.NamespacedName{
512-
Namespace: ns,
513-
Name: string(parentRef.Name),
514-
}
515-
gw := &gwv1.Gateway{}
516-
err := r.client.Get(ctx, gwName, gw)
517-
if err != nil {
518-
return nil, client.IgnoreNotFound(err)
519-
}
520-
return gw, nil
521-
}
522-
523516
// Validation rules for route parentRefs
524517
//
525518
// Will ignore status update when:
@@ -535,15 +528,13 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
535528
}
536529

537530
parentStatuses := []gwv1.RouteParentStatus{}
531+
gws, err := findControlledParents(ctx, r.client, route)
532+
if len(gws) <= 0 {
533+
return nil, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
534+
}
535+
// TODO assume one parent for now and point to service network
536+
gw := gws[0]
538537
for _, parentRef := range route.Spec().ParentRefs() {
539-
gw, err := r.findRouteParentGw(ctx, route, parentRef)
540-
if err != nil {
541-
return nil, err
542-
}
543-
if gw == nil {
544-
continue // ignore status update if gw not found
545-
}
546-
547538
noMatchingParent := true
548539
for _, listener := range gw.Spec.Listeners {
549540
if parentRef.Port != nil && *parentRef.Port != listener.Port {

‎pkg/gateway/model_build_lattice_service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
134134
t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err)
135135
continue
136136
}
137-
if k8s.IsManagedGateway(ctx, t.client, gw) {
137+
if k8s.IsControlledByLatticeGatewayController(ctx, t.client, gw) {
138138
spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name))
139139
} else {
140140
t.log.Infof(ctx, "Ignoring route %s because gateway %s is not managed by lattice gateway controller", t.route.Name(), gw.Name)
@@ -176,7 +176,7 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
176176
func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (string, error) {
177177
// when a service is associate to multiple service network(s), all listener config MUST be same
178178
// so here we are only using the 1st gateway
179-
gw, err := t.getFirstGateway(ctx)
179+
gw, err := t.findGateway(ctx)
180180
if err != nil {
181181
if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() {
182182
return "", nil // ok if we're deleting the route

‎pkg/gateway/model_build_listener.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func isTLSPassthroughGatewayListener(listener *gwv1.Listener) bool {
5656
return listener.Protocol == gwv1.TLSProtocolType && listener.TLS != nil && listener.TLS.Mode != nil && *listener.TLS.Mode == gwv1.TLSModePassthrough
5757
}
5858

59-
func (t *latticeServiceModelBuildTask) getFirstGateway(ctx context.Context) (*gwv1.Gateway, error) {
59+
func (t *latticeServiceModelBuildTask) findGateway(ctx context.Context) (*gwv1.Gateway, error) {
6060
gw := &gwv1.Gateway{}
6161
gwNamespace := t.route.Namespace()
6262
fails := []string{}
@@ -72,7 +72,7 @@ func (t *latticeServiceModelBuildTask) getFirstGateway(ctx context.Context) (*gw
7272
t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), parentRef.Name, err)
7373
continue
7474
}
75-
if k8s.IsManagedGateway(ctx, t.client, gw) {
75+
if k8s.IsControlledByLatticeGatewayController(ctx, t.client, gw) {
7676
return gw, nil
7777
}
7878
fails = append(fails, gwName.String())
@@ -93,7 +93,7 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack
9393

9494
// when a service is associate to multiple service network(s), all listener config MUST be same
9595
// so here we are only using the 1st gateway
96-
gw, err := t.getFirstGateway(ctx)
96+
gw, err := t.findGateway(ctx)
9797
if err != nil {
9898
return err
9999
}

‎pkg/k8s/utils.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func IsGVKSupported(mgr ctrl.Manager, groupVersion string, kind string) (bool, e
5757
}
5858

5959
// validate if the gateway is managed by the lattice gateway controller
60-
func IsManagedGateway(ctx context.Context, c client.Client, gw *gwv1.Gateway) bool {
60+
func IsControlledByLatticeGatewayController(ctx context.Context, c client.Client, gw *gwv1.Gateway) bool {
6161
gwClass := &gwv1.GatewayClass{}
6262
// GatewayClass is cluster-scoped resource, so we don't need to specify namespace
6363
err := c.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass)

‎test/suites/integration/access_log_policy_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package integration
22

33
import (
44
"fmt"
5+
"strings"
6+
"time"
7+
58
anaws "github.com/aws/aws-application-networking-k8s/pkg/aws"
69
"github.com/aws/aws-application-networking-k8s/pkg/utils"
710
arn2 "github.com/aws/aws-sdk-go/aws/arn"
811
"github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
9-
"strings"
10-
"time"
1112

1213
"github.com/aws/aws-sdk-go/aws"
1314
"github.com/aws/aws-sdk-go/aws/session"
@@ -86,7 +87,7 @@ var _ = Describe("Access Log Policy", Ordered, func() {
8687
)
8788

8889
BeforeAll(func() {
89-
SetDefaultEventuallyTimeout(10 * time.Minute)
90+
SetDefaultEventuallyTimeout(5 * time.Minute)
9091
SetDefaultEventuallyPollingInterval(10 * time.Second)
9192

9293
awsResourceName = awsResourceNamePrefix + utils.RandomAlphaString(10)

0 commit comments

Comments
 (0)