Merge pull request #19 from almaslennikov/link-type-validation-fix

fix: Validate the linkType correctly when it can and cannot be changed
Mellanox · Oct 1, 2024 · 8cbc88b · 8cbc88b
2 parents aac12d1 + 82e0934
commit 8cbc88b
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 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{}
+
+			_, 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 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{
+				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() {