Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/bminventory/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, common.X86CPUArchitecture)
ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
if err != nil {
return common.GenerateErrorResponder(err)
}
Expand Down Expand Up @@ -5878,7 +5878,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, common.X86CPUArchitecture)
ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
if err != nil {
return common.GenerateErrorResponder(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/ignition/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, common.X86CPUArchitecture)
ok, err = staticnetworkconfig.NMStatectlServiceSupported(infraEnv.OpenshiftVersion, infraEnv.CPUArchitecture)
Comment thread
linoyaslan marked this conversation as resolved.
Outdated
if err != nil {
return "", err
}
Expand Down
47 changes: 44 additions & 3 deletions internal/ignition/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ 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", func() {
It("produces a valid ignition v3.1 spec with static ips paramters - ocp versions greater than/ equal to 4.14, x86 arch", func() {
mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigDataYAML(formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
infraEnv.StaticNetworkConfig = formattedInput
infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
Expand All @@ -574,6 +574,27 @@ 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() {
mockStaticNetworkConfig.EXPECT().GenerateStaticNetworkConfigData(gomock.Any(), formattedInput).Return(staticNetworkConfigNmpolicyOutput, nil).Times(1)
infraEnv.StaticNetworkConfig = formattedInput
infraEnv.Type = common.ImageTypePtr(models.ImageTypeFullIso)
infraEnv.OpenshiftVersion = ocpVersionInvolvingNmstateService
infraEnv.CPUArchitecture = common.ARM64CPUArchitecture
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, "")
Expect(err).NotTo(HaveOccurred())
config, report, err := config_31.Parse([]byte(text))
Expect(err).NotTo(HaveOccurred())
Expect(report.IsFatal()).To(BeFalse())
count := 0
for _, f := range config.Storage.Files {
if strings.HasSuffix(f.Path, "yml") || strings.HasSuffix(f.Path, "mac_interface.ini") {
count += 1
}
}
Expect(count).Should(Equal(3))
})
It("Doesn't include static network config for minimal isos - ocp versions less than 4.14", func() {
infraEnv.StaticNetworkConfig = formattedInput
infraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)
Expand Down Expand Up @@ -613,8 +634,7 @@ 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", 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 4.14, 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
Expand All @@ -635,6 +655,27 @@ 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() {
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.CPUArchitecture = common.ARM64CPUArchitecture
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())
config, report, err := config_31.Parse([]byte(text))
Expect(err).NotTo(HaveOccurred())
Expect(report.IsFatal()).To(BeFalse())
count := 0
for _, f := range config.Storage.Files {
if strings.HasSuffix(f.Path, "yml") || strings.HasSuffix(f.Path, "mac_interface.ini") {
count += 1
}
}
Expect(count).Should(Equal(3))
})

It("Will not include static network config for full iso type in infraenv if overridden in call to FormatDiscoveryIgnitionFile", func() {
infraEnv.StaticNetworkConfig = formattedInput
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ DefaultDependencies=no
User=root
Type=oneshot
TimeoutSec=60
StandardError=journal
StandardOutput=journal
ExecStart=/bin/bash /usr/local/bin/pre-network-manager-config.sh
PrivateTmp=true
RemainAfterExit=no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ DefaultDependencies=no
User=root
Type=oneshot
TimeoutSec=60
StandardError=journal
StandardOutput=journal
ExecStart=/bin/bash /usr/local/bin/pre-network-manager-config.sh
PrivateTmp=true
RemainAfterExit=no
Expand Down
44 changes: 43 additions & 1 deletion pkg/staticnetworkconfig/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,42 @@ func New(log logrus.FieldLogger) StaticNetworkConfig {
}
}

// 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) {
Comment thread
linoyaslan marked this conversation as resolved.
Outdated
var config map[string]interface{}

// Unmarshal the JSON string into the config struct
err := yaml.Unmarshal([]byte(networkYaml), &config)
if err != nil {
s.log.WithError(err).Errorf("Error unmarshalling yaml")
return "", err
}
Interfaces := config["interfaces"].([]interface{})
for _, iface := range Interfaces {
nic := iface.(map[interface{}]interface{})
if val, exists := nic["ipv4"].(map[interface{}]interface{}); exists {
if _, exists := val["dhcp"]; !exists {
val["dhcp"] = false
}
}
if val, exists := nic["ipv6"].(map[interface{}]interface{}); exists {
if _, exists := val["dhcp"]; !exists {
val["dhcp"] = false
}
if _, exists := val["autoconf"]; !exists {
val["autoconf"] = false
Comment on lines 86 to 87

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also check if dhcp is false before setting auto conf + let add this case to tests so we wont have dhcp = true and autoconf = false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dhcp = true and autoconf = false is translated to method=dhcp in the network manager key file. It is a valid config (stateful IPv6).

}
}
}
// Marshal the updated config back into YAML format
updatedYaml, err := yaml.Marshal(&config)
if err != nil {
s.log.WithError(err).Errorf("Error marshalling yaml")
return "", err
}
return string(updatedYaml), nil
}

func (s *StaticNetworkConfigGenerator) injectNMPolicyCaptures(hostConfig *models.HostStaticNetworkConfig) (string, error) {
interfacesWithCaptures := hostConfig.NetworkYaml
var config interfaceConfig
Expand Down Expand Up @@ -128,6 +164,12 @@ func (s *StaticNetworkConfigGenerator) GenerateStaticNetworkConfigDataYAML(stati
}

for i, hostConfig := range staticNetworkConfig {
modifiedYaml, err := s.injectIPV4V6FieldsIfNeeded(hostConfig.NetworkYaml)
if err != nil {
return nil, err
}
hostConfig.NetworkYaml = modifiedYaml

nmpolicy, err := s.injectNMPolicyCaptures(hostConfig)
if err != nil {
return nil, err
Expand Down Expand Up @@ -277,7 +319,7 @@ func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistenceYAML(macIn
// Unmarshal the JSON string into the config struct
err := yaml.Unmarshal([]byte(networksYaml), &config)
if err != nil {
s.log.WithError(err).Errorf("Error unmarshalling JSON")
s.log.WithError(err).Errorf("Error unmarshalling yaml")
return err
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/staticnetworkconfig/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,12 +597,14 @@ var _ = Describe("StaticNetworkConfig.GenerateStaticNetworkConfigDataYAML - gene
config, err := staticNetworkGenerator.GenerateStaticNetworkConfigDataYAML(fmt.Sprintf(hostsYAML, escapedYamlContent))
fileContent := config[1].FileContents
Expect(fileContent).To(ContainSubstring("capture"))
Expect(fileContent).To(ContainSubstring("dhcp: false"))
Expect(err).NotTo(HaveOccurred())
})
It("Success - with ini file", func() {
config, err := staticNetworkGenerator.GenerateStaticNetworkConfigDataYAML(fmt.Sprintf(hostsYAMLAndIni, escapedYamlContent, macInterfaceMap))
fileContent := config[1].FileContents
Expect(fileContent).To(ContainSubstring("name: \"{{ capture.iface0.interfaces.0.name }}\""))
Expect(fileContent).To(ContainSubstring("dhcp: false"))
Expect(err).NotTo(HaveOccurred())
})
})
Expand Down
12 changes: 7 additions & 5 deletions subsystem/infra_env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ var _ = Describe("Infra_Env", func() {
Expect(s.Size()).ShouldNot(Equal(0))
})

DescribeTable("download infra-env files static network config file", func(ocpVersion string) {
DescribeTable("download infra-env files static network config file", func(ocpVersion, arch string) {
By("Patching the infra env with a static network config")
netYaml := `interfaces:
- ipv4:
Expand All @@ -334,7 +334,7 @@ var _ = Describe("Infra_Env", func() {
},
},
}
infraEnv = internalRegisterInfraEnv(nil, models.ImageTypeFullIso, "", ocpVersion)
infraEnv = internalRegisterInfraEnv(nil, models.ImageTypeFullIso, arch, ocpVersion)
infraEnvID = *infraEnv.ID
staticNetworkConfigs := []*models.HostStaticNetworkConfig{&staticNetworkConfig}
updateParams := &installer.UpdateInfraEnvParams{
Expand All @@ -353,13 +353,15 @@ var _ = Describe("Infra_Env", func() {
contents := buf.String()
Expect(len(contents)).ShouldNot(Equal(0))
Expect(contents).To(ContainSubstring("/etc/assisted/network/host0"))
if ocpVersion < staticnetworkconfig.MinimalVersionForNmstatectl {
if ocpVersion < staticnetworkconfig.MinimalVersionForNmstatectl || arch == common.ARM64CPUArchitecture {
Expect(contents).To(ContainSubstring("192.0.2.1/24"))
}
Expect(contents).To(ContainSubstring("eth0"))
},
Entry("ocp versions greater than/ equal to 4.14", staticnetworkconfig.MinimalVersionForNmstatectl),
Entry("ocp versions less than 4.14", "4.12"),
Entry("ocp versions greater than/ equal to 4.14, x86 arch", staticnetworkconfig.MinimalVersionForNmstatectl, common.X86CPUArchitecture),
Entry("ocp versions greater than/ equal to 4.14, arm arch", staticnetworkconfig.MinimalVersionForNmstatectl, common.ARM64CPUArchitecture),
Comment on lines 361 to 362

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a typo ? than/ equal ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

greater than 4.14 / equal to 4.14

Entry("ocp versions less than 4.14, x86 arch", "4.12", common.X86CPUArchitecture),
Entry("ocp versions less than 4.14, arm arch", "4.12", common.ARM64CPUArchitecture),
Comment on lines 363 to 364

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what we are testing here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For OCP versions below 4.14, the static networking flow follows the older process, without using nmpolicy or the nmstate service. In this case, the files under /etc/assisted/network/host0 will be the nmconnection files, not nmpolicy. This behavior is consistent across all architectures.

It might be worth updating the condition in the test to:
if ocpVersion < staticnetworkconfig.MinimalVersionForNmstatectl || arch == common.ARM64CPUArchitecture

)

It("download infra-env files invalid filename option", func() {
Expand Down