Skip to content

Commit f93a37a

Browse files
zmingxizmingxi
authored
Fix service delete event (#255)
* Fix service delete event * Fix typo and wording * Remove debug logging * Remove debug logging * Fix log info format
1 parent 4b49921 commit f93a37a

File tree

5 files changed

+155
-29
lines changed

5 files changed

+155
-29
lines changed

‎controllers/eventhandlers/endpoints.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package eventhandlers
22

33
import (
44
"context"
5-
"fmt"
65
"github.com/golang/glog"
76

87
corev1 "k8s.io/api/core/v1"
@@ -27,14 +26,14 @@ func NewEnqueueRequestEndpointEvent(client client.Client) handler.EventHandler {
2726
}
2827

2928
func (h *enqueueRequestsForEndpointsEvent) Create(e event.CreateEvent, queue workqueue.RateLimitingInterface) {
30-
glog.V(6).Info("endpoint create")
29+
glog.V(6).Info("Event: endpoint create")
3130

3231
epNew := e.Object.(*corev1.Endpoints)
3332
h.enqueueImpactedService(queue, epNew)
3433
}
3534

3635
func (h *enqueueRequestsForEndpointsEvent) Update(e event.UpdateEvent, queue workqueue.RateLimitingInterface) {
37-
glog.V(6).Info("endpoints Update")
36+
glog.V(6).Info("Event: endpoints update")
3837
epOld := e.ObjectOld.(*corev1.Endpoints)
3938
epNew := e.ObjectNew.(*corev1.Endpoints)
4039
// fmt.Printf("endpoints update epOld [%v] epNew[%v]\n", epOld, epNew)
@@ -45,15 +44,16 @@ func (h *enqueueRequestsForEndpointsEvent) Update(e event.UpdateEvent, queue wor
4544
}
4645

4746
func (h *enqueueRequestsForEndpointsEvent) Delete(e event.DeleteEvent, queue workqueue.RateLimitingInterface) {
48-
fmt.Printf("TODO endpoints Delete \n")
47+
glog.V(6).Infof("Event: endpoints delete")
48+
// service event handler handles this event here
4949
}
5050

5151
func (h *enqueueRequestsForEndpointsEvent) Generic(e event.GenericEvent, queue workqueue.RateLimitingInterface) {
5252

5353
}
5454

5555
func (h *enqueueRequestsForEndpointsEvent) enqueueImpactedService(queue workqueue.RateLimitingInterface, ep *corev1.Endpoints) {
56-
glog.V(6).Infof("enqueueImpactedService [%v]", ep)
56+
glog.V(6).Infof("Event: enqueueImpactedService [%v]", ep)
5757

5858
var targetIPList []string
5959

@@ -76,7 +76,7 @@ func (h *enqueueRequestsForEndpointsEvent) enqueueImpactedService(queue workqueu
7676
}
7777

7878
if err := h.client.Get(context.TODO(), namespaceName, svc); err != nil {
79-
glog.V(2).Infof("enqueueImpactedService, service not found %v\n", err)
79+
glog.V(6).Infof("Event: enqueueImpactedService, service not found %v\n", err)
8080
return
8181
}
8282

‎controllers/eventhandlers/service.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,48 @@ func NewEqueueRequestServiceEvent(client client.Client) handler.EventHandler {
2828
}
2929

3030
func (h *enqueueRequetsForServiceEvent) Create(e event.CreateEvent, queue workqueue.RateLimitingInterface) {
31+
glog.V(6).Info("Event: service create")
3132
service := e.Object.(*corev1.Service)
33+
h.enqueueImpactedService(queue, service)
3234
h.enqueueImpactedServiceExport(queue, service)
3335
}
3436

3537
func (h *enqueueRequetsForServiceEvent) Update(e event.UpdateEvent, queue workqueue.RateLimitingInterface) {
3638
}
3739

3840
func (h *enqueueRequetsForServiceEvent) Delete(e event.DeleteEvent, queue workqueue.RateLimitingInterface) {
41+
glog.V(6).Info("Event: service delete")
3942
service := e.Object.(*corev1.Service)
43+
h.enqueueImpactedService(queue, service)
4044
h.enqueueImpactedServiceExport(queue, service)
4145
}
4246

4347
func (h *enqueueRequetsForServiceEvent) Generic(e event.GenericEvent, queue workqueue.RateLimitingInterface) {
4448

4549
}
4650

51+
func (h *enqueueRequetsForServiceEvent) enqueueImpactedService(queue workqueue.RateLimitingInterface, ep *corev1.Service) {
52+
glog.V(6).Infof("Event: enqueueImpactedService: %v\n", ep)
53+
54+
srv := &corev1.Service{}
55+
namespacedName := types.NamespacedName{
56+
Namespace: ep.Namespace,
57+
Name: ep.Name,
58+
}
59+
60+
if err := h.client.Get(context.TODO(), namespacedName, srv); err != nil {
61+
glog.V(6).Infof("Event: enqueueImpactedService, service not found %v\n", err)
62+
return
63+
}
64+
65+
queue.Add(reconcile.Request{
66+
NamespacedName: namespacedName,
67+
})
68+
69+
}
70+
4771
func (h *enqueueRequetsForServiceEvent) enqueueImpactedServiceExport(queue workqueue.RateLimitingInterface, ep *corev1.Service) {
48-
glog.V(6).Infof("enqueueImpactedServiceExport: %v\n", ep)
72+
glog.V(6).Infof("Event: enqueueImpactedServiceExport: %v\n", ep)
4973

5074
srvExport := &mcs_api.ServiceExport{}
5175
namespacedName := types.NamespacedName{
@@ -54,7 +78,7 @@ func (h *enqueueRequetsForServiceEvent) enqueueImpactedServiceExport(queue workq
5478
}
5579

5680
if err := h.client.Get(context.TODO(), namespacedName, srvExport); err != nil {
57-
glog.V(6).Infof("enqueueImpactedServiceExport, serviceexport not found %v\n", err)
81+
glog.V(6).Infof("Event: enqueueImpactedServiceExport, serviceexport not found %v\n", err)
5882
return
5983
}
6084

@@ -91,7 +115,7 @@ func (h *enqueueHTTPRequetsForServiceEvent) Generic(e event.GenericEvent, queue
91115
}
92116

93117
func (h *enqueueHTTPRequetsForServiceEvent) enqueueImpactedHTTPRoute(queue workqueue.RateLimitingInterface, ep *corev1.Service) {
94-
glog.V(6).Infof("enqueueImpactedHTTPRoute: %v\n", ep)
118+
glog.V(6).Infof("Event: enqueueImpactedHTTPRoute: %v\n", ep)
95119

96120
httpRouteList := &gateway_api.HTTPRouteList{}
97121

@@ -101,7 +125,7 @@ func (h *enqueueHTTPRequetsForServiceEvent) enqueueImpactedHTTPRoute(queue workq
101125
if !isServiceUsedByHTTPRoute(httpRoute, ep) {
102126
continue
103127
}
104-
glog.V(6).Infof("enqueueImpactedHTTPRoute --> httproute %v \n", httpRoute)
128+
glog.V(6).Infof("Event: enqueueImpactedHTTPRoute --> httproute %v \n", httpRoute)
105129
namespacedName := types.NamespacedName{
106130
Namespace: httpRoute.Namespace,
107131
Name: httpRoute.Name,

‎controllers/service_controller.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
)
4343

4444
const (
45+
// Typo
4546
serviceFinalizer = "service.ki8s.aws/resources"
4647
)
4748

@@ -101,23 +102,29 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
101102
svcLog.Info("ServiceReconciler")
102103

103104
svc := &corev1.Service{}
105+
ds := r.latticeDataStore
104106

105107
if err := r.Client.Get(ctx, req.NamespacedName, svc); err != nil {
106108
return ctrl.Result{}, client.IgnoreNotFound(err)
107109
}
108110

109111
if !svc.DeletionTimestamp.IsZero() {
112+
tgNameD := latticestore.TargetGroupName(svc.Name, svc.Namespace)
113+
TGDeleted := ds.GetTargetGroupsByTG(tgNameD)
114+
for _, tg := range TGDeleted {
115+
glog.V(6).Infof("service deletion trigger target IP list registration %v and tg %v\n",
116+
tgNameD, tg)
117+
r.reconcileTargetsResource(ctx, svc, tg.TargetGroupKey.RouteName)
118+
119+
}
110120
r.finalizerManager.RemoveFinalizers(ctx, svc, serviceFinalizer)
111-
return ctrl.Result{}, nil
112121

122+
return ctrl.Result{}, nil
113123
}
114124

115-
ds := r.latticeDataStore
116-
117125
// TODO also need to check serviceexport object to trigger building TargetGroup
118126
tgName := latticestore.TargetGroupName(svc.Name, svc.Namespace)
119127
TGs := ds.GetTargetGroupsByTG(tgName) // isServiceImport = false
120-
121128
for _, tg := range TGs {
122129

123130
glog.V(6).Infof("endpoints change trigger target IP list registration %v and tg %v\n",
@@ -131,7 +138,6 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
131138
}
132139

133140
func (r *ServiceReconciler) reconcileTargetsResource(ctx context.Context, svc *corev1.Service, routename string) {
134-
135141
if err := r.finalizerManager.AddFinalizers(ctx, svc, serviceFinalizer); err != nil {
136142
r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedAddFinalizer, fmt.Sprintf("Failed and finalizer due %v", err))
137143
}

‎pkg/gateway/model_build_targets.go‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -97,34 +97,40 @@ func (t *latticeTargetsModelBuildTask) buildLatticeTargets(ctx context.Context)
9797

9898
endPoints := &corev1.Endpoints{}
9999

100+
svc := &corev1.Service{}
100101
namespacedName := types.NamespacedName{
101102
Namespace: t.tgNamespace,
102103
Name: t.tgName,
103104
}
104105

105-
if err := t.Client.Get(ctx, namespacedName, endPoints); err != nil {
106+
if err := t.Client.Get(ctx, namespacedName, svc); err != nil {
106107
errmsg := fmt.Sprintf("Build Targets failed because K8S service %v does not exist", namespacedName)
107-
glog.V(6).Infof("errmsg: %v\n", errmsg)
108108
return errors.New(errmsg)
109109
}
110-
111-
glog.V(6).Infof("Build Targets: endPoints %v \n", endPoints)
112110
var targetList []latticemodel.Target
113111

114-
for _, endPoint := range endPoints.Subsets {
112+
if svc.DeletionTimestamp.IsZero() {
113+
if err := t.Client.Get(ctx, namespacedName, endPoints); err != nil {
114+
errmsg := fmt.Sprintf("Build Targets failed because K8S service %v does not exist", namespacedName)
115+
glog.V(6).Infof("errmsg: %v\n", errmsg)
116+
return errors.New(errmsg)
117+
}
118+
119+
glog.V(6).Infof("Build Targets: endPoints %v \n", endPoints)
115120

116-
for _, address := range endPoint.Addresses {
117-
for _, port := range endPoint.Ports {
118-
glog.V(6).Infof("serviceReconcile-endpoints: address %v, port %v\n", address, port)
119-
target := latticemodel.Target{
120-
TargetIP: address.IP,
121-
Port: int64(port.Port),
121+
for _, endPoint := range endPoints.Subsets {
122+
123+
for _, address := range endPoint.Addresses {
124+
for _, port := range endPoint.Ports {
125+
glog.V(6).Infof("serviceReconcile-endpoints: address %v, port %v\n", address, port)
126+
target := latticemodel.Target{
127+
TargetIP: address.IP,
128+
Port: int64(port.Port),
129+
}
130+
targetList = append(targetList, target)
122131
}
123-
targetList = append(targetList, target)
124132
}
125-
126133
}
127-
128134
}
129135

130136
glog.V(6).Infof("Build Targets--- targetIPList [%v]\n", targetList)

‎pkg/gateway/model_build_targets_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"testing"
7+
"time"
78

89
"github.com/golang/mock/gomock"
910
"github.com/stretchr/testify/assert"
@@ -26,6 +27,7 @@ func Test_Targets(t *testing.T) {
2627
srvExportName string
2728
srvExportNamespace string
2829
endPoints []corev1.Endpoints
30+
svc corev1.Service
2931
inDataStore bool
3032
refByServiceExport bool
3133
refByService bool
@@ -50,6 +52,13 @@ func Test_Targets(t *testing.T) {
5052
},
5153
},
5254
},
55+
svc: corev1.Service{
56+
ObjectMeta: metav1.ObjectMeta{
57+
Namespace: "ns1",
58+
Name: "export1",
59+
DeletionTimestamp: nil,
60+
},
61+
},
5362
inDataStore: true,
5463
refByServiceExport: true,
5564
wantErrIsNil: true,
@@ -72,6 +81,57 @@ func Test_Targets(t *testing.T) {
7281
},
7382
},
7483
},
84+
{
85+
name: "Delete svc and all endpoints to build spec",
86+
srvExportName: "export1",
87+
srvExportNamespace: "ns1",
88+
endPoints: []corev1.Endpoints{
89+
{
90+
ObjectMeta: metav1.ObjectMeta{
91+
Namespace: "ns1",
92+
Name: "export1",
93+
},
94+
Subsets: []corev1.EndpointSubset{
95+
{
96+
Addresses: []corev1.EndpointAddress{{IP: "10.10.1.1"}, {IP: "10.10.2.2"}},
97+
Ports: []corev1.EndpointPort{{Name: "a", Port: 8675}, {Name: "b", Port: 309}},
98+
},
99+
},
100+
},
101+
},
102+
svc: corev1.Service{
103+
ObjectMeta: metav1.ObjectMeta{
104+
Namespace: "ns1",
105+
Name: "export1",
106+
DeletionTimestamp: &metav1.Time{
107+
Time: time.Now(),
108+
},
109+
},
110+
},
111+
inDataStore: true,
112+
refByServiceExport: true,
113+
wantErrIsNil: true,
114+
expectedTargetList: nil,
115+
},
116+
{
117+
name: "Delete svc and no endpoints to build spec",
118+
srvExportName: "export1",
119+
srvExportNamespace: "ns1",
120+
endPoints: []corev1.Endpoints{},
121+
svc: corev1.Service{
122+
ObjectMeta: metav1.ObjectMeta{
123+
Namespace: "ns1",
124+
Name: "export1",
125+
DeletionTimestamp: &metav1.Time{
126+
Time: time.Now(),
127+
},
128+
},
129+
},
130+
inDataStore: true,
131+
refByServiceExport: true,
132+
wantErrIsNil: true,
133+
expectedTargetList: nil,
134+
},
75135
{
76136
name: "Endpoints without TargetGroup",
77137
srvExportName: "export2",
@@ -90,6 +150,13 @@ func Test_Targets(t *testing.T) {
90150
},
91151
},
92152
},
153+
svc: corev1.Service{
154+
ObjectMeta: metav1.ObjectMeta{
155+
Namespace: "ns1",
156+
Name: "export1",
157+
DeletionTimestamp: nil,
158+
},
159+
},
93160
inDataStore: false,
94161
refByServiceExport: true,
95162
wantErrIsNil: false,
@@ -112,6 +179,13 @@ func Test_Targets(t *testing.T) {
112179
},
113180
},
114181
},
182+
svc: corev1.Service{
183+
ObjectMeta: metav1.ObjectMeta{
184+
Namespace: "ns1",
185+
Name: "export1",
186+
DeletionTimestamp: nil,
187+
},
188+
},
115189
inDataStore: true,
116190
refByServiceExport: false,
117191
refByService: false,
@@ -124,6 +198,13 @@ func Test_Targets(t *testing.T) {
124198
inDataStore: false,
125199
refByServiceExport: true,
126200
wantErrIsNil: false,
201+
svc: corev1.Service{
202+
ObjectMeta: metav1.ObjectMeta{
203+
Namespace: "ns1",
204+
Name: "export1",
205+
DeletionTimestamp: nil,
206+
},
207+
},
127208
},
128209
{
129210
name: "Add all endpoints to build spec",
@@ -143,6 +224,13 @@ func Test_Targets(t *testing.T) {
143224
},
144225
},
145226
},
227+
svc: corev1.Service{
228+
ObjectMeta: metav1.ObjectMeta{
229+
Namespace: "ns1",
230+
Name: "export5",
231+
DeletionTimestamp: nil,
232+
},
233+
},
146234
inDataStore: true,
147235
refByService: true,
148236
wantErrIsNil: true,
@@ -180,6 +268,8 @@ func Test_Targets(t *testing.T) {
180268
assert.NoError(t, k8sClient.Create(ctx, tt.endPoints[0].DeepCopy()))
181269
}
182270

271+
assert.NoError(t, k8sClient.Create(ctx, tt.svc.DeepCopy()))
272+
183273
ds := latticestore.NewLatticeDataStore()
184274

185275
if tt.inDataStore {

0 commit comments

Comments
 (0)