Skip to content

Commit 1f0cc41

Browse files
committed
Prune egress endpoint slices by desired state
Every reconcile stamped the applied endpoint slices with a time.Now-based last-updated label used to select stale slices for deletion. Since the slices are watched by this controller, each reconcile mutated its own watch sources, and each mutation enqueued another reconcile: creating a single NetworkEgress produced hundreds of ConfigMap and EndpointSlice rewrites before the loop happened to settle, and the churn repeatedly toggled forwarder rules. Track the desired slice names instead and delete unwanted ones by comparing against them. A no-change reconcile now applies no-ops and generates no events, and the label plus its selector machinery goes away entirely.
1 parent 41d9956 commit 1f0cc41

2 files changed

Lines changed: 190 additions & 26 deletions

File tree

internal/controller/forwarderservice_controller.go

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,11 @@ import (
99
"maps"
1010
"net/netip"
1111
"strings"
12-
"time"
1312

1413
corev1 "k8s.io/api/core/v1"
1514
discoveryv1 "k8s.io/api/discovery/v1"
1615
kerrors "k8s.io/apimachinery/pkg/api/errors"
1716
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18-
"k8s.io/apimachinery/pkg/labels"
19-
"k8s.io/apimachinery/pkg/selection"
2017
"k8s.io/apimachinery/pkg/types"
2118
corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
2219
discoveryv1ac "k8s.io/client-go/applyconfigurations/discovery/v1"
@@ -37,7 +34,6 @@ const (
3734
ForwarderRouterNameLabel = "netbird.io/forwarder-router-name"
3835
EgressRouterNameLabel = "netbird.io/egress-router-name"
3936
EgressRouterNamespaceLabel = "netbird.io/egress-router-namespace"
40-
LastUpdatedLabel = "netbird.io/last-updated"
4137
)
4238

4339
// ForwarderServiceReconciler reconciles a EndpointSlice object
@@ -96,7 +92,7 @@ func (r *ForwarderServiceReconciler) Reconcile(ctx context.Context, req ctrl.Req
9692
}
9793

9894
// Copy router endpoint slices to egress services.
99-
lastUpdated := fmt.Sprintf("%d", time.Now().Unix())
95+
desired := map[string]struct{}{}
10096

10197
for _, egressSvc := range egressSvcList.Items {
10298
netEgress := &nbv1alpha1.NetworkEgress{
@@ -143,12 +139,13 @@ func (r *ForwarderServiceReconciler) Reconcile(ctx context.Context, req ctrl.Req
143139
labels := map[string]string{
144140
discoveryv1.LabelServiceName: egressSvc.Name,
145141
discoveryv1.LabelManagedBy: "netbird-operator.netbird.io",
146-
LastUpdatedLabel: lastUpdated,
147142
}
148143
maps.Copy(labels, egressSvc.Labels)
149144

145+
name := fmt.Sprintf("%s-%s", egressSvc.Name, nameSuffx)
146+
desired[name] = struct{}{}
150147
endpointACs := toEndpointApplyConfigurations(endpointSlice.Endpoints)
151-
endpointSliceAC := discoveryv1ac.EndpointSlice(fmt.Sprintf("%s-%s", egressSvc.Name, nameSuffx), egressSvc.Namespace).
148+
endpointSliceAC := discoveryv1ac.EndpointSlice(name, egressSvc.Namespace).
152149
WithLabels(labels).
153150
WithOwnerReferences(egressSvcOwnerRef).
154151
WithAddressType(endpointSlice.AddressType).
@@ -175,29 +172,14 @@ func (r *ForwarderServiceReconciler) Reconcile(ctx context.Context, req ctrl.Req
175172
}
176173

177174
// Cleanup old endpoint slices.
178-
updateReq, err := labels.NewRequirement(LastUpdatedLabel, selection.NotEquals, []string{lastUpdated})
179-
if err != nil {
180-
return ctrl.Result{}, err
181-
}
182-
egressNameReq, err := labels.NewRequirement(EgressRouterNameLabel, selection.Equals, []string{routerName})
183-
if err != nil {
184-
return ctrl.Result{}, err
185-
}
186-
egressNamespaceReq, err := labels.NewRequirement(EgressRouterNamespaceLabel, selection.Equals, []string{req.Namespace})
187-
if err != nil {
188-
return ctrl.Result{}, err
189-
}
190-
sourceReq, err := labels.NewRequirement(discoveryv1.LabelServiceName, selection.NotEquals, []string{svc.Name})
191-
if err != nil {
192-
return ctrl.Result{}, err
193-
}
194-
deleteSelector := labels.NewSelector().Add(*updateReq).Add(*egressNameReq).Add(*egressNamespaceReq).Add(*sourceReq)
195-
196-
err = r.Client.List(ctx, endpointSliceList, client.MatchingLabelsSelector{Selector: deleteSelector})
175+
err = r.Client.List(ctx, endpointSliceList, &client.MatchingLabels{EgressRouterNameLabel: routerName, EgressRouterNamespaceLabel: req.Namespace})
197176
if err != nil {
198177
return ctrl.Result{}, err
199178
}
200179
for _, item := range endpointSliceList.Items {
180+
if _, ok := desired[item.Name]; ok {
181+
continue
182+
}
201183
if err := r.Client.Delete(ctx, &item); err != nil && !kerrors.IsNotFound(err) {
202184
return ctrl.Result{}, err
203185
}
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
3+
package controller
4+
5+
import (
6+
"context"
7+
"fmt"
8+
"time"
9+
10+
. "github.com/onsi/ginkgo/v2"
11+
. "github.com/onsi/gomega"
12+
corev1 "k8s.io/api/core/v1"
13+
discoveryv1 "k8s.io/api/discovery/v1"
14+
kerrors "k8s.io/apimachinery/pkg/api/errors"
15+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
"sigs.k8s.io/controller-runtime/pkg/client"
17+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
18+
19+
nbv1alpha1 "github.com/netbirdio/kubernetes-operator/api/v1alpha1"
20+
)
21+
22+
var _ = Describe("ForwarderService Controller", func() {
23+
Context("When reconciling", func() {
24+
ctx := context.Background()
25+
26+
const routerName = "fwd-router"
27+
var namespace string
28+
var fwdSvcNN client.ObjectKey
29+
30+
var netEgressRec *NetworkEgressReconciler
31+
var forwarderRec *ForwarderServiceReconciler
32+
33+
BeforeEach(func() {
34+
namespace = fmt.Sprintf("forwarder-service-%d", time.Now().UnixNano())
35+
fwdSvcNN = client.ObjectKey{
36+
Name: fmt.Sprintf("networkrouter-%s-forwarder", routerName),
37+
Namespace: namespace,
38+
}
39+
netEgressRec = &NetworkEgressReconciler{
40+
Client: k8sClient,
41+
}
42+
forwarderRec = &ForwarderServiceReconciler{
43+
Client: k8sClient,
44+
}
45+
46+
ns := &corev1.Namespace{
47+
ObjectMeta: metav1.ObjectMeta{
48+
Name: namespace,
49+
},
50+
}
51+
Expect(k8sClient.Create(ctx, ns)).To(Succeed())
52+
53+
netRouter := &nbv1alpha1.NetworkRouter{
54+
ObjectMeta: metav1.ObjectMeta{
55+
Name: routerName,
56+
Namespace: namespace,
57+
},
58+
Spec: nbv1alpha1.NetworkRouterSpec{
59+
DNSZoneRef: nbv1alpha1.DNSZoneReference{
60+
Name: "foo.bar",
61+
},
62+
},
63+
}
64+
Expect(k8sClient.Create(ctx, netRouter)).To(Succeed())
65+
66+
fwdSvc := &corev1.Service{
67+
ObjectMeta: metav1.ObjectMeta{
68+
Name: fwdSvcNN.Name,
69+
Namespace: namespace,
70+
Labels: map[string]string{
71+
ForwarderRouterNameLabel: routerName,
72+
},
73+
},
74+
Spec: corev1.ServiceSpec{
75+
Ports: []corev1.ServicePort{
76+
{Name: "http", Port: 80},
77+
},
78+
},
79+
}
80+
Expect(k8sClient.Create(ctx, fwdSvc)).To(Succeed())
81+
82+
fwdSlice := &discoveryv1.EndpointSlice{
83+
ObjectMeta: metav1.ObjectMeta{
84+
Name: fwdSvcNN.Name + "-abc12",
85+
Namespace: namespace,
86+
Labels: map[string]string{
87+
discoveryv1.LabelServiceName: fwdSvcNN.Name,
88+
},
89+
},
90+
AddressType: discoveryv1.AddressTypeIPv4,
91+
Endpoints: []discoveryv1.Endpoint{
92+
{Addresses: []string{"10.0.0.1"}},
93+
},
94+
}
95+
Expect(k8sClient.Create(ctx, fwdSlice)).To(Succeed())
96+
97+
netEgress := &nbv1alpha1.NetworkEgress{
98+
ObjectMeta: metav1.ObjectMeta{
99+
Name: "egress",
100+
Namespace: namespace,
101+
},
102+
Spec: nbv1alpha1.NetworkEgressSpec{
103+
NetworkRouterRef: nbv1alpha1.CrossNamespaceReference{
104+
Name: routerName,
105+
Namespace: namespace,
106+
},
107+
Target: nbv1alpha1.NetworkEgressTarget{
108+
FQDN: &nbv1alpha1.NetworkEgressFQDNTarget{
109+
Hostname: "example.com",
110+
},
111+
},
112+
Ports: []nbv1alpha1.NetworkEgressPort{
113+
{Name: "http", Port: 80},
114+
},
115+
},
116+
}
117+
Expect(k8sClient.Create(ctx, netEgress)).To(Succeed())
118+
_, err := netEgressRec.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(netEgress)})
119+
Expect(err).NotTo(HaveOccurred())
120+
})
121+
122+
AfterEach(func() {
123+
ns := &corev1.Namespace{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Name: namespace,
126+
},
127+
}
128+
Expect(k8sClient.Delete(ctx, ns)).To(Succeed())
129+
})
130+
131+
It("does not modify derived objects when nothing changed", func() {
132+
_, err := forwarderRec.Reconcile(ctx, reconcile.Request{NamespacedName: fwdSvcNN})
133+
Expect(err).NotTo(HaveOccurred())
134+
135+
cm := &corev1.ConfigMap{}
136+
Expect(k8sClient.Get(ctx, fwdSvcNN, cm)).To(Succeed())
137+
sliceList := &discoveryv1.EndpointSliceList{}
138+
Expect(k8sClient.List(ctx, sliceList, client.InNamespace(namespace), client.MatchingLabels{EgressRouterNameLabel: routerName})).To(Succeed())
139+
Expect(sliceList.Items).NotTo(BeEmpty())
140+
versions := map[string]string{cm.Name: cm.ResourceVersion}
141+
for _, item := range sliceList.Items {
142+
versions[item.Name] = item.ResourceVersion
143+
}
144+
145+
time.Sleep(1100 * time.Millisecond)
146+
_, err = forwarderRec.Reconcile(ctx, reconcile.Request{NamespacedName: fwdSvcNN})
147+
Expect(err).NotTo(HaveOccurred())
148+
149+
Expect(k8sClient.Get(ctx, fwdSvcNN, cm)).To(Succeed())
150+
Expect(cm.ResourceVersion).To(Equal(versions[cm.Name]))
151+
Expect(k8sClient.List(ctx, sliceList, client.InNamespace(namespace), client.MatchingLabels{EgressRouterNameLabel: routerName})).To(Succeed())
152+
for _, item := range sliceList.Items {
153+
Expect(item.ResourceVersion).To(Equal(versions[item.Name]), item.Name)
154+
}
155+
})
156+
157+
It("prunes endpoint slices that are no longer desired", func() {
158+
stray := &discoveryv1.EndpointSlice{
159+
ObjectMeta: metav1.ObjectMeta{
160+
Name: "stray",
161+
Namespace: namespace,
162+
Labels: map[string]string{
163+
discoveryv1.LabelServiceName: "gone",
164+
EgressRouterNameLabel: routerName,
165+
EgressRouterNamespaceLabel: namespace,
166+
},
167+
},
168+
AddressType: discoveryv1.AddressTypeIPv4,
169+
Endpoints: []discoveryv1.Endpoint{
170+
{Addresses: []string{"10.0.0.2"}},
171+
},
172+
}
173+
Expect(k8sClient.Create(ctx, stray)).To(Succeed())
174+
175+
_, err := forwarderRec.Reconcile(ctx, reconcile.Request{NamespacedName: fwdSvcNN})
176+
Expect(err).NotTo(HaveOccurred())
177+
178+
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(stray), stray)
179+
Expect(kerrors.IsNotFound(err)).To(BeTrue())
180+
})
181+
})
182+
})

0 commit comments

Comments
 (0)