Fix comments

Signed-off-by: Sebastian Sch <[email protected]>
k8snetworkplumbingwg · Sep 27, 2023 · 53cfd3d · 53cfd3d
1 parent 835f5ac
commit 53cfd3d
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 17 deletions.
diff --git a/pkg/utils/mock/mock_store.go b/pkg/utils/mock/mock_store.go
diff --git a/pkg/utils/store.go b/pkg/utils/store.go
@@ -22,7 +22,7 @@ const (
 //go:generate ../../bin/mockgen -destination mock/mock_store.go -source store.go
 type StoreManagerInterface interface {
 	ClearPCIAddressFolder() error
-	SaveLastPfAppliedStatus(pciAddress string, PfInfo *sriovnetworkv1.Interface) error
+	SaveLastPfAppliedStatus(PfInfo *sriovnetworkv1.Interface) error
 	LoadPfsStatus(pciAddress string) (*sriovnetworkv1.Interface, bool, error)
 }
 
@@ -82,7 +82,7 @@ func (s *StoreManager) ClearPCIAddressFolder() error {
 		if os.IsNotExist(err) {
 			return nil
 		}
-		return fmt.Errorf("failed to check the pci address folder path %s", PfAppliedConfigUse)
+		return fmt.Errorf("failed to check the pci address folder path %s: %v", PfAppliedConfigUse, err)
 	}
 
 	err = os.RemoveAll(PfAppliedConfigUse)
@@ -100,15 +100,15 @@ func (s *StoreManager) ClearPCIAddressFolder() error {
 
 // SaveLastPfAppliedStatus will save the PF object as a json into the /etc/sriov-operator/pci/<pci-address>
 // this function must be called after running the chroot function
-func (s *StoreManager) SaveLastPfAppliedStatus(pciAddress string, PfInfo *sriovnetworkv1.Interface) error {
+func (s *StoreManager) SaveLastPfAppliedStatus(PfInfo *sriovnetworkv1.Interface) error {
 	data, err := json.Marshal(PfInfo)
 	if err != nil {
 		glog.Errorf("failed to marshal PF status %+v: %v", *PfInfo, err)
 		return err
 	}
 
 	hostExtension := getHostExtension(s.RunOnHost)
-	pathFile := filepath.Join(hostExtension, PfAppliedConfig, pciAddress)
+	pathFile := filepath.Join(hostExtension, PfAppliedConfig, PfInfo.PciAddress)
 	err = os.WriteFile(pathFile, data, 0644)
 	return err
 }

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -45,6 +45,7 @@ const (
 	udevFolder      = "/etc/udev"
 	udevRulesFolder = udevFolder + "/rules.d"
 	udevDisableNM   = "/bindata/scripts/udev-find-sriov-pf.sh"
+	nmUdevRule      = "SUBSYSTEM==\"net\", ACTION==\"add|change|move\", ATTRS{device}==\"%s\", IMPORT{program}=\"/etc/udev/disable-nm-sriov.sh $env{INTERFACE} %s\""
 )
 
 var InitialState sriovnetworkv1.SriovNetworkNodeState
@@ -135,10 +136,10 @@ func DiscoverSriovDevices(withUnsupported bool, storeManager StoreManagerInterfa
 		pfStatus, exist, err := storeManager.LoadPfsStatus(iface.PciAddress)
 		if err != nil {
 			glog.Warningf("DiscoverSriovDevices(): failed to load PF status from disk: %v", err)
-		}
-
-		if exist {
-			iface.ExternallyManaged = pfStatus.ExternallyManaged
+		} else {
+			if exist {
+				iface.ExternallyManaged = pfStatus.ExternallyManaged
+			}
 		}
 
 		if dputils.IsSriovPF(device.Address) {
@@ -196,7 +197,7 @@ func ConfigSriovInterfaces(interfaces []sriovnetworkv1.Interface, ifaceStatuses
 					glog.V(2).Infof("syncNodeState(): no need update interface %s", iface.PciAddress)
 
 					// Save the PF status to the host
-					err = storeManager.SaveLastPfAppliedStatus(iface.PciAddress, &iface)
+					err = storeManager.SaveLastPfAppliedStatus(&iface)
 					if err != nil {
 						glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err)
 						return err
@@ -217,7 +218,7 @@ func ConfigSriovInterfaces(interfaces []sriovnetworkv1.Interface, ifaceStatuses
 				}
 
 				// Save the PF status to the host
-				err = storeManager.SaveLastPfAppliedStatus(iface.PciAddress, &iface)
+				err = storeManager.SaveLastPfAppliedStatus(&iface)
 				if err != nil {
 					glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err)
 					return err
@@ -930,9 +931,10 @@ func PrepareNMUdevRule(supportedVfIds []string) error {
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
+		glog.Errorf("PrepareNMUdevRule(): failed to prepare nmUdevRule, stderr %s: %v", stderr.String(), err)
 		return err
 	}
-	glog.V(2).Infof("PrepareNMUdevRule(): %v", cmd.Stdout)
+	glog.V(2).Infof("PrepareNMUdevRule(): %v", stdout.String())
 
 	//save the device list to use for udev rules
 	SupportedVfIds = supportedVfIds
@@ -942,7 +944,7 @@ func PrepareNMUdevRule(supportedVfIds []string) error {
 func AddUdevRule(pfPciAddress string) error {
 	glog.V(2).Infof("AddUdevRule(): %s", pfPciAddress)
 	pathFile := udevRulesFolder
-	udevRuleContent := fmt.Sprintf("SUBSYSTEM==\"net\", ACTION==\"add|change|move\", ATTRS{device}==\"%s\", IMPORT{program}=\"/etc/udev/disable-nm-sriov.sh $env{INTERFACE} %s\"", strings.Join(SupportedVfIds, "|"), pfPciAddress)
+	udevRuleContent := fmt.Sprintf(nmUdevRule, strings.Join(SupportedVfIds, "|"), pfPciAddress)
 
 	err := os.MkdirAll(pathFile, os.ModePerm)
 	if err != nil && !os.IsExist(err) {

diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
@@ -304,7 +304,7 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s
 				}
 
 				if policy.Spec.LinkType != "" && strings.ToLower(policy.Spec.LinkType) != strings.ToLower(iface.LinkType) {
-					return fmt.Errorf("LinkType(%d) in CR %s is not equal to the LinkType for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu)
+					return fmt.Errorf("LinkType(%s) in CR %s is not equal to the LinkType for the PF externally value(%s)", policy.Spec.LinkType, policy.GetName(), iface.LinkType)
 				}
 			}
 			// vdpa: only mellanox cards are supported