Skip to content

Commit

Permalink
Merge pull request #18024 from xlab-si/prepare-compat-for-vmdb-config…
Browse files Browse the repository at this point in the history
…-removal

Prepare compat for vmdb config removal
  • Loading branch information
carbonin authored Sep 26, 2018
2 parents dc7975f + 6f3ec65 commit c901870
Show file tree
Hide file tree
Showing 14 changed files with 90 additions and 52 deletions.
10 changes: 7 additions & 3 deletions app/models/miq_server/configuration_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ module MiqServer::ConfigurationManagement
extend ActiveSupport::Concern
include ConfigurationManagementMixin

def settings
(is_local? ? ::Settings : settings_for_resource).to_hash
end

def get_config(type = "vmdb")
if is_local?
VMDB::Config.new(type)
Expand All @@ -24,7 +28,7 @@ def reload_settings

# The purpose of this method is to do special activation of things
# that can only happen once per server. Normally, the
# VMDB::Config::Activator would be used, however the activations
# Vmdb::Settings::Activator would be used, however the activations
# will end up occurring once per worker on the entire server, which
# can be detrimental.
#
Expand All @@ -41,7 +45,7 @@ def servers_for_settings_reload
[self]
end

# Callback from VMDB::Config::Activator#activate when the configuration has
# Callback from Vmdb::Settings::Activator#activate when the configuration has
# changed for this server
def config_activated(data)
# Check that the column exists in the table and we are passed data that does not match
Expand Down Expand Up @@ -80,7 +84,7 @@ def sync_blacklisted_event_names
end

def sync_log_level
# TODO: Can this be removed since the VMDB::Config::Activator will do this anyway?
# TODO: Can this be removed since the Vmdb::Settings::Activator will do this anyway?
Vmdb::Loggers.apply_config(::Settings.log)
end
end
2 changes: 1 addition & 1 deletion app/models/miq_server/ntp_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def ntp_reload

# Bust the settings cache allowing this worker to apply any recent changes made by another (UI) worker
Vmdb::Settings.reload!
ntp_settings = get_config("vmdb").config[:ntp]
ntp_settings = settings[:ntp]

if @ntp_settings && @ntp_settings == ntp_settings
_log.info("Skipping reload of ntp settings since they are unchanged")
Expand Down
3 changes: 1 addition & 2 deletions app/models/miq_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ def self.fetch_worker_settings_from_server(miq_server, options = {})
settings = {}

unless miq_server.nil?
server_config = options[:config] || miq_server.get_config("vmdb")
server_config = server_config.config if server_config.respond_to?(:config)
server_config = options[:config] || miq_server.settings
# Get the configuration values
section = server_config[:workers]
unless section.nil?
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_worker/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def sync_config
end

def sync_log_level
# TODO: Can this be removed since the VMDB::Config::Activator will do this anyway?
# TODO: Can this be removed since the Vmdb::Settings::Activator will do this anyway?
Vmdb::Loggers.apply_config(::Settings.log)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/tasks/evm_settings.rake
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ module EvmSettings

def self.config_import(import_hash)
if import_hash.present?
full_config_hash = MiqServer.my_server.get_config.config
full_config_hash = MiqServer.my_server.settings
MiqServer.my_server.set_config(full_config_hash.deep_merge(import_hash))
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/vmdb/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ def config=(settings)
end

def validate
valid, @errors = Validator.new(self).validate
valid, @errors = Vmdb::Settings::Validator.new(self).validate
valid
end

def activate
Activator.new(self).activate
Vmdb::Settings::Activator.new(self).activate
end

# Get the worker settings as they are in the yaml: 1.seconds, 1, etc.
Expand Down
4 changes: 2 additions & 2 deletions lib/vmdb/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ def self.walk(settings = ::Settings, path = [], &block)
end

def self.activate
VMDB::Config::Activator.new(::Settings).activate
Vmdb::Settings::Activator.new(::Settings).activate
end

