Merge pull request #19758 from carbonin/remove_settings_last_loaded

Remove Vmdb::Settings last_loaded and simplify server settings reload

app/models/miq_server/configuration_management.rb

@@ -10,7 +10,12 @@ def reload_settings
     return if is_remote?
 
     Vmdb::Settings.reload!
+
     activate_settings_for_appliance
+    sync_config
+    sync_assigned_roles
+    reset_queue_messages
+    update_sync_timestamp(Time.now.utc)
   end
 
   # The purpose of this method is to do special activation of things
@@ -51,21 +56,8 @@ def config_activated(data)
   end
 
   def sync_config
-    @config_last_loaded = Vmdb::Settings.last_loaded
-    sync_log_level
     sync_worker_monitor_settings
     sync_child_worker_settings
     $log.log_hashes(@worker_monitor_settings)
   end
-
-  def sync_config_changed?
-    stale = @config_last_loaded != Vmdb::Settings.last_loaded
-    @config_last_loaded = Vmdb::Settings.last_loaded if stale
-    stale
-  end
-
-  def sync_log_level
-    # TODO: Can this be removed since the Vmdb::Settings::Activator will do this anyway?
-    Vmdb::Loggers.apply_config(::Settings.log)
-  end
 end

app/models/miq_server/worker_management/monitor.rb

@@ -124,17 +124,12 @@ def sync_monitor
     @last_sync ||= Time.now.utc
     sync_interval         = @worker_monitor_settings[:sync_interval] || 30.minutes
     sync_interval_reached = sync_interval.seconds.ago.utc > @last_sync
-    config_changed        = self.sync_config_changed?
     roles_changed         = self.active_roles_changed?
-    resync_needed         = config_changed || roles_changed || sync_interval_reached
+    resync_needed         = roles_changed || sync_interval_reached
 
     roles_added, roles_deleted, _roles_unchanged = role_changes
 
     if resync_needed
-      @last_sync = Time.now.utc
-
-      sync_config                if config_changed
-      sync_assigned_roles        if config_changed
       log_role_changes           if roles_changed
       sync_active_roles          if roles_changed
       set_active_role_flags      if roles_changed
@@ -144,8 +139,9 @@ def sync_monitor
 
       EvmDatabase.restart_failover_monitor_service if (roles_added | roles_deleted).include?("database_operations")
 
-      reset_queue_messages       if config_changed || roles_changed
+      reset_queue_messages       if roles_changed
 
+      @last_sync = Time.now.utc
       update_sync_timestamp(@last_sync)
     end
   end

app/models/miq_worker/runner.rb

@@ -246,7 +246,6 @@ def sync_config
     # Sync settings
     Vmdb::Settings.reload!
     @my_zone ||= MiqServer.my_zone
-    sync_log_level
     sync_worker_settings
     sync_blacklisted_events
     after_sync_config
@@ -259,11 +258,6 @@ def sync_config
     @worker.release_db_connection if @worker.respond_to?(:release_db_connection)
   end
 
-  def sync_log_level
-    # TODO: Can this be removed since the Vmdb::Settings::Activator will do this anyway?
-    Vmdb::Loggers.apply_config(::Settings.log)
-  end
-
   def sync_worker_settings
     @worker_settings = self.class.corresponding_model.worker_settings(:config => ::Settings.to_hash)
     @poll = @worker_settings[:poll]

lib/patches/config_patch.rb

@@ -3,7 +3,7 @@ def reload!
     Vmdb::Settings.decrypt_passwords!(super).tap do
       # The following should only be done when loading/reloading the current
       #   process' Settings, as opposed to a remote server's settings.
-      Vmdb::Settings.on_reload if equal?(::Settings)
+      Vmdb::Settings.dump_to_log_directory(::Settings) if equal?(::Settings)
     end
   end
 

lib/vmdb/settings.rb

@@ -25,17 +25,10 @@ def initialize(errors)
     RESET_COMMAND = "<<reset>>".freeze
     RESET_VALUE = HashDiffer::MissingKey
 
