From fb9ac4b98e3b8e72dab7b0d7d893dc78639f1fab Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 9 Jan 2019 13:52:55 -0600 Subject: [PATCH] Add cache columns to operating_systems table Both platform and image_name are relatively stable values that get re-calculated every time they are accessed, and in the worst case, causes it to traverse through many different sources to try and find a proper `platform`/`image_name`. This is compounded when you are trying to fetch thousands of records at once. This adds the columns to be cache store for those values, and should be triggered on every update of those records, and related records (not addressed in this patch). --- ...411_add_image_name_to_operating_systems.rb | 157 +++++++++++++++++ ...dd_image_name_to_operating_systems_spec.rb | 159 ++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 db/migrate/20190108031411_add_image_name_to_operating_systems.rb create mode 100644 spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb diff --git a/db/migrate/20190108031411_add_image_name_to_operating_systems.rb b/db/migrate/20190108031411_add_image_name_to_operating_systems.rb new file mode 100644 index 000000000..7461b92f5 --- /dev/null +++ b/db/migrate/20190108031411_add_image_name_to_operating_systems.rb @@ -0,0 +1,157 @@ +class AddImageNameToOperatingSystems < ActiveRecord::Migration[5.0] + include MigrationHelper + + StubOperatingSystemObj = Struct.new(:operating_system) do + attr_reader :hardware # always nil + def name + "" + end + end + + class OperatingSystem < ActiveRecord::Base + belongs_to :host + belongs_to :vm_or_template + belongs_to :computer_system + + # Using a `before_save` here since this is the mechanism that will be used + # in the app. Causes a bit of issues in the specs, but proves that this + # would work moving forward. + before_save :update_platform_and_image_name + + def update_platform_and_image_name + obj = case + when host_id then host + when vm_or_template_id then vm_or_template + when computer_system_id then computer_system + else + StubOperatingSystemObj.new(self) + end + + if obj + self.image_name = self.class.image_name(obj) + self.platform = self.image_name.split("_").first + end + end + + @@os_map = [ # rubocop:disable Style/ClassVars + ["windows_generic", %w(winnetenterprise w2k3 win2k3 server2003 winnetstandard servernt)], + ["windows_generic", %w(winxppro winxp)], + ["windows_generic", %w(vista longhorn)], + ["windows_generic", %w(win2k win2000)], + ["windows_generic", %w(microsoft windows winnt)], + ["linux_ubuntu", %w(ubuntu)], + ["linux_chrome", %w(chromeos)], + ["linux_chromium", %w(chromiumos)], + ["linux_suse", %w(suse sles)], + ["linux_redhat", %w(redhat rhel)], + ["linux_fedora", %w(fedora)], + ["linux_gentoo", %w(gentoo)], + ["linux_centos", %w(centos)], + ["linux_debian", %w(debian)], + ["linux_coreos", %w(coreos)], + ["linux_esx", %w(vmnixx86 vmwareesxserver esxserver vmwareesxi)], + ["linux_solaris", %w(solaris)], + ["linux_generic", %w(linux)] + ] + + # rubocop:disable Naming/VariableName + def self.normalize_os_name(osName) + findStr = osName.downcase.gsub(/[^a-z0-9]/, "") + @@os_map.each do |a| + a[1].each do |n| + return a[0] unless findStr.index(n).nil? + end + end + "unknown" + end + # rubocop:enable Naming/VariableName + + # rubocop:disable Naming/VariableName + def self.image_name(obj) + osName = nil + + # Select most accurate name field + os = obj.operating_system + if os + # check the given field names for possible matching value + osName = [:distribution, :product_type, :product_name].each do |field| # rubocop:disable Style/SymbolArray + os_field = os.send(field) + break(os_field) if os_field && OperatingSystem.normalize_os_name(os_field) != "unknown" + end + + # If the normalized name comes back as unknown, nil out the value so we can get it from another field + if osName.kind_of?(String) + osName = nil if OperatingSystem.normalize_os_name(osName) == "unknown" + else + osName = nil + end + end + + # If the OS Name is still blank check the 'user_assigned_os' + if osName.nil? && obj.respond_to?(:user_assigned_os) && obj.user_assigned_os + osName = obj.user_assigned_os + end + + # If the OS Name is still blank check the hardware table + if osName.nil? && obj.hardware && !obj.hardware.guest_os.nil? + osName = obj.hardware.guest_os + # if we get generic linux or unknown back see if the vm name is better + norm_os = OperatingSystem.normalize_os_name(osName) + if norm_os == "linux_generic" || norm_os == "unknown" # rubocop:disable Style/MultipleComparison + vm_name = OperatingSystem.normalize_os_name(obj.name) + return vm_name unless vm_name == "unknown" + end + end + + # If the OS Name is still blank use the name field from the object given + osName = obj.name if osName.nil? && obj.respond_to?(:name) + + # Normalize name to match existing icons + OperatingSystem.normalize_os_name(osName || "") + end + # rubocop:enable Naming/VariableName + end + + class VmOrTemplate < ActiveRecord::Base + self.inheritance_column = :_type_disabled + self.table_name = 'vms' + + has_one :operating_system + has_one :hardware + end + + class Host < ActiveRecord::Base + self.inheritance_column = :_type_disabled + + has_one :operating_system + has_one :hardware + end + + class ComputerSystem < ActiveRecord::Base + has_one :operating_system + has_one :hardware + end + + class Hardware < ActiveRecord::Base + end + + def up + add_column :operating_systems, :platform, :string + add_column :operating_systems, :image_name, :string + + say_with_time("Updating platform and image_name in OperatingSystems") do + base_relation = OperatingSystem.all + say_batch_started(base_relation.size) + + base_relation.find_in_batches do |group| + group.each(&:save) + say_batch_processed(group.count) + end + end + end + + def down + remove_column :operating_systems, :platform, :string + remove_column :operating_systems, :image_name, :string + end +end diff --git a/spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb b/spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb new file mode 100644 index 000000000..88e0304cc --- /dev/null +++ b/spec/migrations/20190108031411_add_image_name_to_operating_systems_spec.rb @@ -0,0 +1,159 @@ +require_migration + +describe AddImageNameToOperatingSystems do + let(:host_stub) { migration_stub(:Host) } + let(:hardware_stub) { migration_stub(:Hardware) } + let(:vm_or_template_stub) { migration_stub(:VmOrTemplate) } + let(:computer_system_stub) { migration_stub(:ComputerSystem) } + let(:operating_system_stub) { migration_stub(:OperatingSystem) } + + # rubocop:disable Layout/SpaceInsideArrayPercentLiteral + let(:test_os_values) do + [ + %w(an_amazing_undiscovered_os unknown), + %w(centos-7 linux_centos), + %w(debian-8 linux_debian), + %w(opensuse-13 linux_suse), + %w(sles-12 linux_suse), + %w(rhel-7 linux_redhat), + %w(ubuntu-15-10 linux_ubuntu), + %w(windows-2012-r2 windows_generic), + %w(vmnix-x86 linux_esx), + %w(vista windows_generic), + %w(coreos-cloud linux_coreos) + ] + end + # rubocop:enable Layout/SpaceInsideArrayPercentLiteral + + def record_with_os(klass, os_attributes = nil, record_attributes = {:name => ""}, hardware_attributes = nil) + os_record = operating_system_stub.new(os_attributes) if os_attributes + + if klass == operating_system_stub + os_record.save! + os_record + else + record = klass.new + record_attributes.each do |attr, val| + record.send("#{attr}=", val) if record.respond_to?(attr) + end + + record.operating_system = os_record + record.hardware = hardware_stub.new(hardware_attributes) if hardware_attributes + record.save! + record + end + end + + # Runs tests for class type to confirm they + def test_for_klass(klass) + begin + # This callback is necessary after the migration, but fails when the + # column doesn't eixst (prior to the migration). Removing it and + # re-enabling it after the migration. + operating_system_stub.skip_callback(:save, :before, :update_platform_and_image_name) + + distribution_based = [] + product_type_based = [] + product_name_based = [] + fallback_records = [] + + test_os_values.each do |(value, _)| + distribution_based << record_with_os(klass, :distribution => value) + product_type_based << record_with_os(klass, :product_type => value) + product_name_based << record_with_os(klass, :product_name => value) + end + + # favor distribution over product_type + fallback_records << record_with_os(klass, :distribution => "rhel-7", :product_type => "centos-7") + # falls back to os.product_type if invalid os.distribution + fallback_records << record_with_os(klass, :distribution => "undiscovered-7", :product_type => "rhel-7") + # falls back to os.product_name + fallback_records << record_with_os(klass, :distribution => "undiscovered-7", :product_name => "rhel-7") + # falls back to hardware.guest_os + fallback_records << record_with_os(klass, {:distribution => "undiscovered-7"}, {}, {:guest_os => "rhel-7"}) + # falls back to Host#user_assigned_os + fallback_records << record_with_os(klass, {:distribution => "undiscovered-7"}, {:user_assigned_os => "rhel-7"}) + ensure + # If the any of the above fails, make sure we re-enable callbacks so + # subsequent specs don't fail trying to skip this callback when it + # doesn't exist. + operating_system_stub.set_callback(:save, :before, :update_platform_and_image_name) + end + + migrate + + test_os_values.each.with_index do |(_, image_name), index| + [distribution_based, product_type_based, product_name_based].each do |record_list| + os_record = record_list[index] + os_record.reload + os_record = os_record.operating_system if os_record.respond_to?(:operating_system) + + expect(os_record.image_name).to eq(image_name) + expect(os_record.platform).to eq(image_name.split("_").first) + end + end + + fallback_records.each(&:reload) + + platform, image_name = %w(linux linux_redhat) + fallback_records.each.with_index do |record, index| + os_record = record + os_record = os_record.operating_system if os_record.respond_to?(:operating_system) + + # OperatingSystem records don't have a hardware relation, so this will be + # a "unknown" OS + platform, image_name = %w(unknown unknown) if index == 3 && klass == operating_system_stub + + # Both ComputerSystem and VmOrTemplate don't have :user_assigned_os, so + # these will return "unknown" instead of what we (tried to) set. + platform, image_name = %w(unknown unknown) if index == 4 && klass != host_stub + + expect(os_record.image_name).to eq(image_name) + expect(os_record.platform).to eq(platform) + end + end + + migration_context :up do + it "adds the columns" do + before_columns = operating_system_stub.columns.map(&:name) + expect(before_columns).to_not include("platform") + expect(before_columns).to_not include("image_name") + + migrate + + after_columns = operating_system_stub.columns.map(&:name) + expect(after_columns).to include("platform") + expect(after_columns).to include("image_name") + end + + it "updates OperatingSystem for Host records" do + test_for_klass host_stub + end + + it "updates OperatingSystem for VmOrTemplate records" do + test_for_klass vm_or_template_stub + end + + it "updates OperatingSystem for ComputerSystem records" do + test_for_klass computer_system_stub + end + + it "updates orphaned OperatingSystem records" do + test_for_klass operating_system_stub + end + end + + migration_context :down do + it "adds the columns" do + before_columns = operating_system_stub.columns.map(&:name) + expect(before_columns).to include("platform") + expect(before_columns).to include("image_name") + + migrate + + after_columns = operating_system_stub.columns.map(&:name) + expect(after_columns).to_not include("platform") + expect(after_columns).to_not include("image_name") + end + end +end