Merge pull request #999 from rollandf/daemonresult

fix: check for sriov result file in systemd mode

Author: SchSeba

pkg/daemon/daemon.go

@@ -181,7 +181,7 @@ func (dn *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	}
 
 	// if we are running in systemd mode we want to get the sriov result from the config-daemon that runs in systemd
-	sriovResult, sriovResultExists, err := dn.checkSystemdStatus()
+	sriovResult, sriovResultExists, err := dn.CheckSystemdStatus()
 	//TODO: in the case we need to think what to do if we try to apply again or not
 	if err != nil {
 		reqLogger.Error(err, "failed to check systemd status unexpected error")
@@ -322,14 +322,14 @@ func (dn *NodeReconciler) checkOnNodeStateChange(desiredNodeState *sriovnetworkv
 	return reqReboot, reqDrain, nil
 }
 
-// checkSystemdStatus Checks the status of systemd services on the host node.
+// CheckSystemdStatus Checks the status of systemd services on the host node.
 // return the sriovResult struct a boolean if the result file exist on the node
-func (dn *NodeReconciler) checkSystemdStatus() (*hosttypes.SriovResult, bool, error) {
+func (dn *NodeReconciler) CheckSystemdStatus() (*hosttypes.SriovResult, bool, error) {
 	if !vars.UsingSystemdMode {
 		return nil, false, nil
 	}
 
-	funcLog := log.Log.WithName("checkSystemdStatus")
+	funcLog := log.Log.WithName("CheckSystemdStatus")
 	serviceEnabled, err := dn.hostHelpers.IsServiceEnabled(consts.SriovServicePath)
 	if err != nil {
 		funcLog.Error(err, "failed to check if sriov-config service exist on host")
@@ -351,12 +351,12 @@ func (dn *NodeReconciler) checkSystemdStatus() (*hosttypes.SriovResult, bool, er
 
 	// check if the service exist
 	if serviceEnabled && postNetworkServiceEnabled {
-		exist = true
 		sriovResult, err = dn.hostHelpers.ReadSriovResult()
 		if err != nil {
 			funcLog.Error(err, "failed to load sriov result file from host")
 			return nil, false, err
 		}
+		exist = sriovResult != nil
 	}
 	return sriovResult, exist, nil
 }

pkg/daemon/daemon_test.go

@@ -2,6 +2,7 @@ package daemon_test
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"sync"
 	"time"
@@ -689,6 +690,90 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 	})
 })
 
+var _ = Describe("Daemon CheckSystemdStatus", func() {
+	var (
+		myMockCtrl   *gomock.Controller
+		myHostHelper *mock_helper.MockHostHelpersInterface
+		reconciler   *daemon.NodeReconciler
+	)
+
+	BeforeEach(func() {
+		myMockCtrl = gomock.NewController(GinkgoT())
+		myHostHelper = mock_helper.NewMockHostHelpersInterface(myMockCtrl)
+		reconciler = daemon.New(nil, myHostHelper, nil, nil, nil)
+
+		originalUsingSystemdMode := vars.UsingSystemdMode
+		vars.UsingSystemdMode = true
+		DeferCleanup(func() {
+			vars.UsingSystemdMode = originalUsingSystemdMode
+		})
+	})
+
+	Context("when systemd services are enabled", func() {
+		BeforeEach(func() {
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovServicePath).Return(true, nil)
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovPostNetworkServicePath).Return(true, nil)
+		})
+
+		It("should return exist=true if sriov result file exists", func() {
+			myHostHelper.EXPECT().ReadSriovResult().Return(&hostTypes.SriovResult{}, nil)
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exist).To(BeTrue())
+			Expect(result).ToNot(BeNil())
+		})
+
+		It("should return exist=false if sriov result file does not exist", func() {
+			myHostHelper.EXPECT().ReadSriovResult().Return(nil, nil)
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exist).To(BeFalse())
+			Expect(result).To(BeNil())
+		})
+
+		It("should return error if ReadSriovResult fails", func() {
+			myHostHelper.EXPECT().ReadSriovResult().Return(nil, fmt.Errorf("read error"))
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).To(HaveOccurred())
+			Expect(exist).To(BeFalse())
+			Expect(result).To(BeNil())
+		})
+	})
+
+	Context("when systemd services are NOT enabled", func() {
+		It("should return exist=false if first service is disabled", func() {
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovServicePath).Return(false, nil)
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovPostNetworkServicePath).Return(true, nil)
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exist).To(BeFalse())
+			Expect(result.SyncStatus).To(Equal(constants.SyncStatusFailed))
+		})
+
+		It("should return exist=false if second service is disabled", func() {
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovServicePath).Return(true, nil)
+			myHostHelper.EXPECT().IsServiceEnabled(constants.SriovPostNetworkServicePath).Return(false, nil)
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exist).To(BeFalse())
+			Expect(result.SyncStatus).To(Equal(constants.SyncStatusFailed))
+		})
+	})
+
+	Context("when not in systemd mode", func() {
+		BeforeEach(func() {
+			vars.UsingSystemdMode = false
+		})
+
+		It("should return exist=false and no error", func() {
+			result, exist, err := reconciler.CheckSystemdStatus()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exist).To(BeFalse())
+			Expect(result).To(BeNil())
+		})
+	})
+})
+
 func patchAnnotation(nodeState *sriovnetworkv1.SriovNetworkNodeState, key, value string) {
 	originalNodeState := nodeState.DeepCopy()
 	nodeState.Annotations[key] = value