Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37367 - Switch to 'network' directive instead of ifcfg #9961

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Fixes #37367 - Switch to 'network' directive instead of ifcfg #9961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions app/models/nic/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ class Base < ApplicationRecord
end
class Jail < ::Safemode::Jail
allow :id, :subnet, :subnet6, :virtual?, :physical?, :mac, :ip, :ip6, :identifier, :attached_to,
:link, :tag, :domain, :vlanid, :mtu, :bond_options, :attached_devices, :mode,
:attached_devices_identifiers, :primary, :provision, :alias?, :inheriting_mac,
:children_mac_addresses, :nic_delay, :fqdn, :shortname, :type, :managed?, :bond?, :bridge?, :bmc?
:link, :tag, :domain, :bond_options, :attached_devices, :mode,
:attached_devices_identifiers, :primary, :provision, :inheriting_mac,
:children_mac_addresses, :nic_delay, :fqdn, :shortname, :type, :managed?, :bond?, :bmc?, :provision?
end

# include STI inheritance column in audits
Expand Down
4 changes: 4 additions & 0 deletions app/models/nic/interface.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class Interface < Base
alias_method :network, :subnet_network
alias_method :network6, :subnet6_network

class Jail < Nic::Base::Jail
allow :mtu, :vlanid, :bridge?, :alias?
end

