Skip to content

Commit 0bf7a6b

Browse files
zhouhaibing089squeed
authored andcommitted
ciliumendpoint-gc: skip ciliumendpoints which are too new
I have see real production occurrences when a ciliumendpoint was just created, but then immediately deleted by cilium operator. The timing is a strong indicator that we have hit this [known flaw][1] where it is possible to delete a ciliumendpoint because the operator haven't received the pod notification. To reduce the chance of this happenning further, I propose that we wait for at least one interval before starting to delete a just-created ciliumendpoint. [1]: https://github.com/cilium/cilium/blob/2e216d1d735073932074497e1687c73bc2f56d1f/operator/endpointgc/gc.go#L134-L137 Signed-off-by: Haibing Zhou <zhouhaibing089@gmail.com>
1 parent 0b60dac commit 0bf7a6b

2 files changed

Lines changed: 16 additions & 2 deletions

File tree

operator/endpointgc/gc.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,16 @@ func (g *GC) checkIfCEPShouldBeDeleted(ctx context.Context, cep *cilium_api_v2.C
156156
return true
157157
}
158158

159+
// In rare cases when ciliumendpoint is too new, and we haven't received pod
160+
// notifications, we might delete this ciliumendpoint unexpectedly. Whie we
161+
// are still prone to the cache delay if g.interval is too small or when the
162+
// watch cache for pods has an arbitrary delay, the chance is much lower than
163+
// it is today.
164+
if time.Since(cep.CreationTimestamp.Time) < g.interval {
165+
scopedLog.DebugContext(ctx, "CiliumEndpoint is too new", logfields.Name, cep.Namespace+"/"+cep.Name)
166+
return false
167+
}
168+
159169
podChecked := false
160170
podStore, err := g.pods.Store(ctx)
161171
if err != nil {

operator/endpointgc/gc_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestRegisterController(t *testing.T) {
5858
cepStore, _ := ciliumEndpoint.Store(t.Context())
5959
// wait for all CEPs to be deleted except for those with running pods or
6060
// cilium node owner reference
61-
waitForCEPs(t, cepStore, 2)
61+
waitForCEPs(t, cepStore, 3)
6262
if err := hive.Stop(tlog, t.Context()); err != nil {
6363
t.Fatalf("failed to stop: %s", err)
6464
}
@@ -134,7 +134,7 @@ func TestRegisterControllerWithCRDDisabled(t *testing.T) {
134134
// wait for potential GC
135135
time.Sleep(500 * time.Millisecond)
136136
// gc is disabled so no CEPs should be deleted
137-
waitForCEPs(t, cepStore, 6)
137+
waitForCEPs(t, cepStore, 7)
138138
if err := hive.Stop(tlog, t.Context()); err != nil {
139139
t.Fatalf("failed to stop: %s", err)
140140
}
@@ -170,6 +170,10 @@ func prepareCiliumEndpoints(t *testing.T, fakeClient *k8sClient.FakeClientset) {
170170
cepWithOwnerPodDoesntExist := createCiliumEndpoint("cep6", "ns")
171171
cepWithOwnerPodDoesntExist.OwnerReferences = []meta_v1.OwnerReference{createOwnerReference("Pod", "pod6")}
172172
fakeClient.CiliumV2().CiliumEndpoints("ns").Create(t.Context(), cepWithOwnerPodDoesntExist, meta_v1.CreateOptions{})
173+
// - CEP that is just created
174+
cepWithRecentCreationTime := createCiliumEndpoint("cep7", "ns")
175+
cepWithRecentCreationTime.CreationTimestamp = meta_v1.Now()
176+
fakeClient.CiliumV2().CiliumEndpoints("ns").Create(t.Context(), cepWithRecentCreationTime, meta_v1.CreateOptions{})
173177

174178
// Create Pods
175179
// - pod that is running for cep2

0 commit comments

Comments
 (0)