Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ func (dn *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
}
// periodically ensure device plugin is unblocked,
// this is required to ensure that device plugin can start in case if it is restarted for some reason
if vars.FeatureGate.IsEnabled(consts.BlockDevicePluginUntilConfiguredFeatureGate) {
if vars.FeatureGate.IsEnabled(consts.BlockDevicePluginUntilConfiguredFeatureGate) &&
len(desiredNodeState.Spec.Interfaces) > 0 {
devicePluginPods, err := dn.getDevicePluginPodsForNode(ctx)
if err != nil {
reqLogger.Error(err, "failed to get device plugin pods")
Expand Down Expand Up @@ -410,9 +411,13 @@ func (dn *NodeReconciler) apply(ctx context.Context, desiredNodeState *sriovnetw
}

if vars.FeatureGate.IsEnabled(consts.BlockDevicePluginUntilConfiguredFeatureGate) {
if err := dn.waitForDevicePluginPodAndTryUnblock(ctx, desiredNodeState); err != nil {
reqLogger.Error(err, "failed to wait for device plugin pod to start and try to unblock it")
return ctrl.Result{}, err
if len(desiredNodeState.Spec.Interfaces) == 0 {
reqLogger.Info("no interfaces in desired state, skipping device plugin wait as device plugin won't be deployed")
} else {
if err := dn.waitForDevicePluginPodAndTryUnblock(ctx, desiredNodeState); err != nil {
reqLogger.Error(err, "failed to wait for device plugin pod to start and try to unblock it")
return ctrl.Result{}, err
}
}
Comment on lines +414 to 421

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better readability, you could invert the condition to check for the positive case first. This avoids having an else block that only contains an if statement, which can make the code slightly easier to follow.

Suggested change
if len(desiredNodeState.Spec.Interfaces) == 0 {
reqLogger.Info("no interfaces in desired state, skipping device plugin wait as device plugin won't be deployed")
} else {
if err := dn.waitForDevicePluginPodAndTryUnblock(ctx, desiredNodeState); err != nil {
reqLogger.Error(err, "failed to wait for device plugin pod to start and try to unblock it")
return ctrl.Result{}, err
}
}
if len(desiredNodeState.Spec.Interfaces) > 0 {
if err := dn.waitForDevicePluginPodAndTryUnblock(ctx, desiredNodeState); err != nil {
reqLogger.Error(err, "failed to wait for device plugin pod to start and try to unblock it")
return ctrl.Result{}, err
}
} else {
reqLogger.Info("no interfaces in desired state, skipping device plugin wait as device plugin won't be deployed")
}

} else {
// if the feature gate is not enabled we preserver the old behavior
Expand Down
33 changes: 33 additions & 0 deletions pkg/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,39 @@ var _ = Describe("Daemon Controller", Ordered, func() {
}, waitTime, retryTime).Should(Succeed())
})

It("Should not wait for device plugin pod when there are no interfaces and blockDevicePluginUntilConfigured is enabled", func(ctx context.Context) {
DeferCleanup(func(x bool) { vars.DisableDrain = x }, vars.DisableDrain)
vars.DisableDrain = true

By("waiting for drain idle states")
EventuallyWithOffset(1, func(g Gomega) {
g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)).
ToNot(HaveOccurred())

g.Expect(nodeState.Annotations[constants.NodeStateDrainAnnotation]).To(Equal(constants.DrainIdle))
g.Expect(nodeState.Annotations[constants.NodeStateDrainAnnotationCurrent]).To(Equal(constants.DrainIdle))
}, waitTime, retryTime).Should(Succeed())

By("setting an empty interfaces spec (no policies)")
EventuallyWithOffset(1, func(g Gomega) {
err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
g.Expect(err).ToNot(HaveOccurred())

nodeState.Spec.Interfaces = []sriovnetworkv1.Interface{}
err = k8sClient.Update(ctx, nodeState)
g.Expect(err).ToNot(HaveOccurred())
}, waitTime, retryTime).Should(Succeed())

By("verifying sync status reaches Succeeded without stalling on device plugin wait")
eventuallySyncStatusEqual(nodeState, constants.SyncStatusSucceeded)

By("verifying the device plugin pod still has the wait-for-config annotation (was not unblocked)")
devicePluginPod := &corev1.Pod{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: devicePluginPodName, Namespace: testNamespace},
devicePluginPod)).ToNot(HaveOccurred())
Expect(devicePluginPod.Annotations).To(HaveKey(constants.DevicePluginWaitConfigAnnotation))
})

It("Should unblock the device plugin pod when configuration is finished", func(ctx context.Context) {
DeferCleanup(func(x bool) { vars.DisableDrain = x }, vars.DisableDrain)
vars.DisableDrain = true
Expand Down
Loading