def vlanid
# Determine a vlanid according to the following cascading rules:
# 1. if the interface has a tag, use that as the vlanid
Expand Down
7 changes: 7 additions & 0 deletions app/services/foreman/template_snapshot_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def host4dhcp
name: 'snapshot-ipv4-dhcp-el7',
subnet: FactoryBot.build(:subnet_ipv4_dhcp_for_snapshots),
interfaces: [ipv4_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to build a host with the right interfaces. I think it means ipv4_interface needs to be marked as managed.

Copy link
Member Author

Choose a reason for hiding this comment

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

managed_interfaces is based on DB scopes, it does not work correctly for non-saved hosts:

      host = FactoryBot.build(:managed_host,
        name: 'snapshot-ipv6-dhcp-el7',
        interfaces: [FactoryBot.build(:managed_interface, managed: true)])
      host.managed_interfaces
      # => []

      host.save!
      host.managed_interfaces
      # => [<Nic::Managed ... >]

define_host_params(host)
end

Expand All @@ -122,6 +123,7 @@ def host4static
name: 'snapshot-ipv4-static-el7',
subnet: FactoryBot.build(:subnet_ipv4_static_for_snapshots),
interfaces: [ipv4_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand All @@ -130,6 +132,7 @@ def host6dhcp
name: 'snapshot-ipv6-dhcp-el7',
subnet: FactoryBot.build(:subnet_ipv6_dhcp_for_snapshots),
interfaces: [ipv6_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand All @@ -138,6 +141,7 @@ def host6static
name: 'snapshot-ipv6-static-el7',
subnet: FactoryBot.build(:subnet_ipv6_static_for_snapshots),
interfaces: [ipv6_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand All @@ -147,6 +151,7 @@ def host4and6dhcp
subnet: FactoryBot.build(:subnet_ipv4_dhcp_for_snapshots),
subnet6: FactoryBot.build(:subnet_ipv6_dhcp_for_snapshots),
interfaces: [ipv46_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand All @@ -155,6 +160,7 @@ def debian4dhcp
name: 'snapshot-ipv4-dhcp-deb10',
subnet: FactoryBot.build(:subnet_ipv4_dhcp_for_snapshots),
interfaces: [ipv4_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand All @@ -179,6 +185,7 @@ def rhel9_dhcp
name: 'snapshot-ipv4-dhcp-rhel9',
subnet: FactoryBot.build(:subnet_ipv4_dhcp_for_snapshots),
interfaces: [ipv4_interface])
host.stubs(:managed_interfaces).returns(host.interfaces)
define_host_params(host)
end

Expand Down
8 changes: 0 additions & 8 deletions app/views/unattended/provisioning_templates/finish/kickstart_default_finish.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ description: |
%>

<%= snippet_if_exists(template_name + " custom pre") -%>
<% if @host.subnet.respond_to?(:dhcp_boot_mode?) -%>
<%= snippet 'kickstart_networking_setup' %>
<% if (rhel_compatible && os_major >= 8) -%>
systemctl restart NetworkManager
<% else -%>
service network restart
<% end -%>
<% end -%>

<% if @host.provision_method == 'image' && root_pass.present? -%>
# Install the root password
Expand Down
95 changes: 15 additions & 80 deletions app/views/unattended/provisioning_templates/provision/kickstart_default.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,85 +114,22 @@ selinux --<%= host_param('selinux-mode') || host_param('selinux') || 'enforcing'
keyboard <%= host_param('keyboard') || 'us' %>

<%
network_options = []
nameservers = []
subnet4 = iface.subnet
subnet6 = iface.subnet6

# device and hostname
if iface.bond? && rhel_compatible && os_major >= 6
network_options.push("--device=#{iface.identifier}")
else
network_options.push("--device=#{iface.mac || iface.identifier}")
end
network_options.push("--hostname #{@host.name}")

# single stack
if subnet4 && !subnet6
network_options.push("--noipv6")
elsif !subnet4 && subnet6
network_options.push("--noipv4")
end

# dual stack MTU check
raise("IPv4 and IPv6 subnets have different MTU") if subnet4 && subnet6 && subnet4.mtu.present? && subnet6.mtu.present? && subnet4.mtu != subnet6.mtu

# IPv4
if (subnet4 && !subnet4.dhcp_boot_mode?) || @static
network_options.push("--bootproto static")
network_options.push("--ip=#{iface.ip}")
network_options.push("--netmask=#{subnet4.mask}")
network_options.push("--gateway=#{subnet4.gateway}")
elsif subnet4 && subnet4.dhcp_boot_mode?
network_options.push("--bootproto dhcp")
end
if subnet4
nameservers.concat(subnet4.dns_servers)
network_options.push("--mtu=#{subnet4.mtu}") if subnet4.mtu.present?
end

# IPv6
if rhel_compatible && os_major >= 6
if (subnet6 && !subnet6.dhcp_boot_mode?) || @static6
network_options.push("--ipv6=#{iface.ip6}/#{subnet6.cidr}")
network_options.push("--ipv6gateway=#{subnet6.gateway}")
elsif subnet6 && subnet6.dhcp_boot_mode?
if host_param_true?('use-slaac')
network_options.push("--ipv6 auto")
else
network_options.push("--ipv6 dhcp")
end
end
if subnet6
nameservers.concat(subnet6.dns_servers)
network_options.push("--mtu=#{subnet6.mtu}") if subnet6.mtu.present?
end
end

# bond
if iface.bond? && rhel_compatible && os_major >= 6
bond_slaves = iface.attached_devices_identifiers.join(',')
network_options.push("--bondslaves=#{bond_slaves}")
network_options.push("--bondopts=mode=#{iface.mode},#{iface.bond_options.tr(' ', ',')}")
end

# VLAN (only on physical is recognized)
if iface.virtual? && iface.tag.present? && iface.attached_to.present?
if rhel_compatible && os_major == 6
network_options.push("--vlanid=#{iface.tag}")
else
network_options.push("--interfacename=vlan#{iface.tag}")
end
end

# DNS
if nameservers.size > 0
network_options.push("--nameserver=#{nameservers.join(',')}")
else
network_options.push("--nodns")
end
# start with provisioning interface, then other non-bond non-bridge interfaces and the bonds + bridges at the end
@host.interfaces.reject{ |iface| iface.bmc? }.sort_by { |iface| (iface.bond? || iface.bridge?) ? 0 : iface.provision? ? 20 : 10 }.each do |iface|
-%>
<%= snippet(
stejskalleos marked this conversation as resolved.
Show resolved Hide resolved
'kickstart_network_interface',
variables: {
iface: iface,
host: @host,
use_slaac: host_param_true?('use-slaac'),
static: @static,
static6: @static6
}
) -%>
<%
end
-%>
network <%= network_options.join(' ') %>

rootpw --iscrypted <%= root_pass %>
<% if host_param_true?('disable-firewall') -%>
Expand Down Expand Up @@ -300,8 +237,6 @@ exec < /dev/tty3 > /dev/tty3
chvt 3
(
logger "Starting anaconda <%= @host %> postinstall"
<%= snippet 'kickstart_networking_setup' %>

<%= snippet 'ntp' %>

<%= snippet 'yum_proxy' %>
Expand Down
26 changes: 18 additions & 8 deletions app/views/unattended/provisioning_templates/provision/kickstart_ovirt.erb
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I think we should get rid of this template. RHV is deprecated and going away and oVirt doesn't look too healthy these days.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's an issue that should be properly discussed in the community (started a thread: https://community.theforeman.org/t/proposal-to-drop-support-for-ovirt/36324) and anyway it's a matter for a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

I meant to only drop the kicjstart tenplate. The whole of oVirt is a separate discussion. I don't know if you even still can provision with this template or if we provide functionality for something that no longer exists.

Having said that, I also noticed I can no longer compile the oVirt SDK on Fedora so that alone can be a reason to drop it.

I'll weigh in on the RFC later, when I'm home

Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,24 @@ end

liveimg --url=<%= liveimg_url %>

<% subnet = @host.subnet -%>
<% if subnet.respond_to?(:dhcp_boot_mode?) -%>
<% dhcp = subnet.dhcp_boot_mode? && !@static -%>
<% else -%>
<% dhcp = !@static -%>
<% end -%>
network --bootproto <%= dhcp ? 'dhcp' : "static --ip=#{@host.ip} --netmask=#{subnet.mask} --gateway=#{subnet.gateway} --nameserver=#{subnet.dns_servers.join(',')}" %> --hostname <%= @host %> --device=<%= @host.mac -%>
<%
# start with provisioning interface, then other non-bond interfaces and the bonds at the end
@host.managed_interfaces.sort_by { |iface| (iface.bond? || iface.bridge?) 0 : iface.provision? 20 : 10 }.each |iface| do
%>
<%= snippet(
'kickstart_network_interface',
variables: {
iface: iface,
host: @host,
use_slaac: false,
static: @static,
static6: @static6
}
) -%>
<%
end
-%>


rootpw --iscrypted <%= root_pass %>
<% if host_param_true?('disable-firewall') -%>
Expand All @@ -77,7 +88,6 @@ reboot
%post --log=/root/ks.post.log --erroronfail
nodectl init
<%= snippet 'redhat_register' %>
<%= snippet 'kickstart_networking_setup' %>
<%= snippet 'efibootmgr_netboot' %>
<% if host_param_true?('host_registration_insights') -%>
<%= snippet 'insights' -%>
Expand Down
108 changes: 108 additions & 0 deletions app/views/unattended/provisioning_templates/snippet/kickstart_network_interface.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<%#
name: kickstart_network_interface
model: ProvisioningTemplate
snippet: true
model: ProvisioningTemplate
kind: snippet
description: |
Generates network directive for a given interface. It is not expected to be used directly.
-%>
<%#
# Variables: iface, host, use_slaac, static, static6
-%>
<%if @iface.managed? -%>
<%
network_options = []
nameservers = []
subnet4 = @iface.subnet
subnet6 = @iface.subnet6

rhel_compatible = @host.operatingsystem.family == 'Redhat' && @host.operatingsystem.name != 'Fedora'
is_fedora = @host.operatingsystem.name == 'Fedora'
os_major = @host.operatingsystem.major.to_i

# device and hostname
if @iface.bond? || @iface.bridge?
network_options.push("--device=#{@iface.identifier}")
else
network_options.push("--device=#{@iface.mac || @iface.identifier}")
end
network_options.push("--hostname #{@host.name}")

# single stack
network_options.push("--noipv6") if subnet4 && !subnet6
network_options.push("--noipv4") if !subnet4 && subnet6

# dual stack MTU check
raise("IPv4 and IPv6 subnets have different MTU") if subnet4 && subnet6 && subnet4.mtu.present? && subnet6.mtu.present? && subnet4.mtu != subnet6.mtu
Copy link
Member

Choose a reason for hiding this comment

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

I think we should implement this check in @iface.mtu instead. Perhaps not in this PR because I don't know how big the implications are.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created an issue: https://projects.theforeman.org/issues/37216


# mtu method is taking into account both ipv4 and ipv6 stacks
if @iface.respond_to?(:mtu) && @iface.mtu
network_options.push("--mtu=#{@iface.mtu}")
end

# IPv4
if subnet4
if !subnet4.dhcp_boot_mode? || @static
network_options.push("--bootproto static")
network_options.push("--ip=#{@iface.ip}")
network_options.push("--netmask=#{subnet4.mask}")
network_options.push("--gateway=#{subnet4.gateway}")
elsif subnet4.dhcp_boot_mode?
network_options.push("--bootproto dhcp")
end

nameservers.concat(subnet4.dns_servers)
end

# IPv6
if subnet6
if !subnet6.dhcp_boot_mode? || @static6
network_options.push("--ipv6=#{@iface.ip6}/#{subnet6.cidr}")
network_options.push("--ipv6gateway=#{subnet6.gateway}")
elsif subnet6.dhcp_boot_mode?
if @use_slaac
network_options.push("--ipv6 auto")
else
network_options.push("--ipv6 dhcp")
end
end

nameservers.concat(subnet6.dns_servers)
end

# bond
if @iface.bond?
bond_slaves = @iface.attached_devices_identifiers.join(',')
network_options.push("--bondslaves=#{bond_slaves}")
network_options.push("--bondopts=mode=#{@iface.mode},#{@iface.bond_options.tr(' ', ',')}")
end

# bridge
if @iface.bridge?
bridge_slaves = @iface.attached_devices_identifiers.join(',')
network_options.push("--bridgeslaves=#{bridge_slaves}")
# Currently no support for bridge options in the interface.
stejskalleos marked this conversation as resolved.
Show resolved Hide resolved
end

# VLAN (only on physical is recognized)
if @iface.virtual? && @iface.tag.present? && @iface.attached_to.present?
stejskalleos marked this conversation as resolved.
Show resolved Hide resolved
network_options.push("--vlanid=#{@iface.tag}")
network_options.push("--interfacename=vlan#{@iface.tag}") if (is_fedora && os_major >= 21) || (rhel_compatible && os_major >= 7)
end

# DNS
if nameservers.empty?
network_options.push("--nodns")
else
network_options.push("--nameserver=#{nameservers.join(',')}")
end

# Search domain - available from Fedora 39 (RHEL 10)
if @iface.domain && ((is_fedora && os_major >= 39) || (rhel_compatible && os_major >= 10))
network_options.push("--ipv4-dns-search=#{@iface.domain}") if subnet4
network_options.push("--ipv6-dns-search=#{@iface.domain}") if subnet6
end
-%>
<%= "network #{network_options.join(' ')}\n" -%>
<% end -%>
10 changes: 7 additions & 3 deletions config/initializers/safemode_jail.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# Permit safemode template rendering to have basic read-only access over
# model relations
class ActiveRecord::AssociationRelation::Jail < Safemode::Jail
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids, :sort_by, :select, :reject
end

class ActiveRecord::Relation::Jail < Safemode::Jail
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids, :select, :reject
end

class ActiveRecord::Associations::CollectionProxy::Jail < Safemode::Jail
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids
allow :[], :each, :first, :to_a, :map, :find_in_batches, :size, :group_by, :ids, :sort_by, :select, :reject
end

class ActiveRecord::Batches::BatchEnumerator::Jail < Safemode::Jail
Expand All @@ -23,3 +23,7 @@ class URI::Generic::Jail < Safemode::Jail
class ActiveSupport::TimeWithZone::Jail < Safemode::Jail
allow(*Safemode.core_jail_methods(Time).uniq)
end

class Array::Jail < Safemode::Jail
allow :sort_by, :select, :reject
end
4 changes: 0 additions & 4 deletions ...enderer/snapshots/ProvisioningTemplate/finish/Kickstart_default_finish.host4dhcp.snap.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@



service network restart






Expand Down
Loading