Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def update_candlepin_associations(consumer_params = nil)
content_facet.cvenvs_changed = false if content_facet
content_facet&.save!

if subscription_facet
if subscription_facet && !subscription_facet.destroyed?
consumer_params ||= subscription_facet.consumer_attributes

host_uuid = consumer_params.dig(:facts, 'dmi.system.uuid')
Expand All @@ -48,7 +48,12 @@ def update_candlepin_associations(consumer_params = nil)
override_value ||= subscription_facet.update_dmi_uuid_override&.value
consumer_params[:facts]['dmi.system.uuid'] = override_value
end
::Katello::Resources::Candlepin::Consumer.update(subscription_facet.uuid, consumer_params)

begin
::Katello::Resources::Candlepin::Consumer.update(subscription_facet.uuid, consumer_params)
rescue RestClient::Gone, RestClient::NotFound
raise "Unable to update missing or deleted candlepin consumer uuid=#{subscription_facet.uuid} host_id=#{self.id}"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could go either way on this but as the bug shows RestClient rescues could hide bugs

end

if consumer_params.try(:[], :facts)
::Katello::Host::SubscriptionFacet.update_facts(self, consumer_params[:facts])
Expand Down
2 changes: 2 additions & 0 deletions app/models/katello/host/subscription_facet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ def candlepin_consumer
end

def backend_update_needed?
return false if self.destroyed?

%w(release_version service_level purpose_role purpose_usage).each do |method|
if self.send("#{method}_changed?")
Rails.logger.debug("backend_update_needed: subscription facet #{method} changed")
Expand Down
24 changes: 22 additions & 2 deletions test/factories/content_facet_factory.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
FactoryBot.define do
factory :katello_content_facets, :aliases => [:content_facet], :class => ::Katello::Host::ContentFacet do
# UUID removed - it belongs to subscription_facet, not content_facet
factory :katello_content_facet, :aliases => [:content_facet], :class => ::Katello::Host::ContentFacet do
host { association(:host, content_facet: @instance) }

trait :with_content_view_environment do
content_view_environments { [association(:katello_content_view_environment)] }
end

trait :with_content_source do
content_source { association(:smart_proxy, :with_pulp3) }
end

trait :with_kickstart_repository do
kickstart_repository { association(:katello_repository, :with_product) }
end

trait :with_applicable_errata do
applicable_errata { [association(:katello_erratum)] }
end

trait :with_bound_repositories do
bound_repositories { [association(:katello_repository)] }
end
end
end
19 changes: 3 additions & 16 deletions test/factories/content_view_environment_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,8 @@
# - environment: the lifecycle environment
# The content_view will be automatically set from the version

association :content_view_version, :factory => :katello_content_view_version

after(:build) do |cve, _evaluator|
# Set content_view from the version
if cve.content_view_version && !cve.content_view
cve.content_view = cve.content_view_version.content_view
end

# Set environment to library if not provided
unless cve.environment
org = cve.content_view&.organization || cve.content_view_version&.content_view&.organization
if org
cve.environment = org.library
end
end
end
content_view_version { association(:katello_content_view_version) }
content_view { content_view_version.content_view }
environment { association(:katello_environment, organization: content_view.organization) }
end
end
2 changes: 1 addition & 1 deletion test/factories/host_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
end

trait :with_content do
association :content_facet, :factory => :content_facet, :strategy => :build
association :content_facet, host: @instance, :factory => :content_facet, :strategy => :build

after(:build) do |host, evaluator|
if host.content_facet
Expand Down
5 changes: 5 additions & 0 deletions test/factories/katello_erratum_factory.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FactoryBot.define do
factory :katello_erratum, class: Katello::Erratum do
sequence(:pulp_id) { |n| n }
end
end
2 changes: 1 addition & 1 deletion test/factories/repository_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
trait :with_product do
with_content_view

product { FactoryBot.create(:katello_product, :with_provider, organization: organization) }
product { association(:katello_product, :with_provider, organization: organization) }
end

trait :deb do
Expand Down
7 changes: 7 additions & 0 deletions test/models/concerns/host_managed_extensions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,13 @@ def test_backend_update_needed?
assert subscription_facet.backend_update_needed?
end

def test_backend_update_needed_facet_destroyed
host = FactoryBot.build_stubbed(:host, :with_subscription)
host.subscription_facet.expects(:destroyed?).returns(true)

refute host.subscription_facet.backend_update_needed?
end
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def test_host_update_with_overridden_dmi_uuid
::Setting[:host_dmi_uuid_duplicates] = ['duplicate-dmi-uuid']
params = {facts: {'dmi.system.uuid' => 'duplicate-dmi-uuid'}}.with_indifferent_access
Expand Down
26 changes: 26 additions & 0 deletions test/models/concerns/subscription_facet_host_extensions_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
require 'katello_test_helper'

module Katello
class SubscriptionFacetHostExtensionsTest
class UpdateCandlepinAssociationsTest < ActiveSupport::TestCase
test "doesn't update candlepin if subscription facet was destroyed" do
host = FactoryBot.build_stubbed(:host, :with_content, :with_subscription)
host.content_facet.stubs(:save!)
host.subscription_facet.expects(:destroyed?).once.returns(true)
::Katello::Resources::Candlepin::Consumer.expects(:update).never

host.update_candlepin_associations
end

test "raises RuntimeError if consumer is gone" do
host = FactoryBot.build_stubbed(:host, :with_content, :with_subscription)
::Katello::Resources::Candlepin::Consumer.expects(:update).raises(RestClient::Gone)
host.content_facet.stubs(:save!)

assert_raises(RuntimeError, /missing or deleted/) do
host.update_candlepin_associations
end
end
end
end
end
Loading