-    cattr_accessor :last_loaded
-
     def self.init
       ::Config.overwrite_arrays = true
       ::Config.merge_nil_values = false
       reset_settings_constant(for_resource(:my_server))
-      on_reload
-    end
-
-    def self.on_reload
-      self.last_loaded = Time.now.utc
       dump_to_log_directory(::Settings)
     end
 

spec/lib/vmdb/settings_spec.rb

@@ -1,37 +1,25 @@
 RSpec.describe Vmdb::Settings do
-  describe ".on_reload" do
+  describe ".dump_to_log_directory" do
     it "is called on top-level ::Settings.reload!" do
-      expect(described_class).to receive(:on_reload)
+      expect(described_class).to receive(:dump_to_log_directory)
 
       ::Settings.reload!
     end
 
-    it "updates the last_loaded time" do
-      Timecop.freeze(Time.now.utc) do
-        expect(described_class.last_loaded).to_not eq(Time.now.utc)
+    it "writes them" do
+      ::Settings.api.token_ttl = "1.minute"
+      described_class.dump_to_log_directory(::Settings)
 
-        described_class.on_reload
-
-        expect(described_class.last_loaded).to eq(Time.now.utc)
-      end
+      dumped_yaml = YAML.load_file(described_class::DUMP_LOG_FILE)
+      expect(dumped_yaml.fetch_path(:api, :token_ttl)).to eq "1.minute"
     end
 
-    context "dumping the settings to the log directory" do
-      it "writes them" do
-        ::Settings.api.token_ttl = "1.minute"
-        described_class.on_reload
-
-        dumped_yaml = YAML.load_file(described_class::DUMP_LOG_FILE)
-        expect(dumped_yaml.fetch_path(:api, :token_ttl)).to eq "1.minute"
-      end
-
-      it "masks passwords" do
-        ::Settings.authentication.bind_pwd = "pa$$w0rd"
-        described_class.on_reload
+    it "masks passwords" do
+      ::Settings.authentication.bind_pwd = "pa$$w0rd"
+      described_class.dump_to_log_directory(::Settings)
 
-        dumped_yaml = YAML.load_file(described_class::DUMP_LOG_FILE)
-        expect(dumped_yaml.fetch_path(:authentication, :bind_pwd)).to eq "********"
-      end
+      dumped_yaml = YAML.load_file(described_class::DUMP_LOG_FILE)
+      expect(dumped_yaml.fetch_path(:authentication, :bind_pwd)).to eq "********"
     end
   end
 

spec/models/miq_server/configuration_management_spec.rb

@@ -37,11 +37,12 @@
   end
 
   describe "#reload_settings" do
-    let(:miq_server) { EvmSpecHelper.local_miq_server }
+    let(:miq_server) { EvmSpecHelper.local_miq_server.tap(&:setup_drb_variables) }
 
     it "reloads the new changes into the settings for the resource" do
       Vmdb::Settings.save!(miq_server, :some_test_setting => 2)
       expect(Settings.some_test_setting).to be_nil
+      expect(miq_server).to receive(:update_sync_timestamp)
 
       miq_server.reload_settings
 

spec/support/evm_spec_helper.rb

@@ -30,8 +30,7 @@ def self.assign_embedded_ansible_role(miq_server = nil)
 
   # Clear all EVM caches
   def self.clear_caches
-    @settings_loaded = Vmdb::Settings.last_loaded
-
+    settings_digest = Digest::MD5.hexdigest(::Settings.to_json)
     yield if block_given?
   ensure
     Module.clear_all_cache_with_timeout if Module.respond_to?(:clear_all_cache_with_timeout)
@@ -47,7 +46,7 @@ def self.clear_caches
     # Clear the thread local variable to prevent test contamination
     User.current_user = nil if defined?(User) && User.respond_to?(:current_user=)
 
-    ::Settings.reload! if @settings_loaded != Vmdb::Settings.last_loaded
+    ::Settings.reload! if Digest::MD5.hexdigest(::Settings.to_json) != settings_digest
   end
 
   def self.clear_instance_variables(instance)