Skip to content

Commit

Permalink
Fixes #30175,#30195 - Moving Quota validation to before_save in Ovirt…
Browse files Browse the repository at this point in the history
… CR (theforeman#7774)

Co-authored-by: Shira Maximov <shiramaximov@gmail.com>
  • Loading branch information
yifatmakias and shiramax committed Jul 15, 2020
1 parent f49b344 commit 389f5fc
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
4 changes: 2 additions & 2 deletions app/models/compute_resources/foreman/model/ovirt.rb
Expand Up @@ -11,6 +11,7 @@ class Ovirt < ComputeResource
validates :keyboard_layout, :inclusion => { :in => ALLOWED_KEYBOARD_LAYOUTS }
validates :user, :password, :presence => true
after_validation :connect, :update_available_operating_systems unless Rails.env.test?
before_save :validate_quota

alias_attribute :datacenter, :uuid

Expand Down Expand Up @@ -494,7 +495,7 @@ def nictypes
]
end

def validate_quota(client)
def validate_quota
if attrs[:ovirt_quota_id].nil?
attrs[:ovirt_quota_id] = client.quotas.first.id
else
Expand Down Expand Up @@ -525,7 +526,6 @@ def client
:api_version => use_v4? ? 'v4' : 'v3'
)
client.datacenters
validate_quota(client)
@client = client
rescue => e
if e.message =~ /SSL_connect.*certificate verify failed/ ||
Expand Down
4 changes: 4 additions & 0 deletions test/controllers/hosts_controller_test.rb
Expand Up @@ -1344,8 +1344,12 @@ class Host::Valid < Host::Managed; end
end

test "#host update shouldn't diassociate from VM" do
require 'fog/ovirt/models/compute/quota'
hostgroup = FactoryBot.create(:hostgroup, :with_environment, :with_subnet, :with_domain, :with_os)
compute_resource = compute_resources(:ovirt)
quota = Fog::Ovirt::Compute::Quota.new({ :id => '1', :name => "Default" })
client_mock = mock.tap { |m| m.stubs(datacenters: [], quotas: [quota]) }
compute_resource.stubs(:client).returns(client_mock)
compute_resource.update(:locations => hostgroup.locations, :organizations => hostgroup.organizations)
host = FactoryBot.create(:host, :hostgroup => hostgroup, :compute_resource => compute_resource)
host_attributes = host.attributes
Expand Down
12 changes: 8 additions & 4 deletions test/models/compute_resources/ovirt_test.rb
Expand Up @@ -62,6 +62,7 @@ class Foreman::Model:: OvirtTest < ActiveSupport::TestCase
end
@compute_resource = FactoryBot.build(:ovirt_cr)
@host = FactoryBot.build(:host, :mac => 'ca:d0:e6:32:16:97')
@quota = Fog::Ovirt::Compute::Quota.new({ :id => '1', :name => "Default" })
end

it 'maps operating system to ovirt operating systems' do
Expand Down Expand Up @@ -90,13 +91,15 @@ class Foreman::Model:: OvirtTest < ActiveSupport::TestCase
it 'caches the operating systems in the compute resource' do
client_mock = mock.tap { |m| m.stubs(:operating_systems).returns(@ovirt_oses) }
@compute_resource.stubs(:client).returns(client_mock)
client_mock.stubs(:quotas).returns([@quota])
assert @compute_resource.supports_operating_systems?
assert_equal @os_hashes, @compute_resource.available_operating_systems
end

it 'handles a case when the operating systems endpoint is missing' do
client_mock = mock.tap { |m| m.stubs(:operating_systems).raises(Fog::Ovirt::Errors::OvirtEngineError, StandardError.new('404')) }
@compute_resource.stubs(:client).returns(client_mock)
client_mock.stubs(:quotas).returns([@quota])
refute @compute_resource.supports_operating_systems?
end
end
Expand Down Expand Up @@ -145,26 +148,27 @@ class Foreman::Model:: OvirtTest < ActiveSupport::TestCase
@compute_resource = FactoryBot.build(:ovirt_cr)
@quota = Fog::Ovirt::Compute::Quota.new({ :id => '1', :name => 'Default' })
@client_mock = mock.tap { |m| m.stubs(datacenters: [], quotas: [@quota]) }
@compute_resource.stubs(:client).returns(@client_mock)
end

test 'quota validation - id entered' do
@compute_resource.ovirt_quota = '1'
assert_equal('1', @compute_resource.validate_quota(@client_mock))
assert_equal('1', @compute_resource.validate_quota)
end

test 'quota validation - name entered' do
@compute_resource.ovirt_quota = 'Default'
assert_equal('1', @compute_resource.validate_quota(@client_mock))
assert_equal('1', @compute_resource.validate_quota)
end

test 'quota validation - nothing entered' do
assert_equal('1', @compute_resource.validate_quota(@client_mock))
assert_equal('1', @compute_resource.validate_quota)
end

test 'quota validation - name entered' do
@compute_resource.ovirt_quota = 'Default2'
assert_raise Foreman::Exception do
@compute_resource.validate_quota(@client_mock)
@compute_resource.validate_quota
end
end
end
Expand Down

0 comments on commit 389f5fc

Please sign in to comment.