Skip to content

Commit c84685e

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4192 from zac-nixon/znixon/gw-delete
[feat: gw api] support gw deletion
2 parents 8ed0f0e + 82548f7 commit c84685e

File tree

4 files changed

+179
-32
lines changed

4 files changed

+179
-32
lines changed

‎controllers/gateway/gateway_controller.go‎

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,6 @@ type gatewayReconciler struct {
145145
func (r *gatewayReconciler) Reconcile(ctx context.Context, req reconcile.Request) (ctrl.Result, error) {
146146
r.reconcileTracker(req.NamespacedName)
147147
err := r.reconcileHelper(ctx, req)
148-
if err != nil {
149-
r.logger.Error(err, "Got this error!")
150-
}
151148
return runtime.HandleReconcileError(err, r.logger)
152149
}
153150

@@ -196,7 +193,7 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
196193
}
197194

198195
if lb == nil {
199-
err = r.reconcileDelete(ctx, gw, allRoutes)
196+
err = r.reconcileDelete(ctx, gw, stack, allRoutes)
200197
if err != nil {
201198
r.logger.Error(err, "Failed to process gateway delete")
202199
}
@@ -206,39 +203,36 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
206203
return r.reconcileUpdate(ctx, gw, stack, lb, backendSGRequired)
207204
}
208205

209-
func (r *gatewayReconciler) resolveLoadBalancerConfig(ctx context.Context, k8sClient client.Client, reference *gwv1.ParametersReference) (*elbv2gw.LoadBalancerConfiguration, error) {
210-
var lbConf *elbv2gw.LoadBalancerConfiguration
211-
var err error
212-
if reference != nil {
213-
lbConf = &elbv2gw.LoadBalancerConfiguration{}
214-
if reference.Namespace != nil {
215-
err = k8sClient.Get(ctx, types.NamespacedName{
216-
Namespace: string(*reference.Namespace),
217-
Name: reference.Name,
218-
}, lbConf)
219-
} else {
220-
err = errors.New("Namespace must be specified in ParametersRef")
221-
}
222-
}
223-
return lbConf, err
224-
}
225-
226-
func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, routes map[int32][]routeutils.RouteDescriptor) error {
206+
func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error {
227207
for _, routeList := range routes {
228208
if len(routeList) != 0 {
229-
// TODO - Better error messaging (e.g. tell user the routes that are still attached)
230-
return errors.New("Gateway still has routes attached")
209+
err := errors.Errorf("Gateway deletion invoked with routes attached [%s]", generateRouteList(routes))
210+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedDeleteWithRoutesAttached, err.Error())
211+
return err
231212
}
232213
}
233214

234-
return r.finalizerManager.RemoveFinalizers(ctx, gw, r.finalizer)
215+
if k8s.HasFinalizer(gw, r.finalizer) {
216+
err := r.deployModel(ctx, gw, stack)
217+
if err != nil {
218+
return err
219+
}
220+
if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeGateway, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil {
221+
return err
222+
}
223+
if err := r.finalizerManager.RemoveFinalizers(ctx, gw, r.finalizer); err != nil {
224+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed remove finalizer due to %v", err))
225+
return err
226+
}
227+
}
228+
return nil
235229
}
236230

237231
func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gateway, stack core.Stack,
238232
lb *elbv2model.LoadBalancer, backendSGRequired bool) error {
239233

240234
if err := r.finalizerManager.AddFinalizers(ctx, gw, r.finalizer); err != nil {
241-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
235+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
242236
return err
243237
}
244238
err := r.deployModel(ctx, gw, stack)
@@ -251,16 +245,16 @@ func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gatewa
251245
}
252246

253247
if !backendSGRequired {
254-
if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeService, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil {
248+
if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeGateway, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil {
255249
return err
256250
}
257251
}
258252

259253
if err = r.updateGatewayStatus(ctx, lbDNS, gw); err != nil {
260-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedUpdateStatus, fmt.Sprintf("Failed update status due to %v", err))
254+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedUpdateStatus, fmt.Sprintf("Failed update status due to %v", err))
261255
return err
262256
}
263-
r.eventRecorder.Event(gw, corev1.EventTypeNormal, k8s.ServiceEventReasonSuccessfullyReconciled, "Successfully reconciled")
257+
r.eventRecorder.Event(gw, corev1.EventTypeNormal, k8s.GatewayEventReasonSuccessfullyReconciled, "Successfully reconciled")
264258
return nil
265259
}
266260

