Skip to content
This repository was archived by the owner on Mar 27, 2026. It is now read-only.

Commit ed82489

Browse files
wnagelewnagele
committed
Fix race conditions
1 parent 0b2b7a8 commit ed82489

File tree

9 files changed

+659
-7
lines changed

9 files changed

+659
-7
lines changed

‎internal/controller/vpcattachment_controller.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,6 @@ func (r *VPCAttachmentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
7070
vpcAttachment.Status.Identifier, _ = r.Identifier.ForVPCAttachment()
7171
}
7272

73-
vpcAttachment.Status.Ready = true
74-
7573
if err := r.Status().Update(ctx, &vpcAttachment); err != nil {
7674
return ctrl.Result{}, err
7775
}
@@ -103,6 +101,13 @@ func (r *VPCAttachmentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
103101
return ctrl.Result{}, err
104102
}
105103

104+
if !vpcAttachment.Status.Ready {
105+
vpcAttachment.Status.Ready = true
106+
if err := r.Status().Update(ctx, &vpcAttachment); err != nil {
107+
return ctrl.Result{}, err
108+
}
109+
}
110+
106111
return ctrl.Result{}, nil
107112
}
108113

‎internal/webhook/v1/pod_webhook.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,11 @@ func (d *PodCustomDefaulter) Default(ctx context.Context, obj runtime.Object) er
5555
return nil
5656
}
5757

58-
if vpcAttachment, _ := vpcAttachmentByName(d.Client, ctx, pod.Annotations[galacticv1alpha.VPCAttachmentAnnotation], pod.GetNamespace()); vpcAttachment != nil {
59-
pod.Annotations[PodAnnotationMultusNetworks] = fmt.Sprintf("%s@%s", vpcAttachment.Name, vpcAttachment.Spec.Interface.Name)
58+
vpcAttachment, err := vpcAttachmentByName(d.Client, ctx, pod.Annotations[galacticv1alpha.VPCAttachmentAnnotation], pod.GetNamespace())
59+
if err != nil {
60+
return err
6061
}
62+
pod.Annotations[PodAnnotationMultusNetworks] = fmt.Sprintf("%s@%s", vpcAttachment.Name, vpcAttachment.Spec.Interface.Name)
6163

6264
return nil
6365
}
@@ -115,5 +117,8 @@ func vpcAttachmentByName(k8sClient client.Client, ctx context.Context, name, nam
115117
if err := k8sClient.Get(ctx, typeNamespacedName, &vpcAttachment); err != nil {
116118
return nil, err
117119
}
120+
if !vpcAttachment.Status.Ready {
121+
return nil, fmt.Errorf("VPCAttachment %s/%s is not ready", namespace, name)
122+
}
118123
return &vpcAttachment, nil
119124
}

‎internal/webhook/v1/pod_webhook_test.go‎

Lines changed: 214 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ var _ = Describe("Pod Webhook", func() {
4848
},
4949
}
5050
Expect(k8sClient.Create(ctx, vpcAttachment)).To(Succeed())
51+
vpcAttachment.Status.Ready = true
52+
Expect(k8sClient.Status().Update(ctx, vpcAttachment)).To(Succeed())
5153
})
5254