def self.validator(settings = ::Settings)
VMDB::Config::Validator.new(settings)
Vmdb::Settings::Validator.new(settings)
end
private_class_method :validator

Expand Down
18 changes: 10 additions & 8 deletions lib/vmdb/config/activator.rb → lib/vmdb/settings/activator.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module VMDB
class Config
module Vmdb
class Settings
class Activator
include Vmdb::Logging

Expand All @@ -18,11 +18,11 @@ def activate
raise "configuration invalid, see errors for details" unless Validator.new(@config).valid?

@config.each_key do |k|
if respond_to?(k.to_s, true)
_log.debug("Activating #{k}")
ost = OpenStruct.new(@config[k].stringify_keys)
send(k.to_s, ost)
end
next unless respond_to?(k.to_s, true)

_log.debug("Activating #{k}")
ost = OpenStruct.new(@config[k].stringify_keys)
send(k.to_s, ost)
end
end

Expand All @@ -38,7 +38,9 @@ def session(data)
end

def server(data)
MiqServer.my_server.config_activated(data) unless MiqServer.my_server.nil? rescue nil
MiqServer.my_server&.config_activated(data)
rescue StandardError
nil
end
end
end
Expand Down
36 changes: 16 additions & 20 deletions lib/vmdb/config/validator.rb → lib/vmdb/settings/validator.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module VMDB
class Config
module Vmdb
class Settings
class Validator
include Vmdb::Logging

Expand All @@ -22,20 +22,16 @@ def validate
@errors = {}
valid = true
@config.each_key do |k|
if respond_to?(k.to_s, true)
_log.debug("Validating #{k}")
ost = OpenStruct.new(@config[k].stringify_keys)
section_valid, errors = send(k.to_s, ost)

unless section_valid
_log.debug(" Invalid: #{errors}")
errors.each do |e|
key, msg = e
@errors[[k, key].join("-")] = msg
end
valid = false
end
end
next unless respond_to?(k.to_s, true)

_log.debug("Validating #{k}")
ost = OpenStruct.new(@config[k].stringify_keys)
section_valid, errors = send(k.to_s, ost)
next if section_valid

_log.debug(" Invalid: #{errors}")
errors.each { |key, msg| @errors["#{k}-#{key}"] = msg }
valid = false
end
return valid, @errors
end
Expand All @@ -47,12 +43,12 @@ def webservices(data)

keys = data.each_pair.to_a.transpose.first.to_set

if keys.include?(:mode) && !["invoke", "disable"].include?(data.mode)
if keys.include?(:mode) && !%w(invoke disable).include?(data.mode)
valid = false
errors << [:mode, "webservices mode, \"#{data.mode}\", invalid. Should be one of: invoke or disable"]
end

if keys.include?(:contactwith) && !["ipaddress", "hostname"].include?(data.contactwith)
if keys.include?(:contactwith) && !%w(ipaddress hostname).include?(data.contactwith)
valid = false
errors << [:contactwith, "webservices contactwith, \"#{data.contactwith}\", invalid. Should be one of: ipaddress or hostname"]
end
Expand Down Expand Up @@ -136,7 +132,7 @@ def server(data)
end
end

if keys.include?(:session_store) && !["sql", "memory", "cache"].include?(data.session_store)
if keys.include?(:session_store) && !%w(sql memory cache).include?(data.session_store)
valid = false
errors << [:session_store, "session_store, \"#{data.session_store}\", invalid. Should be one of \"sql\", \"memory\", \"cache\""]
end
Expand All @@ -156,7 +152,7 @@ def smtp(data)

keys = data.each_pair.to_a.transpose.first.to_set

