fix: Validate the linkType correctly when it can and cannot be changed

If LINK_TYPE_P1 parameter is not present in the mstconfig defaults, we can't change the link type on the device and should return an error if the actual link type differs from the template. Otherwise, we should not raise an error. Signed-off-by: amaslennikov <[email protected]>
Mellanox · Oct 1, 2024 · 84ceef9 · 84ceef9
1 parent 0463dbc
commit 84ceef9
Show file tree

Hide file tree

Showing 2 changed files with 126 additions and 17 deletions.
diff --git a/pkg/host/configvalidation.go b/pkg/host/configvalidation.go
@@ -89,24 +89,26 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(
 		desiredParameters[consts.SriovNumOfVfsParam] = strconv.Itoa(template.NumVfs)
 	}
 
-	if template.LinkType != "" {
-		// Link type change is not allowed on some devices
-		if _, found := defaultValues[consts.LinkTypeP1Param]; found {
-			linkType := nvParamLinkTypeFromName(string(template.LinkType))
-			desiredParameters[consts.LinkTypeP1Param] = linkType
-			if secondPortPresent {
-				desiredParameters[consts.LinkTypeP2Param] = linkType
-			}
+	// Link type change is not allowed on some devices
+	_, canChangeLinkType := defaultValues[consts.LinkTypeP1Param]
+	if canChangeLinkType {
+		linkType := nvParamLinkTypeFromName(string(template.LinkType))
+		desiredParameters[consts.LinkTypeP1Param] = linkType
+		if secondPortPresent {
+			desiredParameters[consts.LinkTypeP2Param] = linkType
 		}
-		if device.Status.Ports[0].NetworkInterface != "" &&
-			v.utils.GetLinkType(device.Status.Ports[0].NetworkInterface) !=
-				string(device.Spec.Configuration.Template.LinkType) {
-			err := types.IncorrectSpecError(
-				fmt.Sprintf(
-					"device doesn't support link type change, wrong link type provided in the template, should be: %s",
-					v.utils.GetLinkType(pciAddr)))
-			log.Log.Error(err, "incorrect spec", "device", device.Name)
-			return desiredParameters, err
+	} else {
+		desiredLinkType := string(device.Spec.Configuration.Template.LinkType)
+
+		for _, port := range device.Status.Ports {
+			if port.NetworkInterface != "" && v.utils.GetLinkType(port.NetworkInterface) != desiredLinkType {
+				err := types.IncorrectSpecError(
+					fmt.Sprintf(
+						"device doesn't support link type change, wrong link type provided in the template, should be: %s",
+						v.utils.GetLinkType(port.NetworkInterface)))
+				log.Log.Error(err, "incorrect spec", "device", device.Name)
+				return desiredParameters, err
+			}
 		}
 	}
 

diff --git a/pkg/host/configvalidation_test.go b/pkg/host/configvalidation_test.go
@@ -334,6 +334,113 @@ var _ = Describe("ConfigValidationImpl", func() {
 			Expect(nvParams).To(HaveKeyWithValue("TEST_P1", "test"))
 			Expect(nvParams).To(HaveKeyWithValue("TEST_P2", "test"))
 		})
+		It("should report an error when LinkType can be changed and template differs from the actual status", func() {
+			mockHostUtils.On("GetLinkType", mock.Anything).Return(consts.Ethernet)
+			mockHostUtils.On("GetPCILinkSpeed", mock.Anything).Return(16, nil)
+
+			device := &v1alpha1.NicDevice{
+				Spec: v1alpha1.NicDeviceSpec{
+					Configuration: &v1alpha1.NicDeviceConfigurationSpec{
+						Template: &v1alpha1.ConfigurationTemplateSpec{
+							NumVfs:   0,
+							LinkType: consts.Infiniband,
+							PciPerformanceOptimized: &v1alpha1.PciPerformanceOptimizedSpec{
+								Enabled: true,
+							},
+						},
+					},
+				},
+				Status: v1alpha1.NicDeviceStatus{
+					Ports: []v1alpha1.NicDevicePortSpec{
+						{
+							PCI:              "0000:03:00.0",
+							NetworkInterface: "enp3s0f0np0",
+						},
+						{
+							PCI:              "0000:03:00.1",
+							NetworkInterface: "enp3s0f1np1",
+						},
+					},
+				},
+			}
+
+			defaultValues := map[string]string{}
+
+			_, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues)
+			Expect(err).To(MatchError("incorrect spec: device doesn't support link type change, wrong link type provided in the template, should be: Ethernet"))
+		})
+		It("should not report an error when LinkType cannot be changed and template differs from the actual status", func() {
+			mockHostUtils.On("GetLinkType", mock.Anything).Return(consts.Ethernet)
+			mockHostUtils.On("GetPCILinkSpeed", mock.Anything).Return(16, nil)
+
+			device := &v1alpha1.NicDevice{
+				Spec: v1alpha1.NicDeviceSpec{
+					Configuration: &v1alpha1.NicDeviceConfigurationSpec{
+						Template: &v1alpha1.ConfigurationTemplateSpec{
+							NumVfs:   0,
+							LinkType: consts.Infiniband,
+							PciPerformanceOptimized: &v1alpha1.PciPerformanceOptimizedSpec{
+								Enabled: true,
+							},
+						},
+					},
+				},
+				Status: v1alpha1.NicDeviceStatus{
+					Ports: []v1alpha1.NicDevicePortSpec{
+						{
+							PCI:              "0000:03:00.0",
+							NetworkInterface: "enp3s0f0np0",
+						},
+						{
+							PCI:              "0000:03:00.1",
+							NetworkInterface: "enp3s0f1np1",
+						},
+					},
+				},
+			}
+
+			defaultValues := map[string]string{
+				consts.LinkTypeP1Param: consts.Ethernet,
+			}
+
+			_, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues)
+			Expect(err).NotTo(HaveOccurred())
+		})
+		It("should not report an error when LinkType cannot be changed and template matches the actual status", func() {
+			mockHostUtils.On("GetLinkType", mock.Anything).Return(consts.Infiniband)
+			mockHostUtils.On("GetPCILinkSpeed", mock.Anything).Return(16, nil)
+
+			device := &v1alpha1.NicDevice{
+				Spec: v1alpha1.NicDeviceSpec{
+					Configuration: &v1alpha1.NicDeviceConfigurationSpec{
+						Template: &v1alpha1.ConfigurationTemplateSpec{
+							NumVfs:   0,
+							LinkType: consts.Infiniband,
+							PciPerformanceOptimized: &v1alpha1.PciPerformanceOptimizedSpec{
+								Enabled: true,
+							},
+						},
+					},
+				},
+				Status: v1alpha1.NicDeviceStatus{
+					Ports: []v1alpha1.NicDevicePortSpec{
+						{
+							PCI:              "0000:03:00.0",
+							NetworkInterface: "enp3s0f0np0",
+						},
+						{
+							PCI:              "0000:03:00.1",
+							NetworkInterface: "enp3s0f1np1",
+						},
+					},
+				},
+			}
+
+			defaultValues := map[string]string{}
+
+			_, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues)
+			Expect(err).NotTo(HaveOccurred())
+		})
 	})
 
 	Describe("ValidateResetToDefault", func() {