Cherry picks for Foreman 3.8.0 GA

app/models/host/base.rb

@@ -214,19 +214,32 @@ def set_comment(parser)
       desc 'Note that available facts depend on what facts have been uploaded to Foreman,
            typical sources are Puppet facter, subscription manager etc.
            The facts can be out of date, this macro only provides access to the value stored in the database.'
+      optional :fact_names, Array, desc: 'A list of fact names to return. If empty all facts are returned'
       returns Hash, desc: 'A hash of facts, keys are fact names, values are fact values'
       example '@host.facts # => { "hardwareisa"=>"x86_64", "kernel"=>"Linux", "virtual"=>"physical", ... }', desc: 'Getting all host facts'
       example '@host.facts["uptime"] # => "30 days"', desc: 'Getting specific fact value, +uptime+ in this case'
       aliases :facts
     end
-    def facts_hash
-      hash = {}
-      fact_values.includes(:fact_name).collect do |fact|
-        hash[fact.fact_name.name] = fact.value
+    def facts_hash(*fact_names)
+      # keep it to one SQL query
+      query = if fact_names.present?
+                fact_values.joins(:fact_name).where(fact_names: {name: fact_names}).pluck('fact_names.name', :value)
+              else
+                fact_values.joins(:fact_name).pluck('fact_names.name', :value)
+              end
+      query.to_h # { fact_name.name => fact_value.value}
+    end
+
+    def facts(*fact_names)
+      if fact_names.blank?
+        Rails.cache.fetch("hosts/#{id}/facts", :expires_in => 1.minute) do
+          Rails.logger.debug "Caching facts for #{name}"
+          facts_hash
+        end
+      else
+        facts_hash(*fact_names)
       end
-      hash
     end
-    alias_method :facts, :facts_hash
 
     def ==(comparison_object)
       super ||

app/views/subnets/index.html.erb

@@ -24,7 +24,14 @@
         <td class="ellipsis"><%= subnet.domains.map(&:name).to_sentence %></td>
         <td><%= subnet.vlanid %></td>
         <td class="ellipsis"><%= subnet.dhcp %></td>