@@ -270,7 +264,7 @@ func (r *gatewayReconciler) deployModel(ctx context.Context, gw *gwv1.Gateway, s
270264
if errors.As(err, &requeueNeededAfter) {
271265
return err
272266
}
273-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedDeployModel, fmt.Sprintf("Failed deploy model due to %v", err))
267+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedDeployModel, fmt.Sprintf("Failed deploy model due to %v", err))
274268
return err
275269
}
276270
r.logger.Info("successfully deployed model", "gateway", k8s.NamespacedName(gw))
@@ -280,12 +274,12 @@ func (r *gatewayReconciler) deployModel(ctx context.Context, gw *gwv1.Gateway, s
280274
func (r *gatewayReconciler) buildModel(ctx context.Context, gw *gwv1.Gateway, cfg elbv2gw.LoadBalancerConfiguration, listenerToRoute map[int32][]routeutils.RouteDescriptor) (core.Stack, *elbv2model.LoadBalancer, bool, error) {
281275
stack, lb, backendSGRequired, err := r.modelBuilder.Build(ctx, gw, cfg, listenerToRoute)
282276
if err != nil {
283-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
277+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
284278
return nil, nil, false, err
285279
}
286280
stackJSON, err := r.stackMarshaller.Marshal(stack)
287281
if err != nil {
288-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
282+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
289283
return nil, nil, false, err
290284
}
291285
r.logger.Info("successfully built model", "model", stackJSON)

‎controllers/gateway/utils.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@ package gateway
22

33
import (
44
"context"
5+
"fmt"
56
"github.com/pkg/errors"
67
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
78
"k8s.io/apimachinery/pkg/types"
89
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
10+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
911
"sigs.k8s.io/controller-runtime/pkg/client"
1012
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
13+
"sort"
1114
"strconv"
15+
"strings"
1216
"time"
1317
)
1418

@@ -94,3 +98,21 @@ func resolveLoadBalancerConfig(ctx context.Context, k8sClient client.Client, ref
9498

9599
return lbConf, err
96100
}
101+
102+
// generateRouteList generate a deterministic route list.
103+
//
104+
// Due to the nature of golang maps, we need to sort the keys and for good measure we sort the route descriptors too
105+
func generateRouteList(listenerRoutes map[int32][]routeutils.RouteDescriptor) string {
106+
107+
allRoutes := make([]string, 0)
108+
109+
for _, lr := range listenerRoutes {
110+
for _, r := range lr {
111+
allRoutes = append(allRoutes, fmt.Sprintf("(%s, %s:%s)", r.GetRouteKind(), r.GetRouteNamespacedName().Namespace, r.GetRouteNamespacedName().Name))
112+
}
113+
}
114+
115+
sort.Strings(allRoutes)
116+
117+
return strings.Join(allRoutes, ",")
118+
}

‎controllers/gateway/utils_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package gateway
2+
3+
import (
4+
"github.com/stretchr/testify/assert"
5+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
6+
"testing"
7+
)
8+
9+
func Test_generateRouteList(t *testing.T) {
10+
testCases := []struct {
11+
name string
12+
routes map[int32][]routeutils.RouteDescriptor
13+
expected string
14+
}{
15+
{
16+
name: "no routes",
17+
routes: make(map[int32][]routeutils.RouteDescriptor),
18+
expected: "",
19+
},
20+
{
21+
name: "some routes",
22+
routes: map[int32][]routeutils.RouteDescriptor{
23+
1: {
24+
&routeutils.MockRoute{
25+
Name: "1-1-r",
26+
Namespace: "1-1-ns",
27+
Kind: routeutils.GRPCRouteKind,
28+
},
29+
&routeutils.MockRoute{
30+
Name: "1-2-r",
31+
Namespace: "1-2-ns",
32+
Kind: routeutils.TCPRouteKind,
33+
},
34+
&routeutils.MockRoute{
35+
Name: "1-3-r",
36+
Namespace: "1-3-ns",
37+
Kind: routeutils.HTTPRouteKind,
38+
},
39+
&routeutils.MockRoute{
40+
Name: "1-4-r",
41+
Namespace: "1-4-ns",
42+
Kind: routeutils.UDPRouteKind,
43+
},
44+
},
45+
2: {
46+
&routeutils.MockRoute{
47+
Name: "2-1-r",
48+
Namespace: "2-1-ns",
49+
Kind: routeutils.GRPCRouteKind,
50+
},
51+
&routeutils.MockRoute{
52+
Name: "2-2-r",
53+
Namespace: "2-2-ns",
54+
Kind: routeutils.TCPRouteKind,
55+
},
56+
&routeutils.MockRoute{
57+
Name: "2-3-r",
58+
Namespace: "2-3-ns",
59+
Kind: routeutils.HTTPRouteKind,
60+
},
61+
&routeutils.MockRoute{
62+
Name: "2-4-r",
63+
Namespace: "2-4-ns",
64+
Kind: routeutils.UDPRouteKind,
65+
},
66+
},
67+
3: {
68+
&routeutils.MockRoute{
69+
Name: "3-1-r",
70+
Namespace: "3-1-ns",
71+
Kind: routeutils.GRPCRouteKind,
72+
},
73+
&routeutils.MockRoute{
74+
Name: "3-2-r",
75+
Namespace: "3-2-ns",
76+
Kind: routeutils.TCPRouteKind,
77+
},
78+
&routeutils.MockRoute{
79+
Name: "3-3-r",
80+
Namespace: "3-3-ns",
81+
Kind: routeutils.HTTPRouteKind,
82+
},
83+
&routeutils.MockRoute{
84+
Name: "3-4-r",
85+
Namespace: "3-4-ns",
86+
Kind: routeutils.UDPRouteKind,
87+
},
88+
},
89+
4: {
90+
&routeutils.MockRoute{
91+
Name: "4-1-r",
92+
Namespace: "4-1-ns",
93+
Kind: routeutils.GRPCRouteKind,
94+
},
95+
&routeutils.MockRoute{
96+
Name: "4-2-r",
97+
Namespace: "4-2-ns",
98+
Kind: routeutils.TCPRouteKind,
99+
},
100+
&routeutils.MockRoute{
101+
Name: "4-3-r",
102+
Namespace: "4-3-ns",
103+
Kind: routeutils.HTTPRouteKind,
104+
},
105+
&routeutils.MockRoute{
106+
Name: "4-4-r",
107+
Namespace: "4-4-ns",
108+
Kind: routeutils.UDPRouteKind,
109+
},
110+
},
111+
},
112+
expected: "(GRPCRoute, 1-1-ns:1-1-r),(GRPCRoute, 2-1-ns:2-1-r),(GRPCRoute, 3-1-ns:3-1-r),(GRPCRoute, 4-1-ns:4-1-r),(HTTPRoute, 1-3-ns:1-3-r),(HTTPRoute, 2-3-ns:2-3-r),(HTTPRoute, 3-3-ns:3-3-r),(HTTPRoute, 4-3-ns:4-3-r),(TCPRoute, 1-2-ns:1-2-r),(TCPRoute, 2-2-ns:2-2-r),(TCPRoute, 3-2-ns:3-2-r),(TCPRoute, 4-2-ns:4-2-r),(UDPRoute, 1-4-ns:1-4-r),(UDPRoute, 2-4-ns:2-4-r),(UDPRoute, 3-4-ns:3-4-r),(UDPRoute, 4-4-ns:4-4-r)",
113+
},
114+
}
115+
116+
for _, tc := range testCases {
117+
t.Run(tc.name, func(t *testing.T) {
118+
res := generateRouteList(tc.routes)
119+
assert.Equal(t, tc.expected, res)
120+
})
121+
}
122+
}

‎pkg/k8s/events.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,13 @@ const (
2828
TargetGroupBindingEventReasonFailedNetworkReconcile = "FailedNetworkReconcile"
2929
TargetGroupBindingEventReasonBackendNotFound = "BackendNotFound"
3030
TargetGroupBindingEventReasonSuccessfullyReconciled = "SuccessfullyReconciled"
31+
32+
// Gateway events
33+
GatewayEventReasonFailedAddFinalizer = "FailedAddFinalizer"
34+
GatewayEventReasonFailedRemoveFinalizer = "FailedRemoveFinalizer"
35+
GatewayEventReasonFailedDeleteWithRoutesAttached = "FailedDeleteRoutesAttached"
36+
GatewayEventReasonFailedUpdateStatus = "FailedUpdateStatus"
37+
GatewayEventReasonSuccessfullyReconciled = "SuccessfullyReconciled"
38+
GatewayEventReasonFailedDeployModel = "FailedDeployModel"
39+
GatewayEventReasonFailedBuildModel = "FailedBuildModel"
3140
)

0 commit comments

Comments
 (0)