5355
AfterEach(func() {
@@ -96,7 +98,7 @@ var _ = Describe("Pod Webhook", func() {
9698
})
9799

98100
Context("When creating a Pod with invalid VPC attachment", func() {
99-
It("should not set the networks annotation", func() {
101+
It("should reject the pod", func() {
100102
pod = &corev1.Pod{
101103
ObjectMeta: metav1.ObjectMeta{
102104
Name: "test-pod",
@@ -125,8 +127,217 @@ var _ = Describe("Pod Webhook", func() {
125127
Client: k8sClient,
126128
Scheme: k8sClient.Scheme(),
127129
}
128-
Expect(defaulter.Default(ctx, pod)).Error().NotTo(HaveOccurred())
129-
Expect(pod.Annotations).NotTo(HaveKey(PodAnnotationMultusNetworks))
130+
Expect(defaulter.Default(ctx, pod)).Error().To(HaveOccurred())
131+
})
132+
})
133+
})
134+
135+
var _ = Describe("Pod Webhook Without VPCAttachment Annotation", func() {
136+
It("should allow the pod without modification", func() {
137+
pod := &corev1.Pod{
138+
ObjectMeta: metav1.ObjectMeta{
139+
Name: "test-pod",
140+
Namespace: "default",
141+
},
142+
Spec: corev1.PodSpec{
143+
Containers: []corev1.Container{
144+
{
145+
Name: "test-container",
146+
Image: "test:latest",
147+
},
148+
},
149+
},
150+
}
151+
152+
defaulter := PodCustomDefaulter{
153+
Client: k8sClient,
154+
Scheme: k8sClient.Scheme(),
155+
}
156+
Expect(defaulter.Default(ctx, pod)).NotTo(HaveOccurred())
157+
Expect(pod.Annotations).NotTo(HaveKey(PodAnnotationMultusNetworks))
158+
159+
validator := PodCustomValidator{
160+
Client: k8sClient,
161+
Scheme: k8sClient.Scheme(),
162+
}
163+
Expect(validator.ValidateCreate(ctx, pod)).Error().NotTo(HaveOccurred())
164+
})
165+
})
166+
167+
var _ = Describe("Pod Webhook Race Conditions", func() {
168+
var (
169+
pod *corev1.Pod
170+
validator PodCustomValidator
171+
defaulter PodCustomDefaulter
172+
)
173+
174+
BeforeEach(func() {
175+
err := galacticv1alpha.AddToScheme(k8sClient.Scheme())
176+
Expect(err).NotTo(HaveOccurred())
177+
})
178+
179+
Context("When creating a Pod before VPCAttachment exists", func() {
180+
It("should reject the pod in both webhooks", func() {
181+
pod = &corev1.Pod{
182+
ObjectMeta: metav1.ObjectMeta{
183+
Name: "test-pod",
184+
Namespace: "default",
185+
Annotations: map[string]string{
186+
galacticv1alpha.VPCAttachmentAnnotation: "nonexistent-attachment",
187+
},
188+
},
189+
Spec: corev1.PodSpec{
190+
Containers: []corev1.Container{
191+
{
192+
Name: "test-container",
193+
Image: "test:latest",
194+
},
195+
},
196+
},
197+
}
198+
199+
defaulter = PodCustomDefaulter{
200+
Client: k8sClient,
201+
Scheme: k8sClient.Scheme(),
202+
}
203+
Expect(defaulter.Default(ctx, pod)).To(HaveOccurred())
204+
205+
validator = PodCustomValidator{
206+
Client: k8sClient,
207+
Scheme: k8sClient.Scheme(),
208+
}
209+
Expect(validator.ValidateCreate(ctx, pod)).Error().To(HaveOccurred())
210+
})
211+
})
212+
213+
Context("When creating a Pod with a VPCAttachment that is not ready", func() {
214+
var vpcAttachment *galacticv1alpha.VPCAttachment
215+
216+
BeforeEach(func() {
217+
vpcAttachment = &galacticv1alpha.VPCAttachment{
218+
ObjectMeta: metav1.ObjectMeta{
219+
Name: "not-ready-attachment",
220+
Namespace: "default",
221+
},
222+
Spec: galacticv1alpha.VPCAttachmentSpec{
223+
VPC: corev1.ObjectReference{
224+
APIVersion: "galactic.datumapis.com/v1alpha",
225+
Kind: "VPC",
226+
Name: "vpc-sample",
227+
Namespace: "default",
228+
},
229+
Interface: galacticv1alpha.VPCAttachmentInterface{
230+
Name: "galactic0",
231+
Addresses: []string{
232+
"10.1.1.1/24",
233+
},
234+
},
235+
},
236+
}
237+
Expect(k8sClient.Create(ctx, vpcAttachment)).To(Succeed())
238+
})
239+
240+
AfterEach(func() {
241+
Expect(k8sClient.Delete(ctx, vpcAttachment)).To(Succeed())
242+
})
243+
244+
It("should reject the pod in both webhooks", func() {
245+
pod = &corev1.Pod{
246+
ObjectMeta: metav1.ObjectMeta{
247+
Name: "test-pod",
248+
Namespace: "default",
249+
Annotations: map[string]string{
250+
galacticv1alpha.VPCAttachmentAnnotation: "not-ready-attachment",
251+
},
252+
},
253+
Spec: corev1.PodSpec{
254+
Containers: []corev1.Container{
255+
{
256+
Name: "test-container",
257+
Image: "test:latest",
258+
},
259+
},
260+
},
261+
}
262+
263+
defaulter = PodCustomDefaulter{
264+
Client: k8sClient,
265+
Scheme: k8sClient.Scheme(),
266+
}
267+
Expect(defaulter.Default(ctx, pod)).To(HaveOccurred())
268+
269+
validator = PodCustomValidator{
270+
Client: k8sClient,
271+
Scheme: k8sClient.Scheme(),
272+
}
273+
Expect(validator.ValidateCreate(ctx, pod)).Error().To(HaveOccurred())
274+
})
275+
})
276+
277+
Context("When creating a Pod with a ready VPCAttachment", func() {
278+
var vpcAttachment *galacticv1alpha.VPCAttachment
279+
280+
BeforeEach(func() {
281+
vpcAttachment = &galacticv1alpha.VPCAttachment{
282+
ObjectMeta: metav1.ObjectMeta{
283+
Name: "ready-attachment",
284+
Namespace: "default",
285+
},
286+
Spec: galacticv1alpha.VPCAttachmentSpec{
287+
VPC: corev1.ObjectReference{
288+
APIVersion: "galactic.datumapis.com/v1alpha",
289+
Kind: "VPC",
290+
Name: "vpc-sample",
291+
Namespace: "default",
292+
},
293+
Interface: galacticv1alpha.VPCAttachmentInterface{
294+
Name: "galactic0",
295+
Addresses: []string{
296+
"10.1.1.1/24",
297+
},
298+
},
299+
},
300+
}
301+
Expect(k8sClient.Create(ctx, vpcAttachment)).To(Succeed())
302+
vpcAttachment.Status.Ready = true
303+
Expect(k8sClient.Status().Update(ctx, vpcAttachment)).To(Succeed())
304+
})
305+
306+
AfterEach(func() {
307+
Expect(k8sClient.Delete(ctx, vpcAttachment)).To(Succeed())
308+
})
309+
310+
It("should allow the pod and set the Multus annotation", func() {
311+
pod = &corev1.Pod{
312+
ObjectMeta: metav1.ObjectMeta{
313+
Name: "test-pod",
314+
Namespace: "default",
315+
Annotations: map[string]string{
316+
galacticv1alpha.VPCAttachmentAnnotation: "ready-attachment",
317+
},
318+
},
319+
Spec: corev1.PodSpec{
320+
Containers: []corev1.Container{
321+
{
322+
Name: "test-container",
323+
Image: "test:latest",
324+
},
325+
},
326+
},
327+
}
328+
329+
defaulter = PodCustomDefaulter{
330+
Client: k8sClient,
331+
Scheme: k8sClient.Scheme(),
332+
}
333+
Expect(defaulter.Default(ctx, pod)).NotTo(HaveOccurred())
334+
Expect(pod.Annotations[PodAnnotationMultusNetworks]).To(Equal(fmt.Sprintf("%s@%s", "ready-attachment", "galactic0")))
335+
336+
validator = PodCustomValidator{
337+
Client: k8sClient,
338+
Scheme: k8sClient.Scheme(),
339+
}
340+
Expect(validator.ValidateCreate(ctx, pod)).Error().NotTo(HaveOccurred())
130341
})
131342
})
132343
})

0 commit comments

Comments
 (0)