Skip to content

Commit ee2d357

Browse files
committed
fix: set correct nadFound=true after NAD creation in updateConditions
After successfully creating a NetworkAttachmentDefinition, the controller was incorrectly calling updateConditions with nadFound=false, which set the status to 'waiting for namespace' instead of 'ready'. This fix: 1. Changes nadFound from false to true after successful NAD creation 2. Changes err to nil (since there's no error on success) 3. Adds a unit test that specifically verifies conditions are set to Ready=True immediately after NAD creation The bug was reported by Gemini code review on PR k8snetworkplumbingwg#1010. Signed-off-by: Sebastian Sch <sebassch@gmail.com>
1 parent 60345b1 commit ee2d357

File tree

2 files changed

+50
-1
lines changed

2 files changed

+50
-1
lines changed

controllers/generic_network_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (r *genericNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque
220220
}
221221

222222
// Update conditions to reflect successful creation
223-
updateErr := r.updateConditions(ctx, instance, false, err)
223+
updateErr := r.updateConditions(ctx, instance, true, nil)
224224
if updateErr != nil {
225225
reqLogger.Error(updateErr, "Failed to update conditions")
226226
return reconcile.Result{}, updateErr

controllers/generic_network_controller_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,55 @@ var _ = Describe("All Network Controllers", Ordered, func() {
271271
}).WithTimeout(5 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed())
272272
})
273273

274+
It("should set Ready=True immediately after NAD creation (not waiting state)", func() {
275+
// This test verifies the fix for the bug where after successfully creating
276+
// the NetworkAttachmentDefinition, the controller incorrectly set conditions
277+
// indicating it was waiting for the namespace instead of being ready.
278+
cr := sriovnetworkv1.SriovNetwork{
279+
ObjectMeta: metav1.ObjectMeta{
280+
Name: "test-nad-creation-condition",
281+
Namespace: testNamespace,
282+
},
283+
Spec: sriovnetworkv1.SriovNetworkSpec{
284+
NetworkNamespace: "default",
285+
ResourceName: "test-resource-creation",
286+
},
287+
}
288+
289+
err := k8sClient.Create(ctx, &cr)
290+
Expect(err).NotTo(HaveOccurred())
291+
292+
// First, wait for the NAD to be created
293+
netAttDef := &netattdefv1.NetworkAttachmentDefinition{}
294+
err = util.WaitForNamespacedObject(netAttDef, k8sClient, "default", cr.GetName(), util.RetryInterval, util.Timeout)
295+
Expect(err).NotTo(HaveOccurred())
296+
297+
// Now check that conditions are correctly set to Ready=True
298+
// This should happen immediately after NAD creation, not require a second reconcile
299+
Eventually(func(g Gomega) {
300+
network := &sriovnetworkv1.SriovNetwork{}
301+
err := k8sClient.Get(ctx, client.ObjectKey{Name: cr.Name, Namespace: cr.Namespace}, network)
302+
g.Expect(err).NotTo(HaveOccurred())
303+
304+
// Conditions must be set
305+
g.Expect(network.Status.Conditions).ToNot(BeEmpty())
306+
307+
// Ready must be True (not False with "waiting for namespace" reason)
308+
readyCondition := findCondition(network.Status.Conditions, sriovnetworkv1.ConditionReady)
309+
g.Expect(readyCondition).ToNot(BeNil())
310+
g.Expect(readyCondition.Status).To(Equal(metav1.ConditionTrue),
311+
"Ready condition should be True after NAD creation, got %s with reason %s",
312+
readyCondition.Status, readyCondition.Reason)
313+
g.Expect(readyCondition.Reason).To(Equal(sriovnetworkv1.ReasonNetworkReady),
314+
"Ready reason should be NetworkReady, not waiting for namespace")
315+
316+
// Degraded must be False
317+
degradedCondition := findCondition(network.Status.Conditions, sriovnetworkv1.ConditionDegraded)
318+
g.Expect(degradedCondition).ToNot(BeNil())
319+
g.Expect(degradedCondition.Status).To(Equal(metav1.ConditionFalse))
320+
}).WithTimeout(5 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed())
321+
})
322+
274323
It("should set Degraded condition to False when network is healthy", func() {
275324
cr := sriovnetworkv1.SriovIBNetwork{
276325
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)