From 36b73a6e08b8c36028885911534bcd3051661bab Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Mon, 28 Oct 2024 17:10:05 +0100 Subject: [PATCH] fix: allow to use string aliases for raw nv config values Now, we parse out the string aliases for nv config parameters' values, which results in a case where a user can provide a valid config that can get applied but will never be seen as successfully applied, resulting in an endless return loop e.g. Configurations: Default Current Next Boot KEEP_IB_LINK_UP_P2 False(0) True(1) True(1) KEEP_LINK_UP_ON_BOOT_P2 False(0) False(0) False(0) MEMIC_SIZE_LIMIT _256KB(1) _256KB(1) DISABLED(0) The fix is to keep both numeric values and string aliases and look for a match among them. Signed-off-by: Alexander Maslennikov --- pkg/host/configvalidation.go | 40 +-- pkg/host/configvalidation_test.go | 164 +++++++----- pkg/host/host.go | 22 +- pkg/host/host_test.go | 236 ++++++++++++------ ...onfigValidation.go => ConfigValidation.go} | 34 +-- pkg/host/mocks/HostManager.go | 2 +- pkg/host/mocks/HostUtils.go | 2 +- pkg/host/utils.go | 28 ++- pkg/host/utils_test.go | 26 +- pkg/types/types.go | 13 +- 10 files changed, 353 insertions(+), 214 deletions(-) rename pkg/host/mocks/{configValidation.go => ConfigValidation.go} (78%) diff --git a/pkg/host/configvalidation.go b/pkg/host/configvalidation.go index 13ff940..864eb01 100644 --- a/pkg/host/configvalidation.go +++ b/pkg/host/configvalidation.go @@ -18,6 +18,7 @@ package host import ( "fmt" "reflect" + "slices" "strconv" "strings" @@ -32,14 +33,14 @@ type configValidation interface { // ConstructNvParamMapFromTemplate translates a configuration template into a set of nvconfig parameters // operates under the assumption that spec validation was already carried out ConstructNvParamMapFromTemplate( - device *v1alpha1.NicDevice, defaultValues map[string]string) (map[string]string, error) + device *v1alpha1.NicDevice, nvConfigQuery types.NvConfigQuery) (map[string]string, error) // ValidateResetToDefault checks if device's nv config has been reset to default in current and next boots // returns bool - need to perform reset // returns bool - reboot required // returns error - if an error occurred during validation ValidateResetToDefault(nvConfig types.NvConfigQuery) (bool, bool, error) // AdvancedPCISettingsEnabled returns true if ADVANCED_PCI_SETTINGS param is enabled for current config - AdvancedPCISettingsEnabled(currentConfig map[string]string) bool + AdvancedPCISettingsEnabled(nvConfig types.NvConfigQuery) bool // RuntimeConfigApplied checks if desired runtime config is applied RuntimeConfigApplied(device *v1alpha1.NicDevice) (bool, error) // CalculateDesiredRuntimeConfig returns desired values for runtime config @@ -64,18 +65,19 @@ func nvParamLinkTypeFromName(linkType string) string { } func applyDefaultNvConfigValueIfExists( - paramName string, desiredParameters map[string]string, defaultValues map[string]string) { - defaultValue, found := defaultValues[paramName] + paramName string, desiredParameters map[string]string, query types.NvConfigQuery) { + defaultValues, found := query.DefaultConfig[paramName] // Default values might not yet be available if ENABLE_PCI_OPTIMIZATIONS is disabled if found { - desiredParameters[paramName] = defaultValue + // Take the default numeric value + desiredParameters[paramName] = defaultValues[len(defaultValues)-1] } } // ConstructNvParamMapFromTemplate translates a configuration template into a set of nvconfig parameters // operates under the assumption that spec validation was already carried out func (v *configValidationImpl) ConstructNvParamMapFromTemplate( - device *v1alpha1.NicDevice, defaultValues map[string]string) (map[string]string, error) { + device *v1alpha1.NicDevice, query types.NvConfigQuery) (map[string]string, error) { desiredParameters := map[string]string{} template := device.Spec.Configuration.Template @@ -90,7 +92,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate( } // Link type change is not allowed on some devices - _, canChangeLinkType := defaultValues[consts.LinkTypeP1Param] + _, canChangeLinkType := query.DefaultConfig[consts.LinkTypeP1Param] if canChangeLinkType { linkType := nvParamLinkTypeFromName(string(template.LinkType)) desiredParameters[consts.LinkTypeP1Param] = linkType @@ -131,7 +133,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate( // maxReadRequest is applied as runtime configuration } else { - applyDefaultNvConfigValueIfExists(consts.MaxAccOutReadParam, desiredParameters, defaultValues) + applyDefaultNvConfigValueIfExists(consts.MaxAccOutReadParam, desiredParameters, query) } if template.RoceOptimized != nil && template.RoceOptimized.Enabled { @@ -154,13 +156,13 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate( // qos settings are applied as runtime configuration } else { - applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP1Param, desiredParameters, defaultValues) - applyDefaultNvConfigValueIfExists(consts.CnpDscpP1Param, desiredParameters, defaultValues) - applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP1Param, desiredParameters, defaultValues) + applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP1Param, desiredParameters, query) + applyDefaultNvConfigValueIfExists(consts.CnpDscpP1Param, desiredParameters, query) + applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP1Param, desiredParameters, query) if secondPortPresent { - applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP2Param, desiredParameters, defaultValues) - applyDefaultNvConfigValueIfExists(consts.CnpDscpP2Param, desiredParameters, defaultValues) - applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP2Param, desiredParameters, defaultValues) + applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP2Param, desiredParameters, query) + applyDefaultNvConfigValueIfExists(consts.CnpDscpP2Param, desiredParameters, query) + applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP2Param, desiredParameters, query) } } @@ -179,7 +181,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate( return desiredParameters, err } } else { - applyDefaultNvConfigValueIfExists(consts.AtsEnabledParam, desiredParameters, defaultValues) + applyDefaultNvConfigValueIfExists(consts.AtsEnabledParam, desiredParameters, query) } for _, rawParam := range template.RawNvConfig { @@ -207,7 +209,7 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue willResetInNextBoot := false advancedPciSettingsEnabledInCurrentConfig := false - if value, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue { + if values, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) { advancedPciSettingsEnabledInCurrentConfig = true } if advancedPciSettingsEnabledInCurrentConfig { @@ -218,7 +220,7 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue } advancedPciSettingsEnabledInNextBootConfig := false - if value, found := nvConfig.NextBootConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue { + if values, found := nvConfig.NextBootConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) { advancedPciSettingsEnabledInNextBootConfig = true } if advancedPciSettingsEnabledInNextBootConfig { @@ -241,8 +243,8 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue } // AdvancedPCISettingsEnabled returns true if ADVANCED_PCI_SETTINGS param is enabled for current config -func (v *configValidationImpl) AdvancedPCISettingsEnabled(currentConfig map[string]string) bool { - if value, found := currentConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue { +func (v *configValidationImpl) AdvancedPCISettingsEnabled(nvConfig types.NvConfigQuery) bool { + if values, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) { return true } return false diff --git a/pkg/host/configvalidation_test.go b/pkg/host/configvalidation_test.go index 193bf7c..170badf 100644 --- a/pkg/host/configvalidation_test.go +++ b/pkg/host/configvalidation_test.go @@ -60,18 +60,21 @@ var _ = Describe("ConfigValidationImpl", func() { }, }, } - defaultValues := map[string]string{ - consts.MaxAccOutReadParam: "testMaxAccOutRead", - consts.RoceCcPrioMaskP1Param: "testRoceCcP1", - consts.CnpDscpP1Param: "testDscpP1", - consts.Cnp802pPrioP1Param: "test802PrioP1", - consts.RoceCcPrioMaskP2Param: "testRoceCcP2", - consts.CnpDscpP2Param: "testDscpP2", - consts.Cnp802pPrioP2Param: "test802PrioP2", - consts.AtsEnabledParam: "testAts", + + defaultValues := map[string][]string{ + consts.MaxAccOutReadParam: {"testMaxAccOutRead"}, + consts.RoceCcPrioMaskP1Param: {"testRoceCcP1"}, + consts.CnpDscpP1Param: {"testDscpP1"}, + consts.Cnp802pPrioP1Param: {"test802PrioP1"}, + consts.RoceCcPrioMaskP2Param: {"testRoceCcP2"}, + consts.CnpDscpP2Param: {"testDscpP2"}, + consts.Cnp802pPrioP2Param: {"test802PrioP2"}, + consts.AtsEnabledParam: {"testAts"}, } + query := types.NewNvConfigQuery() + query.DefaultConfig = defaultValues - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue(consts.SriovEnabledParam, consts.NvParamFalse)) Expect(nvParams).To(HaveKeyWithValue(consts.SriovNumOfVfsParam, "0")) @@ -101,18 +104,20 @@ var _ = Describe("ConfigValidationImpl", func() { }, }, } - defaultValues := map[string]string{ - consts.MaxAccOutReadParam: "testMaxAccOutRead", - consts.RoceCcPrioMaskP1Param: "testRoceCcP1", - consts.CnpDscpP1Param: "testDscpP1", - consts.Cnp802pPrioP1Param: "test802PrioP1", - consts.RoceCcPrioMaskP2Param: "testRoceCcP2", - consts.CnpDscpP2Param: "testDscpP2", - consts.Cnp802pPrioP2Param: "test802PrioP2", - consts.AtsEnabledParam: "testAts", + defaultValues := map[string][]string{ + consts.MaxAccOutReadParam: {"testMaxAccOutRead"}, + consts.RoceCcPrioMaskP1Param: {"testRoceCcP1"}, + consts.CnpDscpP1Param: {"testDscpP1"}, + consts.Cnp802pPrioP1Param: {"test802PrioP1"}, + consts.RoceCcPrioMaskP2Param: {"testRoceCcP2"}, + consts.CnpDscpP2Param: {"testDscpP2"}, + consts.Cnp802pPrioP2Param: {"test802PrioP2"}, + consts.AtsEnabledParam: {"testAts"}, } + query := types.NewNvConfigQuery() + query.DefaultConfig = defaultValues - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue(consts.SriovEnabledParam, consts.NvParamFalse)) Expect(nvParams).To(HaveKeyWithValue(consts.SriovNumOfVfsParam, "0")) @@ -162,9 +167,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue(consts.MaxAccOutReadParam, "1337")) Expect(nvParams).To(HaveKeyWithValue(consts.AtsEnabledParam, "0")) @@ -198,9 +203,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue(consts.MaxAccOutReadParam, "44")) }) @@ -227,9 +232,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue(consts.MaxAccOutReadParam, "0")) }) @@ -256,9 +261,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - _, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + _, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).To(MatchError("incorrect spec: GpuDirectOptimized should only be enabled together with PciPerformanceOptimized")) }) It("should ignore raw config for the second port if device is single port", func() { @@ -290,9 +295,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue("TEST_P1", "test")) Expect(nvParams).NotTo(HaveKey("TEST_P2")) @@ -327,9 +332,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - nvParams, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) Expect(nvParams).To(HaveKeyWithValue("TEST_P1", "test")) Expect(nvParams).To(HaveKeyWithValue("TEST_P2", "test")) @@ -364,9 +369,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - _, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + _, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).To(MatchError("incorrect spec: device does not 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() { @@ -399,11 +404,13 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{ - consts.LinkTypeP1Param: consts.Ethernet, + defaultValues := map[string][]string{ + consts.LinkTypeP1Param: {consts.Ethernet}, } + query := types.NewNvConfigQuery() + query.DefaultConfig = defaultValues - _, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + _, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) }) It("should not report an error when LinkType cannot be changed and template matches the actual status", func() { @@ -436,9 +443,9 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - _, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + _, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).NotTo(HaveOccurred()) }) It("should return an error when RoceOptimized is enabled with linkType Infiniband", func() { @@ -463,22 +470,67 @@ var _ = Describe("ConfigValidationImpl", func() { }, } - defaultValues := map[string]string{} + query := types.NewNvConfigQuery() - _, err := validator.ConstructNvParamMapFromTemplate(device, defaultValues) + _, err := validator.ConstructNvParamMapFromTemplate(device, query) Expect(err).To(MatchError("incorrect spec: RoceOptimized settings can only be used with link type Ethernet")) }) + + It("should take numeric values when both numeric values and string aliases are present in nv config query", func() { + device := &v1alpha1.NicDevice{ + Spec: v1alpha1.NicDeviceSpec{ + Configuration: &v1alpha1.NicDeviceConfigurationSpec{ + Template: &v1alpha1.ConfigurationTemplateSpec{ + NumVfs: 0, + LinkType: consts.Ethernet, + }, + }, + }, + Status: v1alpha1.NicDeviceStatus{ + Ports: []v1alpha1.NicDevicePortSpec{ + {PCI: "0000:03:00.0"}, + {PCI: "0000:03:00.1"}, + }, + }, + } + + defaultValues := map[string][]string{ + consts.MaxAccOutReadParam: {"testMaxAccOutRead", "1"}, + consts.RoceCcPrioMaskP1Param: {"testRoceCcP1", "2"}, + consts.CnpDscpP1Param: {"testDscpP1", "3"}, + consts.Cnp802pPrioP1Param: {"test802PrioP1", "4"}, + consts.RoceCcPrioMaskP2Param: {"testRoceCcP2", "5"}, + consts.CnpDscpP2Param: {"testDscpP2", "6"}, + consts.Cnp802pPrioP2Param: {"test802PrioP2", "7"}, + consts.AtsEnabledParam: {"testAts", "8"}, + } + query := types.NewNvConfigQuery() + query.DefaultConfig = defaultValues + + nvParams, err := validator.ConstructNvParamMapFromTemplate(device, query) + Expect(err).NotTo(HaveOccurred()) + Expect(nvParams).To(HaveKeyWithValue(consts.SriovEnabledParam, consts.NvParamFalse)) + Expect(nvParams).To(HaveKeyWithValue(consts.SriovNumOfVfsParam, "0")) + Expect(nvParams).To(HaveKeyWithValue(consts.MaxAccOutReadParam, "1")) + Expect(nvParams).To(HaveKeyWithValue(consts.RoceCcPrioMaskP1Param, "2")) + Expect(nvParams).To(HaveKeyWithValue(consts.CnpDscpP1Param, "3")) + Expect(nvParams).To(HaveKeyWithValue(consts.Cnp802pPrioP1Param, "4")) + Expect(nvParams).To(HaveKeyWithValue(consts.RoceCcPrioMaskP2Param, "5")) + Expect(nvParams).To(HaveKeyWithValue(consts.CnpDscpP2Param, "6")) + Expect(nvParams).To(HaveKeyWithValue(consts.Cnp802pPrioP2Param, "7")) + Expect(nvParams).To(HaveKeyWithValue(consts.AtsEnabledParam, "8")) + }) }) Describe("ValidateResetToDefault", func() { It("should return false, false if device is already reset in current and next boot", func() { nvConfigQuery := types.NewNvConfigQuery() - nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue - nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue + nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} + nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} - nvConfigQuery.DefaultConfig["RandomParam"] = testVal - nvConfigQuery.CurrentConfig["RandomParam"] = testVal - nvConfigQuery.NextBootConfig["RandomParam"] = testVal + nvConfigQuery.DefaultConfig["RandomParam"] = []string{testVal} + nvConfigQuery.CurrentConfig["RandomParam"] = []string{testVal} + nvConfigQuery.NextBootConfig["RandomParam"] = []string{testVal} nvConfigChangeRequired, rebootRequired, err := validator.ValidateResetToDefault(nvConfigQuery) Expect(nvConfigChangeRequired).To(Equal(false)) @@ -488,12 +540,12 @@ var _ = Describe("ConfigValidationImpl", func() { It("should return false, true if reset will complete after reboot", func() { nvConfigQuery := types.NewNvConfigQuery() - nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue - nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue + nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} + nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} - nvConfigQuery.DefaultConfig["RandomParam"] = testVal - nvConfigQuery.CurrentConfig["RandomParam"] = anotherTestVal - nvConfigQuery.NextBootConfig["RandomParam"] = testVal + nvConfigQuery.DefaultConfig["RandomParam"] = []string{testVal} + nvConfigQuery.CurrentConfig["RandomParam"] = []string{anotherTestVal} + nvConfigQuery.NextBootConfig["RandomParam"] = []string{testVal} nvConfigChangeRequired, rebootRequired, err := validator.ValidateResetToDefault(nvConfigQuery) Expect(nvConfigChangeRequired).To(Equal(false)) @@ -503,12 +555,12 @@ var _ = Describe("ConfigValidationImpl", func() { It("should return true, true if reset is required", func() { nvConfigQuery := types.NewNvConfigQuery() - nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue - nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = consts.NvParamTrue + nvConfigQuery.CurrentConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} + nvConfigQuery.NextBootConfig[consts.AdvancedPCISettingsParam] = []string{consts.NvParamTrue} - nvConfigQuery.DefaultConfig["RandomParam"] = testVal - nvConfigQuery.CurrentConfig["RandomParam"] = anotherTestVal - nvConfigQuery.NextBootConfig["RandomParam"] = anotherTestVal + nvConfigQuery.DefaultConfig["RandomParam"] = []string{testVal} + nvConfigQuery.CurrentConfig["RandomParam"] = []string{anotherTestVal} + nvConfigQuery.NextBootConfig["RandomParam"] = []string{anotherTestVal} nvConfigChangeRequired, rebootRequired, err := validator.ValidateResetToDefault(nvConfigQuery) Expect(nvConfigChangeRequired).To(Equal(true)) diff --git a/pkg/host/host.go b/pkg/host/host.go index 5b3e871..231d4be 100644 --- a/pkg/host/host.go +++ b/pkg/host/host.go @@ -18,7 +18,9 @@ package host import ( "context" "fmt" + "slices" "strconv" + "strings" "github.com/Mellanox/nic-configuration-operator/pkg/types" "sigs.k8s.io/controller-runtime/pkg/log" @@ -155,7 +157,7 @@ func (h hostManager) ValidateDeviceNvSpec(ctx context.Context, device *v1alpha1. return h.configValidation.ValidateResetToDefault(nvConfig) } - desiredConfig, err := h.configValidation.ConstructNvParamMapFromTemplate(device, nvConfig.DefaultConfig) + desiredConfig, err := h.configValidation.ConstructNvParamMapFromTemplate(device, nvConfig) if err != nil { log.Log.Error(err, "failed to calculate desired nvconfig parameters", "device", device.Name) return false, false, err @@ -165,19 +167,19 @@ func (h hostManager) ValidateDeviceNvSpec(ctx context.Context, device *v1alpha1. rebootNeeded := false // If ADVANCED_PCI_SETTINGS are enabled in current config, unknown parameters are treated as spec error - advancedPciSettingsEnabled := h.configValidation.AdvancedPCISettingsEnabled(nvConfig.CurrentConfig) + advancedPciSettingsEnabled := h.configValidation.AdvancedPCISettingsEnabled(nvConfig) for parameter, desiredValue := range desiredConfig { - currentValue, foundInCurrent := nvConfig.CurrentConfig[parameter] - nextValue, foundInNextBoot := nvConfig.NextBootConfig[parameter] + currentValues, foundInCurrent := nvConfig.CurrentConfig[parameter] + nextValues, foundInNextBoot := nvConfig.NextBootConfig[parameter] if advancedPciSettingsEnabled && !foundInCurrent { err = types.IncorrectSpecError(fmt.Sprintf("Parameter %s unsupported for device %s", parameter, device.Name)) log.Log.Error(err, "can't set nv config parameter for device") return false, false, err } - if foundInNextBoot && nextValue == desiredValue { - if !foundInCurrent || currentValue != desiredValue { + if foundInNextBoot && slices.Contains(nextValues, strings.ToLower(desiredValue)) { + if !foundInCurrent || !slices.Contains(currentValues, strings.ToLower(desiredValue)) { rebootNeeded = true } } else { @@ -220,7 +222,7 @@ func (h hostManager) ApplyDeviceNvSpec(ctx context.Context, device *v1alpha1.Nic return false, err } - if !h.configValidation.AdvancedPCISettingsEnabled(nvConfig.CurrentConfig) { + if !h.configValidation.AdvancedPCISettingsEnabled(nvConfig) { log.Log.V(2).Info("AdvancedPciSettings not enabled, fw reset required", "device", device.Name) err = h.hostUtils.SetNvConfigParameter(pciAddr, consts.AdvancedPCISettingsParam, consts.NvParamTrue) if err != nil { @@ -242,7 +244,7 @@ func (h hostManager) ApplyDeviceNvSpec(ctx context.Context, device *v1alpha1.Nic } } - desiredConfig, err := h.configValidation.ConstructNvParamMapFromTemplate(device, nvConfig.DefaultConfig) + desiredConfig, err := h.configValidation.ConstructNvParamMapFromTemplate(device, nvConfig) if err != nil { log.Log.Error(err, "failed to calculate desired nvconfig parameters", "device", device.Name) return false, err @@ -251,14 +253,14 @@ func (h hostManager) ApplyDeviceNvSpec(ctx context.Context, device *v1alpha1.Nic paramsToApply := map[string]string{} for param, value := range desiredConfig { - nextVal, found := nvConfig.NextBootConfig[param] + nextValues, found := nvConfig.NextBootConfig[param] if !found { err = types.IncorrectSpecError(fmt.Sprintf("Parameter %s unsupported for device %s", param, device.Name)) log.Log.Error(err, "can't set nv config parameter for device") return false, err } - if nextVal != value { + if !slices.Contains(nextValues, value) { paramsToApply[param] = value } } diff --git a/pkg/host/host_test.go b/pkg/host/host_test.go index cf33d29..ab83a97 100644 --- a/pkg/host/host_test.go +++ b/pkg/host/host_test.go @@ -463,9 +463,9 @@ var _ = Describe("HostManager", func() { It("should call ValidateResetToDefault and return its results", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } mockHostUtils.On("QueryNvConfig", ctx, pciAddress). @@ -484,9 +484,9 @@ var _ = Describe("HostManager", func() { It("should return an error if ValidateResetToDefault fails", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{}, - NextBootConfig: map[string]string{}, - DefaultConfig: map[string]string{}, + CurrentConfig: map[string][]string{}, + NextBootConfig: map[string][]string{}, + DefaultConfig: map[string][]string{}, } validationErr := errors.New("validation failed") @@ -508,15 +508,15 @@ var _ = Describe("HostManager", func() { Context("when ConstructNvParamMapFromTemplate returns an error", func() { It("should return false, false, and the error", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{}, - NextBootConfig: map[string]string{}, - DefaultConfig: map[string]string{}, + CurrentConfig: map[string][]string{}, + NextBootConfig: map[string][]string{}, + DefaultConfig: map[string][]string{}, } constructErr := errors.New("failed to construct desired config") mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(nil, constructErr) configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) @@ -533,17 +533,17 @@ var _ = Describe("HostManager", func() { Context("when desiredConfig fully matches current and next config", func() { It("should return false, false, nil", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1", "param2": "value2"}, - NextBootConfig: map[string]string{"param1": "value1", "param2": "value2"}, - DefaultConfig: map[string]string{"param1": "default1", "param2": "default2"}, + CurrentConfig: map[string][]string{"param1": {"value1"}, "param2": {"value2"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}, "param2": {"value2"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}, "param2": {"default2"}}, } desiredConfig := map[string]string{"param1": "value1", "param2": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) @@ -560,17 +560,17 @@ var _ = Describe("HostManager", func() { Context("when desiredConfig fully matches next but not current config", func() { It("should return false, true, nil", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "oldValue1", "param2": "value2"}, - NextBootConfig: map[string]string{"param1": "value1", "param2": "value2"}, - DefaultConfig: map[string]string{"param1": "default1", "param2": "default2"}, + CurrentConfig: map[string][]string{"param1": {"oldValue1"}, "param2": {"value2"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}, "param2": {"value2"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}, "param2": {"default2"}}, } desiredConfig := map[string]string{"param1": "value1", "param2": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) @@ -587,17 +587,17 @@ var _ = Describe("HostManager", func() { Context("when desiredConfig does not fully match next boot config", func() { It("should return true, true, nil", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "oldValue1", "param2": "value2"}, - NextBootConfig: map[string]string{"param1": "wrongValue", "param2": "value2"}, - DefaultConfig: map[string]string{"param1": "default1", "param2": "default2"}, + CurrentConfig: map[string][]string{"param1": {"oldValue1"}, "param2": {"value2"}}, + NextBootConfig: map[string][]string{"param1": {"wrongValue"}, "param2": {"value2"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}, "param2": {"default2"}}, } desiredConfig := map[string]string{"param1": "value1", "param2": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) @@ -613,17 +613,17 @@ var _ = Describe("HostManager", func() { Context("when AdvancedPCISettingsEnabled is true and a parameter is missing in CurrentConfig", func() { It("should return an IncorrectSpecError", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param2": "value2"}, - NextBootConfig: map[string]string{"param1": "value1", "param2": "value2"}, - DefaultConfig: map[string]string{"param1": "default1", "param2": "default2"}, + CurrentConfig: map[string][]string{"param2": {"value2"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}, "param2": {"value2"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}, "param2": {"default2"}}, } desiredConfig := map[string]string{"param1": "value1", "param2": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) expectedErr := types.IncorrectSpecError( @@ -638,6 +638,78 @@ var _ = Describe("HostManager", func() { mockConfigValidation.AssertExpectations(GinkgoT()) }) }) + + Context("when desired config contains string aliases", func() { + It("should accept lowercase parameters", func() { + nvConfig := types.NvConfigQuery{ + CurrentConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + NextBootConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + DefaultConfig: map[string][]string{"param1": {"default1", "1"}, "param2": {"default2", "2"}}, + } + desiredConfig := map[string]string{"param1": "value1", "param2": "2"} + + mockHostUtils.On("QueryNvConfig", ctx, pciAddress). + Return(nvConfig, nil) + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). + Return(desiredConfig, nil) + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). + Return(true) + + configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) + Expect(configUpdate).To(BeFalse()) + Expect(reboot).To(BeFalse()) + Expect(err).To(BeNil()) + + mockHostUtils.AssertExpectations(GinkgoT()) + mockConfigValidation.AssertExpectations(GinkgoT()) + }) + It("should accept mixed-case parameters", func() { + nvConfig := types.NvConfigQuery{ + CurrentConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + NextBootConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + DefaultConfig: map[string][]string{"param1": {"default1", "1"}, "param2": {"default2", "2"}}, + } + desiredConfig := map[string]string{"param1": "VaLuE1", "param2": "valUE2"} + + mockHostUtils.On("QueryNvConfig", ctx, pciAddress). + Return(nvConfig, nil) + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). + Return(desiredConfig, nil) + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). + Return(true) + + configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) + Expect(configUpdate).To(BeFalse()) + Expect(reboot).To(BeFalse()) + Expect(err).To(BeNil()) + + mockHostUtils.AssertExpectations(GinkgoT()) + mockConfigValidation.AssertExpectations(GinkgoT()) + }) + It("should process not matching parameters", func() { + nvConfig := types.NvConfigQuery{ + CurrentConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + NextBootConfig: map[string][]string{"param1": {"value1", "1"}, "param2": {"value2", "2"}}, + DefaultConfig: map[string][]string{"param1": {"default1", "1"}, "param2": {"default2", "2"}}, + } + desiredConfig := map[string]string{"param1": "value3", "param2": "val4"} + + mockHostUtils.On("QueryNvConfig", ctx, pciAddress). + Return(nvConfig, nil) + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). + Return(desiredConfig, nil) + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). + Return(true) + + configUpdate, reboot, err := manager.ValidateDeviceNvSpec(ctx, device) + Expect(configUpdate).To(BeTrue()) + Expect(reboot).To(BeTrue()) + Expect(err).To(BeNil()) + + mockHostUtils.AssertExpectations(GinkgoT()) + mockConfigValidation.AssertExpectations(GinkgoT()) + }) + }) }) }) Describe("hostManager.ApplyDeviceNvSpec", func() { @@ -736,15 +808,15 @@ var _ = Describe("HostManager", func() { Context("when AdvancedPCISettingsEnabled is false", func() { It("should set AdvancedPCISettingsParam and reset NIC firmware successfully", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value1"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) mockHostUtils. On("SetNvConfigParameter", pciAddress, consts.AdvancedPCISettingsParam, consts.NvParamTrue). @@ -753,7 +825,7 @@ var _ = Describe("HostManager", func() { Return(nil) mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) reboot, err := manager.ApplyDeviceNvSpec(ctx, device) @@ -766,12 +838,12 @@ var _ = Describe("HostManager", func() { It("should return error if SetNvConfigParameter fails", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } mockHostUtils.On("QueryNvConfig", ctx, pciAddress).Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) setParamErr := errors.New("failed to set nv config parameter") mockHostUtils. @@ -788,12 +860,12 @@ var _ = Describe("HostManager", func() { It("should return error if ResetNicFirmware fails", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } mockHostUtils.On("QueryNvConfig", ctx, pciAddress).Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) mockHostUtils.On("SetNvConfigParameter", pciAddress, consts.AdvancedPCISettingsParam, consts.NvParamTrue). Return(nil) @@ -810,14 +882,14 @@ var _ = Describe("HostManager", func() { It("should return error if second QueryNvConfig fails", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil).Times(1) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(false) mockHostUtils.On("SetNvConfigParameter", pciAddress, consts.AdvancedPCISettingsParam, consts.NvParamTrue). Return(nil) @@ -839,17 +911,17 @@ var _ = Describe("HostManager", func() { Context("when AdvancedPCISettingsEnabled is true", func() { It("should construct desiredConfig and apply no changes if desiredConfig matches NextBootConfig", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value1"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) reboot, err := manager.ApplyDeviceNvSpec(ctx, device) @@ -862,17 +934,17 @@ var _ = Describe("HostManager", func() { It("should construct desiredConfig and apply necessary changes successfully", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "oldValue1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) mockHostUtils.On("SetNvConfigParameter", pciAddress, "param1", "value2"). Return(nil) @@ -887,17 +959,17 @@ var _ = Describe("HostManager", func() { It("should return error if ConstructNvParamMapFromTemplate fails", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } constructErr := errors.New("failed to construct desired config") mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(nil, constructErr) reboot, err := manager.ApplyDeviceNvSpec(ctx, device) @@ -910,17 +982,17 @@ var _ = Describe("HostManager", func() { It("should return error if desiredConfig has a parameter not in NextBootConfig", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value1", "param2": "value2"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) expectedErr := types.IncorrectSpecError( @@ -936,17 +1008,17 @@ var _ = Describe("HostManager", func() { It("should return error if SetNvConfigParameter fails while applying params", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "oldValue1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value3"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) setParamErr := errors.New("failed to set param1") mockHostUtils.On("SetNvConfigParameter", pciAddress, "param1", "value3"). @@ -965,17 +1037,17 @@ var _ = Describe("HostManager", func() { Context("when applying multiple parameters", func() { It("should apply all parameters successfully and require a reboot", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "oldValue1", "param2": "oldValue2"}, - NextBootConfig: map[string]string{"param1": "newValue1", "param2": "newValue2"}, - DefaultConfig: map[string]string{"param1": "default1", "param2": "default2"}, + CurrentConfig: map[string][]string{"param1": {"oldValue1"}, "param2": {"oldValue2"}}, + NextBootConfig: map[string][]string{"param1": {"newValue1"}, "param2": {"newValue2"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}, "param2": {"default2"}}, } desiredConfig := map[string]string{"param1": "newValue3", "param2": "newValue3"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) mockHostUtils.On("SetNvConfigParameter", pciAddress, "param1", "newValue3"). Return(nil) @@ -994,17 +1066,17 @@ var _ = Describe("HostManager", func() { Context("when no parameters need to be applied", func() { It("should return true without applying any parameters", func() { nvConfig := types.NvConfigQuery{ - CurrentConfig: map[string]string{"param1": "value1"}, - NextBootConfig: map[string]string{"param1": "value1"}, - DefaultConfig: map[string]string{"param1": "default1"}, + CurrentConfig: map[string][]string{"param1": {"value1"}}, + NextBootConfig: map[string][]string{"param1": {"value1"}}, + DefaultConfig: map[string][]string{"param1": {"default1"}}, } desiredConfig := map[string]string{"param1": "value1"} mockHostUtils.On("QueryNvConfig", ctx, pciAddress). Return(nvConfig, nil) - mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig.CurrentConfig). + mockConfigValidation.On("AdvancedPCISettingsEnabled", nvConfig). Return(true) - mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig.DefaultConfig). + mockConfigValidation.On("ConstructNvParamMapFromTemplate", device, nvConfig). Return(desiredConfig, nil) reboot, err := manager.ApplyDeviceNvSpec(ctx, device) diff --git a/pkg/host/mocks/configValidation.go b/pkg/host/mocks/ConfigValidation.go similarity index 78% rename from pkg/host/mocks/configValidation.go rename to pkg/host/mocks/ConfigValidation.go index 1132ac3..f683e4d 100644 --- a/pkg/host/mocks/configValidation.go +++ b/pkg/host/mocks/ConfigValidation.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.45.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks @@ -8,22 +8,22 @@ import ( mock "github.com/stretchr/testify/mock" ) -// configValidation is an autogenerated mock type for the configValidation type +// ConfigValidation is an autogenerated mock type for the ConfigValidation type type ConfigValidation struct { mock.Mock } -// AdvancedPCISettingsEnabled provides a mock function with given fields: currentConfig -func (_m *ConfigValidation) AdvancedPCISettingsEnabled(currentConfig map[string]string) bool { - ret := _m.Called(currentConfig) +// AdvancedPCISettingsEnabled provides a mock function with given fields: nvConfig +func (_m *ConfigValidation) AdvancedPCISettingsEnabled(nvConfig types.NvConfigQuery) bool { + ret := _m.Called(nvConfig) if len(ret) == 0 { panic("no return value specified for AdvancedPCISettingsEnabled") } var r0 bool - if rf, ok := ret.Get(0).(func(map[string]string) bool); ok { - r0 = rf(currentConfig) + if rf, ok := ret.Get(0).(func(types.NvConfigQuery) bool); ok { + r0 = rf(nvConfig) } else { r0 = ret.Get(0).(bool) } @@ -66,9 +66,9 @@ func (_m *ConfigValidation) CalculateDesiredRuntimeConfig(device *v1alpha1.NicDe return r0, r1, r2 } -// ConstructNvParamMapFromTemplate provides a mock function with given fields: device, defaultValues -func (_m *ConfigValidation) ConstructNvParamMapFromTemplate(device *v1alpha1.NicDevice, defaultValues map[string]string) (map[string]string, error) { - ret := _m.Called(device, defaultValues) +// ConstructNvParamMapFromTemplate provides a mock function with given fields: device, nvConfigQuery +func (_m *ConfigValidation) ConstructNvParamMapFromTemplate(device *v1alpha1.NicDevice, nvConfigQuery types.NvConfigQuery) (map[string]string, error) { + ret := _m.Called(device, nvConfigQuery) if len(ret) == 0 { panic("no return value specified for ConstructNvParamMapFromTemplate") @@ -76,19 +76,19 @@ func (_m *ConfigValidation) ConstructNvParamMapFromTemplate(device *v1alpha1.Nic var r0 map[string]string var r1 error - if rf, ok := ret.Get(0).(func(*v1alpha1.NicDevice, map[string]string) (map[string]string, error)); ok { - return rf(device, defaultValues) + if rf, ok := ret.Get(0).(func(*v1alpha1.NicDevice, types.NvConfigQuery) (map[string]string, error)); ok { + return rf(device, nvConfigQuery) } - if rf, ok := ret.Get(0).(func(*v1alpha1.NicDevice, map[string]string) map[string]string); ok { - r0 = rf(device, defaultValues) + if rf, ok := ret.Get(0).(func(*v1alpha1.NicDevice, types.NvConfigQuery) map[string]string); ok { + r0 = rf(device, nvConfigQuery) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(map[string]string) } } - if rf, ok := ret.Get(1).(func(*v1alpha1.NicDevice, map[string]string) error); ok { - r1 = rf(device, defaultValues) + if rf, ok := ret.Get(1).(func(*v1alpha1.NicDevice, types.NvConfigQuery) error); ok { + r1 = rf(device, nvConfigQuery) } else { r1 = ret.Error(1) } @@ -159,7 +159,7 @@ func (_m *ConfigValidation) ValidateResetToDefault(nvConfig types.NvConfigQuery) return r0, r1, r2 } -// newConfigValidation creates a new instance of configValidation. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// newConfigValidation creates a new instance of ConfigValidation. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func newConfigValidation(t interface { mock.TestingT diff --git a/pkg/host/mocks/HostManager.go b/pkg/host/mocks/HostManager.go index eaa5409..d585850 100644 --- a/pkg/host/mocks/HostManager.go +++ b/pkg/host/mocks/HostManager.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.45.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/pkg/host/mocks/HostUtils.go b/pkg/host/mocks/HostUtils.go index 323ab45..9dde55e 100644 --- a/pkg/host/mocks/HostUtils.go +++ b/pkg/host/mocks/HostUtils.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.45.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/pkg/host/utils.go b/pkg/host/utils.go index 6770b85..58813ac 100644 --- a/pkg/host/utils.go +++ b/pkg/host/utils.go @@ -375,7 +375,7 @@ func (h *hostUtils) GetRDMADeviceName(pciAddr string) string { // might run recursively to expand array parameters' values func (h *hostUtils) queryMSTConfig(ctx context.Context, query types.NvConfigQuery, pciAddr string, additionalParameter string) error { log.Log.Info(fmt.Sprintf("mstconfig -d %s query %s", pciAddr, additionalParameter)) //TODO change verbosity - valueInBracketsRegex := regexp.MustCompile(`\(([^)]*)\)$`) + valueInBracketsRegex := regexp.MustCompile(`^(.*?)\(([^)]*)\)$`) var cmd execUtils.Cmd if additionalParameter == "" { @@ -446,21 +446,31 @@ func (h *hostUtils) queryMSTConfig(ctx context.Context, query types.NvConfigQuer continue } - // If the actual value is wrapped in brackets, we want to extract it + // If the actual value is wrapped in brackets, we want to save both it and its string alias match := valueInBracketsRegex.FindStringSubmatch(defaultVal) - if len(match) == 2 { - defaultVal = match[1] + if len(match) == 3 { + for i, v := range match { + match[i] = strings.ToLower(v) + } + query.DefaultConfig[paramName] = match[1:] match = valueInBracketsRegex.FindStringSubmatch(currentVal) - currentVal = match[1] + for i, v := range match { + match[i] = strings.ToLower(v) + } + query.CurrentConfig[paramName] = match[1:] match = valueInBracketsRegex.FindStringSubmatch(nextBootVal) - nextBootVal = match[1] + for i, v := range match { + match[i] = strings.ToLower(v) + } + query.NextBootConfig[paramName] = match[1:] + } else { + query.DefaultConfig[paramName] = []string{defaultVal} + query.CurrentConfig[paramName] = []string{currentVal} + query.NextBootConfig[paramName] = []string{nextBootVal} } - query.DefaultConfig[paramName] = defaultVal - query.CurrentConfig[paramName] = currentVal - query.NextBootConfig[paramName] = nextBootVal } } diff --git a/pkg/host/utils_test.go b/pkg/host/utils_test.go index 5d4ba5a..015cf7a 100644 --- a/pkg/host/utils_test.go +++ b/pkg/host/utils_test.go @@ -323,7 +323,7 @@ Configurations: Default Current Nex * KEEP_IB_LINK_UP_P2 False(0) True(1) True(1) KEEP_LINK_UP_ON_BOOT_P2 False(0) False(0) False(0) ESWITCH_HAIRPIN_DESCRIPTORS Array[0..7] Array[0..7] Array[0..7] -* MEMIC_SIZE_LIMIT _256KB(1) _256KB(1) _256KB(0) +* MEMIC_SIZE_LIMIT _256KB(1) _256KB(1) DISABLED(0) The '*' shows parameters with next value different from default/current value. ` return []byte(output), nil, nil @@ -372,24 +372,24 @@ Configurations: Default Current Nex Expect(err).ToNot(HaveOccurred()) // Verify regular parameters - Expect(query.DefaultConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", "0")) - Expect(query.CurrentConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", "1")) - Expect(query.NextBootConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", "1")) + Expect(query.DefaultConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", []string{"false", "0"})) + Expect(query.CurrentConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", []string{"true", "1"})) + Expect(query.NextBootConfig).To(HaveKeyWithValue("KEEP_IB_LINK_UP_P2", []string{"true", "1"})) - Expect(query.DefaultConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", "1")) - Expect(query.CurrentConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", "1")) - Expect(query.NextBootConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", "0")) + Expect(query.DefaultConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", []string{"_256kb", "1"})) + Expect(query.CurrentConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", []string{"_256kb", "1"})) + Expect(query.NextBootConfig).To(HaveKeyWithValue("MEMIC_SIZE_LIMIT", []string{"disabled", "0"})) - Expect(query.DefaultConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", "0")) - Expect(query.CurrentConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", "0")) - Expect(query.NextBootConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", "0")) + Expect(query.DefaultConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", []string{"false", "0"})) + Expect(query.CurrentConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", []string{"false", "0"})) + Expect(query.NextBootConfig).To(HaveKeyWithValue("KEEP_LINK_UP_ON_BOOT_P2", []string{"false", "0"})) // Verify array parameters for i := 0; i <= 7; i++ { key := fmt.Sprintf("ESWITCH_HAIRPIN_DESCRIPTORS[%d]", i) - Expect(query.DefaultConfig).To(HaveKeyWithValue(key, "128")) - Expect(query.CurrentConfig).To(HaveKeyWithValue(key, "128")) - Expect(query.NextBootConfig).To(HaveKeyWithValue(key, "128")) + Expect(query.DefaultConfig).To(HaveKeyWithValue(key, []string{"128"})) + Expect(query.CurrentConfig).To(HaveKeyWithValue(key, []string{"128"})) + Expect(query.NextBootConfig).To(HaveKeyWithValue(key, []string{"128"})) } }) diff --git a/pkg/types/types.go b/pkg/types/types.go index eb22e2f..a05553e 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -20,17 +20,18 @@ import ( "strings" ) +// NvConfigQuery contains a nv config query for a single device, values can contain both the string alias and the numeric value type NvConfigQuery struct { - DefaultConfig map[string]string - CurrentConfig map[string]string - NextBootConfig map[string]string + DefaultConfig map[string][]string + CurrentConfig map[string][]string + NextBootConfig map[string][]string } func NewNvConfigQuery() NvConfigQuery { return NvConfigQuery{ - DefaultConfig: make(map[string]string), - CurrentConfig: make(map[string]string), - NextBootConfig: make(map[string]string), + DefaultConfig: make(map[string][]string), + CurrentConfig: make(map[string][]string), + NextBootConfig: make(map[string][]string), } }