if keys.include?(:authentication) && !["login", "plain", "none"].include?(data.authentication)
if keys.include?(:authentication) && !%w(login plain none).include?(data.authentication)
valid = false
errors << [:mode, "authentication, \"#{data.mode}\", invalid. Should be one of: login, plain, or none"]
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
describe VMDB::Config::Validator do
describe Vmdb::Settings::Validator do
describe ".new" do
it "with a Hash" do
validator = described_class.new(:session => {:timeout => 123})
Expand Down Expand Up @@ -29,9 +29,9 @@
{:webservices => {:timeout => 123}}, true,
{:webservices => {:timeout => "xxx"}}, false,

{:authentication => {:mode => "ldaps"}}, false,
{:authentication => {:mode => "ldaps"}}, false,
{:authentication => {:mode => "ldaps", :ldaphost => "foo"}}, true,
{:authentication => {:mode => "xxx"}}, false,
{:authentication => {:mode => "xxx"}}, false,

{:authentication => {:mode => "ldap", :ldaphost => "foo"}}, true,
{:authentication => {:mode => "ldap", :ldaphost => nil}}, false,
Expand Down
37 changes: 37 additions & 0 deletions spec/models/miq_server/configuration_management_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,41 @@
describe MiqServer, "::ConfigurationManagement" do
describe "#settings" do
shared_examples_for "#settings" do
it "with no changes in the database" do
settings = miq_server.settings
expect(settings).to be_kind_of(Hash)
expect(settings.fetch_path(:api, :token_ttl)).to eq("10.minutes")
end

it "with changes in the database" do
miq_server.settings_changes = [
FactoryGirl.create(:settings_change, :key => "/api/token_ttl", :value => "2.minutes")
]
Settings.reload!

settings = miq_server.settings
expect(settings).to be_kind_of(Hash)
expect(settings.fetch_path(:api, :token_ttl)).to eq("2.minutes")
end
end

context "local server" do
let(:miq_server) { EvmSpecHelper.local_miq_server }

before { stub_local_settings(miq_server) }

include_examples "#settings"
end

context "remote server" do
let(:miq_server) { EvmSpecHelper.remote_miq_server }

before { stub_local_settings(nil) }

include_examples "#settings"
end
end

describe "#get_config" do
shared_examples_for "#get_config" do
it "with no changes in the database" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@

described_class.new.configure

config = miq_server.get_config("vmdb")
expect(config.config.fetch_path(:authentication, :mode)).to eq("httpd")
expect(config.config.fetch_path(:authentication, :ldap_role)).to eq(false)
expect(config.config.fetch_path(:authentication, :httpd_role)).to eq(true)
settings = miq_server.settings
expect(settings.fetch_path(:authentication, :mode)).to eq("httpd")
expect(settings.fetch_path(:authentication, :ldap_role)).to eq(false)
expect(settings.fetch_path(:authentication, :httpd_role)).to eq(true)
end
end
end
4 changes: 2 additions & 2 deletions tools/change_server_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
server.zone = zone
server.save!

settings = server.get_config("vmdb")
settings.config[:server][:zone] = zone.name
settings = server.settings
settings[:server][:zone] = zone.name
server.set_config(settings)

server.save!
Expand Down
6 changes: 3 additions & 3 deletions tools/configure_server_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ def types_valid?(old_val, new_val)
exit 1
end

settings = server.get_config("vmdb")
settings = server.settings

path = settings.config
path = settings
keys = opts[:path].split("/")
key = keys.pop.to_sym
keys.each { |p| path = path[p.to_sym] }
Expand All @@ -89,7 +89,7 @@ def types_valid?(old_val, new_val)
puts "Setting [#{opts[:path]}], old value: [#{path[key]}], new value: [#{newval}]"
path[key] = newval

valid, errors = VMDB::Config::Validator.new(settings).validate
valid, errors = Vmdb::Settings.validate(settings)
unless valid
puts "ERROR: Configuration is invalid:"
errors.each { |k, v| puts "\t#{k}: #{v}" }
Expand Down

0 comments on commit c901870

Please sign in to comment.