Fixes #21691 - clean installed packages on upgrade

app/models/katello/concerns/host_managed_extensions.rb

@@ -65,13 +65,7 @@ def content_and_puppet_match?
       end
 
       def import_package_profile(simple_packages)
-        if Setting[:bulk_query_installed_packages]
-          #this method is slow if the clean_installed_packages rake task has not been run, but faster if it has
-          found = import_package_profile_in_bulk(simple_packages)
-        else
-          found = import_package_profile_individually(simple_packages)
-        end
-
+        found = import_package_profile_in_bulk(simple_packages)
         sync_package_associations(found.map(&:id))
       end
 
@@ -89,21 +83,6 @@ def import_package_profile_in_bulk(simple_packages)
         found
       end
 
-      def import_package_profile_individually(simple_packages)
-        found = []
-        simple_packages.each do |simple_package|
-          ::Katello::Util::Support.active_record_retry do
-            #use limit(1)[0] here to avoid a sort by id
-            pkg = InstalledPackage.where(:nvra => simple_package.nvra, :name => simple_package.name).limit(1)[0]
-            if pkg.nil?
-              pkg = InstalledPackage.create!(:nvra => simple_package.nvra, :name => simple_package.name)
-            end
-            found << pkg
-          end
-        end
-        found
-      end
-
       def sync_package_associations(new_installed_package_ids)
         old_associated_ids = self.installed_package_ids
         table_name = self.host_installed_packages.table_name

app/models/setting/content.rb

@@ -40,8 +40,6 @@ def self.load_defaults
                  false, N_('Restrict Composite Content View promotion')),
         self.set('check_services_before_actions', N_("Whether or not to check the status of backend services such as pulp and candlepin prior to performing some actions."),
                  true, N_('Check services before actions')),
-        self.set('bulk_query_installed_packages', N_("Use a bulk query when looking up installed packages."),
-                 false, N_('Bulk query for installed packages.')),
         self.set('force_post_sync_actions', N_("Force post sync actions such as indexing and email even if no content was available."),
                  false, N_('Force post-sync actions')),
         self.set('default_download_policy', N_("Default download policy for repositories (either 'immediate', 'on_demand', or 'background')"), "on_demand",

db/migrate/20171114150937_cleanup_installed_packages.rb

@@ -0,0 +1,55 @@
+class CleanupInstalledPackages < ActiveRecord::Migration
+  def up
+    create_table "katello_installed_packages_new", force: :cascade do |t|
+      t.string "name", limit: 255, null: false
+      t.string "nvra", limit: 255, null: false
+    end
+
+    create_table "katello_host_installed_packages_new", force: :cascade do |t|
+      t.integer "host_id",              null: false
+      t.integer "installed_package_id", null: false
+    end
+
+    #Copy unique entires to new table
+    execute('insert into katello_installed_packages_new(nvra, name)
+       select distinct(nvra) as nvra, name from katello_installed_packages')
+
+    #copy associations
+    execute('
+    insert into katello_host_installed_packages_new (host_id, installed_package_id)
+      select distinct katello_host_installed_packages.host_id, katello_installed_packages_new.id
+        from katello_installed_packages
+        inner join katello_host_installed_packages on  katello_host_installed_packages.installed_package_id = katello_installed_packages.id
+        inner join  katello_installed_packages_new on katello_installed_packages_new.nvra = katello_installed_packages.nvra')
+
+    remove_foreign_key :katello_host_installed_packages, name: "katello_host_installed_packages_installed_package_id"
+    remove_foreign_key :katello_host_installed_packages, name: "katello_host_installed_packages_host_id"
+
+    drop_table "katello_installed_packages"
+    drop_table "katello_host_installed_packages"
+
+    rename_table "katello_installed_packages_new", "katello_installed_packages"
+    rename_table "katello_host_installed_packages_new", "katello_host_installed_packages"
+
+    add_index :katello_installed_packages, [:name, :nvra]
+
+    add_foreign_key "katello_host_installed_packages", "hosts",
+                    :name => "katello_host_installed_packages_host_id", :column => "host_id"
+
+    add_foreign_key "katello_host_installed_packages", "katello_installed_packages",
+                    :name => "katello_host_installed_packages_installed_package_id", :column => "installed_package_id"
+
+    #At this point, everything should be back to where it was (sans-duplicates)
+    # Now do new things
+    add_index :katello_installed_packages, [:nvra], :unique => true
+    add_index :katello_host_installed_packages, [:host_id, :installed_package_id], :unique => true, :name => :katello_host_installed_packages_h_id_ip_id
+    Setting.where(:name => 'bulk_query_installed_packages', :category => 'Setting::Content').delete_all
+  end
+
+  def down
+    #only revert new things
+    remove_index :katello_installed_packages, [:nvra]
+    remove_index :katello_host_installed_packages, :name => :katello_host_installed_packages_h_id_ip_id
+    #Setting will be recreated on startup
+  end
+end

test/models/concerns/host_managed_extensions_test.rb

@@ -127,7 +127,6 @@ def test_non_matching_puppet_environment
   class HostInstalledPackagesTest < HostManagedExtensionsTestBase
     def setup
       super
-      Setting[:bulk_query_installed_packages] = false
       package_json = {:name => "foo", :version => "1", :release => "1.el7", :arch => "x86_64"}
       @foreman_host.import_package_profile([::Katello::Pulp::SimplePackage.new(package_json)])
       @nvra = 'foo-1-1.el7.x86_64'
@@ -140,15 +139,7 @@ def test_installed_packages
       assert_equal @nvra, @foreman_host.installed_packages.first.nvra
     end
 
-    def test_import_package_profile_adds_removes
-      package_json = {:name => "betterfoo", :version => "1", :release => "1.el7", :arch => "x86_64"}
-      @foreman_host.import_package_profile([::Katello::Pulp::SimplePackage.new(package_json)])
-      assert_equal 1, @foreman_host.installed_packages.count
-      assert_equal 'betterfoo', @foreman_host.installed_packages.first.name
-    end
-
     def test_import_package_profile_adds_removes_bulk
-      Setting[:bulk_query_installed_packages] = true
       packages = [::Katello::Pulp::SimplePackage.new(:name => "betterfoo", :version => "1", :release => "1.el7", :arch => "x86_64")]
       @foreman_host.import_package_profile(packages)
       assert_equal 1, @foreman_host.installed_packages.count