MGMT-18579, MGMT-19018: Add feature flag for minimal OCP version for nmstate service flow

cmd/main.go

@@ -118,6 +118,7 @@ var Options struct {
 	OperatorsConfig                      operators.Options
 	GCConfig                             garbagecollector.Config
 	ReleaseSourcesConfig                 releasesources.Config
+	StaticNetworkConfig                  staticnetworkconfig.Config
 	IgnoredOpenshiftVersions             string        `envconfig:"IGNORED_OPENSHIFT_VERSIONS" default:""`
 	ClusterStateMonitorInterval          time.Duration `envconfig:"CLUSTER_MONITOR_INTERVAL" default:"10s"`
 	ClusterEventsUploaderInterval        time.Duration `envconfig:"CLUSTER_EVENTS_UPLOADER_INTERVAL" default:"15m"`
@@ -319,7 +320,7 @@ func main() {
 	)
 	failOnError(err, "failed to create Versions handlers")
 	domainHandler := domains.NewHandler(Options.BMConfig.BaseDNSDomains)
-	staticNetworkConfig := staticnetworkconfig.New(log.WithField("pkg", "static_network_config"))
+	staticNetworkConfig := staticnetworkconfig.New(log.WithField("pkg", "static_network_config"), Options.StaticNetworkConfig)
 	ignitionBuilder, err := ignition.NewBuilder(log.WithField("pkg", "ignition"), staticNetworkConfig, mirrorRegistriesBuilder, releaseHandler, versionHandler)
 	failOnError(err, "failed to create ignition builder")
 	installConfigBuilder := installcfg.NewInstallConfigBuilder(log.WithField("pkg", "installcfg"), mirrorRegistriesBuilder, providerRegistry)

internal/bminventory/inventory.go

@@ -3708,7 +3708,7 @@ func (b *bareMetalInventory) DownloadMinimalInitrd(ctx context.Context, params i
 	if infraEnv.StaticNetworkConfig != "" {
 		// backward compatibility - nmstate.service has been available on RHCOS since version 4.14+, therefore, we should maintain both flows
 		var ok bool
-		ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
+		ok, err = b.staticNetworkConfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
 		if err != nil {
 			return common.GenerateErrorResponder(err)
 		}
@@ -5915,7 +5915,7 @@ func (b *bareMetalInventory) V2DownloadInfraEnvFiles(ctx context.Context, params
 		if infraEnv.StaticNetworkConfig != "" {
 			// backward compatibility - nmstate.service has been available on RHCOS since version 4.14+, therefore, we should maintain both flows
 			var ok bool
-			ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
+			ok, err = b.staticNetworkConfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
 			if err != nil {
 				return common.GenerateErrorResponder(err)
 			}

internal/common/test_configuration.go

@@ -49,6 +49,7 @@ type TestConfiguration struct {
 
 const TestDiskId = "/dev/disk/by-id/test-disk-id"
 const TestDiskPath = "/dev/test-disk"
+const MinimalVersionForNmstatectl = "4.18"
 
 var (
 	OpenShiftVersion string = "4.6"

internal/ignition/discovery.go

@@ -302,7 +302,7 @@ func (ib *ignitionBuilder) FormatDiscoveryIgnitionFile(ctx context.Context, infr
 
 		// backward compatibility - nmstate.service has been available on RHCOS since version 4.14+, therefore, we should maintain both flows
 		var ok bool
-		ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
+		ok, err = ib.staticNetworkConfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
 		if err != nil {
 			return "", err
 		}

internal/ignition/discovery_test.go

@@ -91,7 +91,6 @@ var _ = Describe("IgnitionBuilder", func() {
 		mockVersionHandler                  *versions.MockHandler
 		ignitionConfig                      IgnitionConfig
 		ocpVersionInvolvingGenerateKeyfiles = "4.12"
-		ocpVersionInvolvingNmstateService   = "4.14"
 	)
 
 	BeforeEach(func() {
@@ -533,12 +532,13 @@ var _ = Describe("IgnitionBuilder", func() {
 			},
 		}
 
-		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions less than 4.14", func() {
+		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions less than MinimalVersionForNmstatectl", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticnetworkConfigOutput, nil).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
 			infraEnv.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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())
@@ -553,12 +553,13 @@ var _ = Describe("IgnitionBuilder", func() {
 			}
 			Expect(count).Should(Equal(3))
 		})
-		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions greater than/ equal to 4.14, x86 arch", func() {
+		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions greater than/ equal to MinimalVersionForNmstatectl, x86 arch", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigDataYAML(formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
-			infraEnv.OpenshiftVersion = ocpVersionInvolvingNmstateService
+			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.X86CPUArchitecture
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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, "")
@@ -574,12 +575,13 @@ var _ = Describe("IgnitionBuilder", func() {
 			}
 			Expect(count).Should(Equal(3))
 		})
-		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions greater than/ equal to 4.14, arm arch", func() {
+		It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions greater than/ equal to MinimalVersionForNmstatectl, arm arch", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
-			infraEnv.OpenshiftVersion = ocpVersionInvolvingNmstateService
+			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.ARM64CPUArchitecture
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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, "")
@@ -595,7 +597,7 @@ var _ = Describe("IgnitionBuilder", func() {
 			}
 			Expect(count).Should(Equal(3))
 		})
-		It("Doesn't include static network config for minimal isos - ocp versions less than 4.14", func() {
+		It("Doesn't include static network config for minimal isos - ocp versions less than MinimalVersionForNmstatectl", func() {
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
 			mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1)
@@ -614,12 +616,13 @@ var _ = Describe("IgnitionBuilder", func() {
 			Expect(count).Should(Equal(0))
 		})
 
-		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions less than 4.14", func() {
+		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions less than MinimalVersionForNmstatectl", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticnetworkConfigOutput, nil).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
 			infraEnv.OpenshiftVersion = ocpVersionInvolvingGenerateKeyfiles
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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())
@@ -634,13 +637,14 @@ var _ = Describe("IgnitionBuilder", func() {
 			}
 			Expect(count).Should(Equal(3))
 		})
-		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions greater than/ equal to 4.14, x86 arch", func() {
+		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions greater than/ equal to MinimalVersionForNmstatectl, x86 arch", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigDataYAML(formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
-			infraEnv.OpenshiftVersion = ocpVersionInvolvingNmstateService
+			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.X86CPUArchitecture
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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())
@@ -655,13 +659,14 @@ var _ = Describe("IgnitionBuilder", func() {
 			}
 			Expect(count).Should(Equal(3))
 		})
-		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions greater than/ equal to 4.14, arm arch", func() {
+		It("Will include static network config for minimal iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile - ocp versions greater than/ equal to MinimalVersionForNmstatectl, arm arch", func() {
 			mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
 			mockVersionHandler.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")).Times(1)
 			infraEnv.StaticNetworkConfig = formattedInput
 			infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
-			infraEnv.OpenshiftVersion = ocpVersionInvolvingNmstateService
+			infraEnv.OpenshiftVersion = common.MinimalVersionForNmstatectl
 			infraEnv.CPUArchitecture = common.ARM64CPUArchitecture
+			mockStaticNetworkConfig.EXPECT().NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture).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())

internal/isoeditor/rhcos_test.go

@@ -20,7 +20,7 @@ func TestIsoEditor(t *testing.T) {
 }
 
 var _ = Describe("RamdiskImageArchive", func() {
-	It("adds a new archive correctly - ocp versions less than 4.14", func() {
+	It("adds a new archive correctly - ocp versions less than MinimalVersionForNmstatectl", func() {
 
 		clusterProxyInfo := ClusterProxyInfo{
 			HTTPProxy:  "http://10.10.1.1:3128",
@@ -90,7 +90,7 @@ var _ = Describe("RamdiskImageArchive", func() {
 			clusterProxyInfo.HTTPProxy, clusterProxyInfo.HTTPSProxy, clusterProxyInfo.NoProxy)
 		Expect(rootfsServiceConfigContent).To(Equal(rootfsServiceConfig))
 	})
-	It("returns nothing when given nothing - ocp versions less than 4.14", func() {
+	It("returns nothing when given nothing - ocp versions less than MinimalVersionForNmstatectl", func() {
 		archive, err := RamdiskImageArchive(
 			[]staticnetworkconfig.StaticNetworkConfigData{},
 			&ClusterProxyInfo{},
@@ -99,7 +99,7 @@ var _ = Describe("RamdiskImageArchive", func() {
 		Expect(archive).To(BeNil())
 	})
 
-	It("adds a new archive correctly - ocp versions greater than/ equal to 4.14", func() {
+	It("adds a new archive correctly - ocp versions greater than/ equal to MinimalVersionForNmstatectl", func() {
 
 		clusterProxyInfo := ClusterProxyInfo{
 			HTTPProxy:  "http://10.10.1.1:3128",
@@ -169,7 +169,7 @@ var _ = Describe("RamdiskImageArchive", func() {
 			clusterProxyInfo.HTTPProxy, clusterProxyInfo.HTTPSProxy, clusterProxyInfo.NoProxy)
 		Expect(rootfsServiceConfigContent).To(Equal(rootfsServiceConfig))
 	})
-	It("returns nothing when given nothing - ocp versions greater than/ equal to 4.14", func() {
+	It("returns nothing when given nothing - ocp versions greater than/ equal to MinimalVersionForNmstatectl", func() {
 		archive, err := RamdiskImageArchive(
 			[]staticnetworkconfig.StaticNetworkConfigData{},
 			&ClusterProxyInfo{},

openshift/template.yaml

@@ -2,6 +2,9 @@
 parameters:
 - name: ASSISTED_SERVICE_IMAGE
   value: quay.io/app-sre/assisted-service
+- name: MIN_VERSION_FOR_NMSTATE_SERVICE
+  value: '4.18'
+  required: false
 - name: IMAGE_TAG
   value: ''
   required: true
@@ -330,6 +333,8 @@ objects:
                 value: ${ENABLE_SKIP_MCO_REBOOT}
               - name: ENABLE_SOFT_TIMEOUTS
                 value: ${ENABLE_SOFT_TIMEOUTS}
+              - name: MIN_VERSION_FOR_NMSTATE_SERVICE
+                value: ${MIN_VERSION_FOR_NMSTATE_SERVICE}
               - name: DB_HOST
                 valueFrom:
                   secretKeyRef:

pkg/staticnetworkconfig/backward_compatibility.go

@@ -5,16 +5,19 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-const MinimalVersionForNmstatectl = "4.14"
+type Config struct {
+	// MinVersionForNmstateService is a flag that enables the static networking flow using the nmstate service for specific OCP versions.
+	MinVersionForNmstateService string `envconfig:"MIN_VERSION_FOR_NMSTATE_SERVICE" default:"4.18"`
+}
 
-func NMStatectlServiceSupported(version, arch string) (bool, error) {
+func (s *StaticNetworkConfigGenerator) NMStatectlServiceSupported(version, arch string) (bool, error) {
 	// When a cluster is imported, the OpenshiftVersion isn't stored in the database.
 	// Consequently, a bound InfraEnv with static networking uses the Cluster's OpenshiftVersion, which is empty.
 	if version == "" {
 		log.Info("ocp version is empty")
 		return false, nil
 	}
-	versionOK, err := common.VersionGreaterOrEqual(version, MinimalVersionForNmstatectl)
+	versionOK, err := common.VersionGreaterOrEqual(version, s.config.MinVersionForNmstateService)
 	if err != nil {
 		return false, err
 	}

pkg/staticnetworkconfig/generator.go

@@ -47,25 +47,28 @@ type StaticNetworkConfig interface {
 	GenerateStaticNetworkConfigDataYAML(staticNetworkConfigStr string) ([]StaticNetworkConfigData, error)
 	FormatStaticNetworkConfigForDB(staticNetworkConfig []*models.HostStaticNetworkConfig) (string, error)
 	ValidateStaticConfigParamsYAML(staticNetworkConfig []*models.HostStaticNetworkConfig, ocpVersion, arch, installerInvoker string) error
+	NMStatectlServiceSupported(version, arch string) (bool, error)
 }
 
 type StaticNetworkConfigGenerator struct {
 	log     logrus.FieldLogger
 	nmstate *nmstate.Nmstate
+	config  Config
 }
 
-func New(log logrus.FieldLogger) StaticNetworkConfig {
+func New(log logrus.FieldLogger, config Config) StaticNetworkConfig {
 	return &StaticNetworkConfigGenerator{
 		log:     log,
 		nmstate: nmstate.New(),
+		config:  config,
 	}
 }
 
 // TODO: Temporary workaround until the nmstate team resolves and deploys the following issue - https://issues.redhat.com/browse/RHEL-59935
 func (s *StaticNetworkConfigGenerator) injectIPV4V6FieldsIfNeeded(networkYaml string) (string, error) {
 	var config map[string]interface{}
 
-	// Unmarshal the JSON string into the config struct
+	// Unmarshal the YAML string into the config struct
 	err := yaml.Unmarshal([]byte(networkYaml), &config)
 	if err != nil {
 		s.log.WithError(err).Errorf("Error unmarshalling yaml")
@@ -110,7 +113,7 @@ func (s *StaticNetworkConfigGenerator) injectNMPolicyCaptures(hostConfig *models
 
 	err := yaml.Unmarshal([]byte(interfacesWithCaptures), &config)
 	if err != nil {
-		s.log.WithError(err).Errorf("error unmarshalling JSON: %v", err)
+		s.log.WithError(err).Errorf("error unmarshalling YAML: %v", err)
 		return "", err
 	}
 
@@ -323,7 +326,7 @@ func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistenceYAML(macIn
 
 	var config map[string]interface{}
 
-	// Unmarshal the JSON string into the config struct
+	// 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")
@@ -364,7 +367,7 @@ func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistenceYAML(macIn
 
 		identifier, exists := nic["identifier"]
 		isMacAddressIdentifier := exists && identifier == "mac-address"
-		isVersionOK, err := NMStatectlServiceSupported(ocpVersion, arch)
+		isVersionOK, err := s.NMStatectlServiceSupported(ocpVersion, arch)
 		if err != nil {
 			return err
 		}