Fixes #34110 - Delay host global status update

[ofedoren]

What are the changes introduced in this pull request?

The issue tells more about what the problem is. I suggest to delay the global status actual save since this is done here [1] afterwards.

Considerations taken when implementing this change?

I've spent a lot of time to dig to this line :/ My steps are following:

• [2] never gets triggered because sometimes saved_changes_to_build? is false, moreover @host.saved_changes are totally empty even if it should contain changes.
• Build exit can be simulated by calling @host.built(false), which is actually being called here [3].
• built method sets the build flag to false and saves/updates the host.
• If there is no orchestration code is called, then everything works.
• Otherwise that save in built is surrounded by [4], thus more tasks are being processed before save in built.
• One of the tasks that I've noticed will make the saved_changes empty is this one [5]
• [5] calls [6] which has refresh_global_status!, which actually does save on @host and thus we save changes to a host while trying to save changes to a host :/
• My main assumption is that there is callback hell in Rails when you don't trace which callback what actually does.

What are the testing steps for this pull request?

Have foreman_webhooks plugin installed, configured to listen to build_exited event, ensure it gets triggered. OR simply check the logs if build_exited.event.foreman gets triggered.

[1] - https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/models/host/managed.rb#L398
[2] - https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/models/host/managed.rb#L54
[3] - https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/controllers/hosts_controller.rb#L242-L243
[4] - https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/models/concerns/orchestration.rb#L12
[5] -

katello/app/models/katello/concerns/host_managed_extensions.rb

Line 61 in 635132d

after_validation :queue_refresh_content_host_status

[6] -

katello/app/models/katello/concerns/host_managed_extensions.rb

Line 108 in 635132d

def refresh_content_host_status

[7] - https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/models/host/managed.rb#L779

[theforeman-bot]

Issues: #34110

[lzap]

So I've spent all day this week reproducing this. First, it worked without Katello for me. Then I installed 6.10 and shellhooks would not work for me, it is giving me 404 and I cannot figure out why (if you have an idea). And in the evening, I was at least confirm in production.log this bug, yeah.

Omg just a bang! Great work.

[adamruzicka]

For what it is worth, this also fixes the issue with ansible roles not being applied post provisioning.

However, if I'm reading it right the non-bang variant just calculates a new value for one of the host's attributes, but since it gets set after the host is saved it is a bit pointless

[ofedoren]

@adamruzicka, what you makes think that global status gets set after the host is saved? This change is actually about not saving (changes to) a host before we actually save it in https://github.com/theforeman/foreman/blob/583ae2b619d813c71f176730a1da4790ecf0c3f1/app/models/host/managed.rb#L398.

I've checked via console that if I do an additional change here (

katello/app/models/katello/concerns/host_managed_extensions.rb

Lines 108 to 113 in 83a6327

	def refresh_content_host_status
	self.host_statuses.where(type: ::Katello::HostStatusManager::STATUSES.map(&:name)).each do |status|
	status.refresh!
	end
	refresh_global_status
	end

), e.g. changing a comment (since it simply sets a new value without storing), then this new comment is present in @host.saved_changes after built method finished.

The only similar concern I have is that if this refresh_content_host_status method is being called in other callbacks then it might not apply changes to global status if there is no @host.save afterwards...

[adamruzicka]

Must have been something in my previous environment that was interfering. Checked again with a fresh machine and it works flawlessly. Please disregard the second part my previous comment.

[adamruzicka]

Let's get this in

[jeremylenz]

ACK based on @lzap and @adamruzicka's testing. Thanks @ofedoren!