-        <td><%= link_to_if_authorized(hosts_count[subnet], hash_for_hosts_path(:search => "subnet.name=\"#{subnet}\"")) %>
+        <td>
+          <%=
+            # TODO: https://projects.theforeman.org/issues/36517 this only works for IPv4
+            count = hosts_count[subnet]
+            search_key = subnet.type == 'Subnet::Ipv6' ? 'subnet6.name' : 'subnet.name'
+            link_to_if_authorized(count, hash_for_hosts_path(:search => "#{search_key}=\"#{subnet}\""))
+          %>
+        </td>
         <td class="col-md-1">
           <%= action_buttons(display_delete_if_authorized(
                                hash_for_subnet_path(:id => subnet).

app/views/taxonomies/_form.html.erb

@@ -22,7 +22,7 @@
       <% if User.current.allowed_to?(:view_provisioning_templates) %>
         <li><a href="#provisioning_templates" data-toggle="tab"><%= _("Provisioning Templates") %></a></li>
       <% end %>
-      <% if User.current.allowed_to?(:view_ptales) %>
+      <% if User.current.allowed_to?(:view_ptables) %>
         <li><a href="#ptables" data-toggle="tab"><%= _("Partition Tables") %></a></li>
       <% end %>
       <% if User.current.allowed_to?(:view_domains) %>

app/views/unattended/provisioning_templates/snippet/kickstart_kernel_options.erb

@@ -126,7 +126,7 @@ description: |
     if (is_fedora && os_major < 17) || (rhel_compatible && os_major < 7)
       options.push("vlanid=#{iface.tag}")
     else
-      options.push("vlan=vlan#{iface.tag}:#{iface.attached_to}")
+      options.push("vlan=#{iface.attached_to}.#{iface.tag}:#{iface.attached_to}")
     end
   end
 

app/views/unattended/provisioning_templates/snippet/redhat_register.erb

@@ -8,9 +8,6 @@ description: |
 
   General parameters:
 
-    redhat_install_agent = [true|false]         Install the management agent.  For Spacewalk,
-                                                this is OSAD.  For Katello, this is katello-agent.
-
     redhat_install_host_tools = [true|false]    Install the katello-host-tools yum/dnf plugins.
 
     redhat_install_host_tracer_tools = [true|false]  Install the katello-host-tools Tracer yum/dnf plugin.
@@ -82,8 +79,6 @@ description: |
 <% if registration_type == 'subscription_manager' %>
   <%
     atomic = @host.operatingsystem.respond_to?(:atomic) ? @host.operatingsystem.atomic? : host_param_true?('atomic')
-    redhat_install_agent_fallback = plugin_present?('katello') && katello_agent_enabled?
-    redhat_install_agent = host_param_true?('redhat_install_agent', redhat_install_agent_fallback)
 
     if host_param('kt_activation_keys')
       subscription_manager_certpkg_url = subscription_manager_configuration_url(@host)
@@ -217,17 +212,15 @@ description: |
   <% end %>
 
   <% if !atomic %>
-    <% if redhat_install_agent || redhat_install_host_tools || redhat_install_host_tracer_tools %>
+    <% if redhat_install_host_tools || redhat_install_host_tracer_tools %>
        if [ -f /usr/bin/dnf ]; then
          PACKAGE_MAN="dnf -y"
        else
          PACKAGE_MAN="yum -t -y"
        fi
     <% end %>
 
-    <% if redhat_install_agent %>
-      $PACKAGE_MAN install katello-agent
-    <% elsif redhat_install_host_tools %>
+    <% if redhat_install_host_tools %>
       $PACKAGE_MAN install katello-host-tools
     <% end %>
 
@@ -293,13 +286,6 @@ description: |
         echo "registration successful."
     fi
 
-    <% if host_param_true?('redhat_install_agent') %>
-    if [ -f /usr/bin/dnf ]; then
-      dnf -y install osad
-    else
-      yum -t -y install osad
-    fi
-    <% end %>
   <% else %>
     echo "No activation key found: Not registering"
   <% end %>

lib/foreman.rb

@@ -23,7 +23,7 @@ def self.in_rake?(*rake_tasks)
   end
 
   def self.in_setup_db_rake?
-    in_rake?('db:create', 'db:migrate', 'db:drop')
+    in_rake?('db:create', 'db:migrate', 'db:drop', 'db:abort_if_pending_migrations')
   end
 
   def self.pending_migrations?

test/models/host_test.rb

@@ -1684,6 +1684,54 @@ class HostTest < ActiveSupport::TestCase
     assert_equal hosts.count, 0
   end
 
+  test "can build a hash of all host facts" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 22)
+    assert_equal host.facts.keys.length, 22
+  end
+
+  test "can build a hash of specific host facts" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 5)
+    assert_equal host.facts(host.fact_names.map(&:name).first(3)).keys.length, 3
+  end
+
+  test "makes only one SQL query when you pass specific fact names" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 5)
+    fact_names = host.fact_names.map(&:name).first(3)
+    assert_sql_queries(1) do
+      host.facts_hash(fact_names)
+    end
+  end
+
+  test "makes one SQL query per host.facts_hash" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 5)
+    fact_names = host.fact_names.map(&:name).first(2)
+    assert_sql_queries(2) do
+      host.facts_hash[fact_names[0]]
+      host.facts_hash[fact_names[1]]
+    end
+  end
+
+  test "facts caches facts_hash" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 5)
+    fact_names = host.fact_names.map(&:name).first(2)
+    assert_sql_queries(1) do
+      host.facts[fact_names[0]]
+      host.facts[fact_names[1]]
+      host.facts[fact_names[0]]
+      host.facts[fact_names[1]]
+    end
+  end
+
+  test "facts does not cache if you pass fact names" do
+    host = FactoryBot.create(:host, :with_facts, :fact_count => 5)
+    fact_names = host.fact_names.map(&:name).first(2)
+    assert_sql_queries(3) do
+      host.facts(fact_names)
+      host.facts(fact_names)
+      host.facts(fact_names)
+    end
+  end
+
   test "can search hosts by numeric and string facts" do
     host = FactoryBot.create(:host, :hostname => 'num001.example.com')
     HostFactImporter.new(host).import_facts({

test/test_helper.rb

@@ -119,6 +119,16 @@ def after_teardown
     Foreman::Plugin.send(:clear, @plugins_backup, @registries_backup)
     @clear_plugins = nil
   end
+
+  def assert_sql_queries(num_of_queries, match = /SELECT/)
+    queries = []
+    ActiveSupport::Notifications.subscribe("sql.active_record") do |_name, _start, _finish, _id, payload|
+      queries << payload[:sql] if payload[:sql] =~ match
+    end
+    yield
+    ActiveSupport::Notifications.unsubscribe("sql.active_record")
+    assert_equal num_of_queries, queries.size, "Expected #{num_of_queries} queries, but got #{queries.size}"
+  end
 end
 
 class ActionView::TestCase