MGMT-20634: Revert mac-identifier and global DNS workarounds as RHEL-91250 and RHEL-72440 resolved

internal/bminventory/inventory.go

@@ -4148,7 +4148,7 @@ func (b *bareMetalInventory) DownloadMinimalInitrd(ctx context.Context, params i
 	var scriptContent, serviceContent string
 	if infraEnv.StaticNetworkConfig != "" {
 		var shouldUseNmstateService bool
-		shouldUseNmstateService, err = b.staticNetworkConfig.ShouldUseNmstateService(infraEnv.StaticNetworkConfig, infraEnv.OpenshiftVersion)
+		shouldUseNmstateService, err = b.staticNetworkConfig.ShouldUseNmstateService(infraEnv.OpenshiftVersion)
 		if err != nil {
 			return common.GenerateErrorResponder(err)
 		}
@@ -6505,7 +6505,7 @@ func (b *bareMetalInventory) V2DownloadInfraEnvFiles(ctx context.Context, params
 		var netFiles []staticnetworkconfig.StaticNetworkConfigData
 		if infraEnv.StaticNetworkConfig != "" {
 			var shouldUseNmstateService bool
-			shouldUseNmstateService, err = b.staticNetworkConfig.ShouldUseNmstateService(infraEnv.StaticNetworkConfig, infraEnv.OpenshiftVersion)
+			shouldUseNmstateService, err = b.staticNetworkConfig.ShouldUseNmstateService(infraEnv.OpenshiftVersion)
 			if err != nil {
 				return common.GenerateErrorResponder(err)
 			}

internal/bminventory/inventory_test.go

@@ -11767,7 +11767,7 @@ var _ = Describe("DownloadMinimalInitrd", func() {
 			infraEnv.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
 			infraEnv = applyProxy(infraEnv)
 			Expect(db.Create(&infraEnv).Error).NotTo(HaveOccurred())
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticnetworkConfigOutput, nil).Times(1)
 			validateArchiveStatNetFlow(infraEnv, false)
 		})
@@ -11777,7 +11777,7 @@ var _ = Describe("DownloadMinimalInitrd", func() {
 			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv = applyProxy(infraEnv)
 			Expect(db.Create(&infraEnv).Error).NotTo(HaveOccurred())
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigDataYAML(formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			validateArchiveStatNetFlow(infraEnv, true)
 		})
@@ -13539,15 +13539,15 @@ var _ = Describe("V2DownloadInfraEnvFiles", func() {
 		It("old flow involving generating nmconnection files", func() {
 			infraEnvWithStatNet.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
 			Expect(db.Create(&infraEnvWithStatNet).Error).To(Succeed())
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnvWithStatNet.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnvWithStatNet.OpenshiftVersion).Return(false, nil).Times(1)
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticnetworkConfigOutput, nil).Times(1)
 			content := getResponseData("static-network-config", false, nil, "", infraEnvWithStatNetID)
 			Expect(string(content)).To(ContainSubstring("/etc/assisted/network/nic10.nmconnection"))
 		})
 		It("new flow involving nmpolicy and nmstate service", func() {
 			infraEnvWithStatNet.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			Expect(db.Create(&infraEnvWithStatNet).Error).To(Succeed())
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnvWithStatNet.OpenshiftVersion).Return(true, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnvWithStatNet.OpenshiftVersion).Return(true, nil).Times(1)
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigDataYAML(formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			content := getResponseData("static-network-config", false, nil, "", infraEnvWithStatNetID)
 			Expect(string(content)).To(ContainSubstring("/etc/assisted/network/nic10.yml"))

internal/ignition/discovery.go

@@ -346,7 +346,7 @@ func (ib *ignitionBuilder) FormatDiscoveryIgnitionFile(ctx context.Context, infr
 		var filesList []staticnetworkconfig.StaticNetworkConfigData
 		var newErr error
 		var shouldUseNmstateService bool
-		shouldUseNmstateService, err = ib.staticNetworkConfig.ShouldUseNmstateService(infraEnv.StaticNetworkConfig, infraEnv.OpenshiftVersion)
+		shouldUseNmstateService, err = ib.staticNetworkConfig.ShouldUseNmstateService(infraEnv.OpenshiftVersion)
 		if err != nil {
 			return "", err
 		}

internal/ignition/discovery_test.go

@@ -541,7 +541,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
 			infraEnv.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, "")
 			Expect(err).NotTo(HaveOccurred())
@@ -562,7 +562,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
 			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.X86CPUArchitecture
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, "")
@@ -584,7 +584,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
 			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.ARM64CPUArchitecture
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, "")
@@ -625,7 +625,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
 			infraEnv.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, string(models.ImageTypeFullIso))
 			Expect(err).NotTo(HaveOccurred())
@@ -647,7 +647,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
 			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.X86CPUArchitecture
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(true, nil).Times(1)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, string(models.ImageTypeFullIso))
 			Expect(err).NotTo(HaveOccurred())
@@ -669,7 +669,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
 			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.ARM64CPUArchitecture
-			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(formattedInput, infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
+			mockStaticNetworkConfig.EXPECT().ShouldUseNmstateService(infraEnv.OpenshiftVersion).Return(false, nil).Times(1)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
 			text, err := builder.FormatDiscoveryIgnitionFile(context.Background(), &infraEnv, ignitionConfig, false, auth.TypeRHSSO, string(models.ImageTypeFullIso))
 			Expect(err).NotTo(HaveOccurred())

pkg/staticnetworkconfig/backward_compatibility.go

@@ -3,7 +3,6 @@ package staticnetworkconfig
 import (
 	"github.com/openshift/assisted-service/internal/common"
 	log "github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v2"
 )
 
 type Config struct {
@@ -25,137 +24,11 @@ func (s *StaticNetworkConfigGenerator) NMStatectlServiceSupported(version string
 	return versionOK, nil
 }
 
-// CheckConfigForGlobalDnsCase detect whether any of the host-provided YAML configurations contain an interface with auto-dns: false, dhcp: true.
-// TODO: This is a temporary workaround and should be removed once the auto-dns:false, dhcp:true bug is fixed
-func (s *StaticNetworkConfigGenerator) CheckConfigForGlobalDnsCase(staticNetworkConfigStr string) (bool, error) {
-	staticNetworkConfig, err := s.decodeStaticNetworkConfig(staticNetworkConfigStr)
-	if err != nil {
-		s.log.WithError(err).Errorf("Failed to decode static network config")
-		return false, err
-	}
-
-	for _, hostConfig := range staticNetworkConfig {
-		isIncludeAutoDnsSetToFalse, err := s.hasDisabledAutoDnsWithDhcp(hostConfig.NetworkYaml)
-		if err != nil {
-			return false, err
-		}
-		if isIncludeAutoDnsSetToFalse {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-func (s *StaticNetworkConfigGenerator) hasDisabledAutoDnsWithDhcp(networksYaml string) (bool, error) {
-	var config map[string]interface{}
-
-	// Unmarshal the YAML string into the config struct
-	err := yaml.Unmarshal([]byte(networksYaml), &config)
-	if err != nil {
-		s.log.WithError(err).Errorf("Error unmarshalling yaml")
-		return false, err
-	}
-
-	interfaces, exists := config["interfaces"]
-	if !exists || interfaces == nil {
-		return false, nil
-	}
-	interfacesSlice, ok := interfaces.([]interface{})
-	if !ok {
-		return false, nil
-	}
-
-	isDHCPButNoAutoDNS := func(ipConfig map[interface{}]interface{}) bool {
-		autoDNSDisabled := false
-		dhcpEnabled := false
-
-		if autoDns, exists := ipConfig["auto-dns"]; exists && autoDns == false {
-			autoDNSDisabled = true
-		}
-		if dhcp, exists := ipConfig["dhcp"]; exists && dhcp == true {
-			dhcpEnabled = true
-		}
-
-		return autoDNSDisabled && dhcpEnabled
-	}
-
-	for _, iface := range interfacesSlice {
-		nic := iface.(map[interface{}]interface{})
-
-		if ipv4, exists := nic["ipv4"].(map[interface{}]interface{}); exists && isDHCPButNoAutoDNS(ipv4) {
-			return true, nil
-		}
-		if ipv6, exists := nic["ipv6"].(map[interface{}]interface{}); exists && isDHCPButNoAutoDNS(ipv6) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-// CheckConfigForMACIdentifier TODO: This is a temporary workaround and should be removed once the mac-identifier bug in nmstate is fixed - RHEL-72440.
-func (s *StaticNetworkConfigGenerator) CheckConfigForMACIdentifier(staticNetworkConfigStr string) (bool, error) {
-	staticNetworkConfig, err := s.decodeStaticNetworkConfig(staticNetworkConfigStr)
-	if err != nil {
-		s.log.WithError(err).Errorf("Failed to decode static network config")
-		return false, err
-	}
-
-	for _, hostConfig := range staticNetworkConfig {
-		isIncludeMacIdentifier, err := s.hasMACIdentifier(hostConfig.NetworkYaml)
-		if err != nil {
-			return false, err
-		}
-		if isIncludeMacIdentifier {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-func (s *StaticNetworkConfigGenerator) hasMACIdentifier(networksYaml string) (bool, error) {
-	var config map[string]interface{}
-
-	// Unmarshal the YAML string into the config struct
-	err := yaml.Unmarshal([]byte(networksYaml), &config)
-	if err != nil {
-		s.log.WithError(err).Errorf("Error unmarshalling yaml")
-		return false, err
-	}
-
-	interfaces, exists := config["interfaces"]
-	if !exists || interfaces == nil {
-		return false, nil
-	}
-	interfacesSlice, ok := interfaces.([]interface{})
-	if !ok {
-		return false, nil
-	}
-
-	for _, iface := range interfacesSlice {
-		nic := iface.(map[interface{}]interface{})
-		identifier, exists := nic["identifier"]
-		if exists && identifier == "mac-address" {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 // ShouldUseNmstatectlService - Both static networking flows should be maintained: one without nmstate.service and one with it, since nmstate.service isn't available in all RHCOS versions.
-func (s *StaticNetworkConfigGenerator) ShouldUseNmstateService(staticNetworkConfigStr, openshiftVersion string) (bool, error) {
-	includesMACIdentifier, err := s.CheckConfigForMACIdentifier(staticNetworkConfigStr)
-	if err != nil {
-		return false, err
-	}
-
-	includeAutoDnsSetToFalse, err := s.CheckConfigForGlobalDnsCase(staticNetworkConfigStr)
-	if err != nil {
-		return false, err
-	}
-
+func (s *StaticNetworkConfigGenerator) ShouldUseNmstateService(openshiftVersion string) (bool, error) {
 	isNmstateServiceSupported, err := s.NMStatectlServiceSupported(openshiftVersion)
 	if err != nil {
 		return false, err
 	}
-	return isNmstateServiceSupported && !includesMACIdentifier && !includeAutoDnsSetToFalse, nil
+	return isNmstateServiceSupported, nil
 }

pkg/staticnetworkconfig/backward_compatibility_test.go

@@ -1,63 +1,46 @@
 package staticnetworkconfig_test
 
 import (
-	"fmt"
-
 	. "github.com/onsi/ginkgo"
-	"github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
 	"github.com/openshift/assisted-service/internal/common"
 	snc "github.com/openshift/assisted-service/pkg/staticnetworkconfig"
 	"github.com/sirupsen/logrus"
 )
 
 var _ = Describe("ShouldUseNmstateService", func() {
-	var (
+	var staticNetworkGenerator snc.StaticNetworkConfig
+
+	BeforeEach(func() {
 		staticNetworkGenerator = snc.New(logrus.New(), snc.Config{MinVersionForNmstateService: common.MinimalVersionForNmstatectl})
-		hostsYAML              = `[{ "network_yaml": "%s" }]`
-		withMacIdentifier      = `interfaces:
-- name: eth0
-  type: ethernet
-  state: up
-  identifier: mac-address
-  mac-address: 02:00:00:80:12:14
-  ipv4:
-    enabled: true
-    address:
-      - ip: 192.0.2.1
-        prefix-length: 24`
-		withoutMacIdentifier = `interfaces:
-- name: eth0
-  type: ethernet
-  state: up
-  ipv4:
-    enabled: true
-    address:
-      - ip: 192.0.2.1
-        prefix-length: 24`
-		withAutoDnsSetToFalseDhcpTrue = `interfaces:
-- ipv4:
-    auto-dns: false
-    dhcp: true
-    enabled: true
-  ipv6:
-    enabled: false
-  name: eth0
-  state: up
-  type: ethernet
-`
-	)
-	table.DescribeTable("different scenarios", func(YAML, version string, expectedResult bool) {
-		escapedYamlContent, err := escapeYAMLForJSON(YAML)
+	})
+
+	It("returns false when version is empty", func() {
+		result, err := staticNetworkGenerator.ShouldUseNmstateService("")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(BeFalse())
+	})
+
+	It("returns false for version below minimum", func() {
+		result, err := staticNetworkGenerator.ShouldUseNmstateService("4.17")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(BeFalse())
+	})
+
+	It("returns true for the minimum version", func() {
+		result, err := staticNetworkGenerator.ShouldUseNmstateService(common.MinimalVersionForNmstatectl)
 		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(BeTrue())
+	})
 
-		shouldUseNmstateService, err := staticNetworkGenerator.ShouldUseNmstateService(fmt.Sprintf(hostsYAML, escapedYamlContent), version)
+	It("returns true for version above minimum", func() {
+		result, err := staticNetworkGenerator.ShouldUseNmstateService("4.19")
 		Expect(err).NotTo(HaveOccurred())
-		Expect(shouldUseNmstateService).To(Equal(expectedResult))
-	},
-		table.Entry("If the YAML contains a mac-identifier, and the version is >= MinimalVersionForNmstatectl,  we shouldn't use the nmstate service flow", withMacIdentifier, common.MinimalVersionForNmstatectl, false),
-		table.Entry("If the YAML contains a mac-identifier, and the version is < MinimalVersionForNmstatectl,  we shouldn't use the nmstate service flow", withMacIdentifier, "4.12", false),
-		table.Entry("If the YAML doesn't contain a mac-identifier and the version is >= MinimalVersionForNmstatectl, we should use the nmstate service flow", withoutMacIdentifier, common.MinimalVersionForNmstatectl, true),
-		table.Entry("If the YAML doesn't contain a mac-identifier, and the version < MinimalVersionForNmstatectl we shouldn't use the nmstate service flow.", withoutMacIdentifier, "4.12", false),
-		table.Entry("If the YAML contains a auto-dns: false, dhcp: true, and the version >= MinimalVersionForNmstatectl we shouldn't use the nmstate service flow.", withAutoDnsSetToFalseDhcpTrue, common.MinimalVersionForNmstatectl, false))
+		Expect(result).To(BeTrue())
+	})
+
+	It("returns an error for an invalid version", func() {
+		_, err := staticNetworkGenerator.ShouldUseNmstateService("not-a-version")
+		Expect(err).To(HaveOccurred())
+	})
 })

pkg/staticnetworkconfig/generator.go

@@ -33,7 +33,7 @@ type StaticNetworkConfig interface {
 	GenerateStaticNetworkConfigDataYAML(staticNetworkConfigStr string) ([]StaticNetworkConfigData, error)
 	FormatStaticNetworkConfigForDB(staticNetworkConfig []*models.HostStaticNetworkConfig) (string, error)
 	ValidateStaticConfigParamsYAML(staticNetworkConfig []*models.HostStaticNetworkConfig) error
-	ShouldUseNmstateService(staticNetworkConfigStr, openshiftVersion string) (bool, error)
+	ShouldUseNmstateService(openshiftVersion string) (bool, error)
 }
 
 type StaticNetworkConfigGenerator struct {