Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Puppet extraction #8510

Merged
merged 72 commits into from
Jul 19, 2021
Merged

Puppet extraction #8510

merged 72 commits into from
Jul 19, 2021

Conversation

ezr-ondrej
Copy link
Member

@ezr-ondrej ezr-ondrej commented May 6, 2021

This is a result of https://community.theforeman.org/t/making-puppet-optional/17668, please track the following conversations if you don't know why we are extracting and have questions.

I'm opening this for visibility, although it is still missing few bits, but those can go separatelly:

  • drop migrations
  • drop puppet proxies
  • related settings

@theforeman-bot
Copy link
Member

Issues: #31130

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I did a brief scan, but deprecated means it's still present but users should migrate away. This removes endpoints. That should be stated instead.

str << (comb.environment_id.nil? ? _("None") : comb.environment.to_s)
str.join(" / ")
end.to_sentence
template.template_combinations.map { |comb| comb.hostgroup.to_s }.to_sentence
Copy link
Member

Choose a reason for hiding this comment

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

Can't hostgroup still be nil?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well logically it should not, having template combination with no association makes no sense :)
Technically I hope not xD

@@ -1,5 +1,5 @@
module SmartProxiesHelper
TABBED_FEATURES = ["Puppet", "Puppet CA", "Logs"]
TABBED_FEATURES = ["Puppet CA", "Logs"]
Copy link
Member

Choose a reason for hiding this comment

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

It's weird to me that Puppet is removed but Puppet CA stays. Especially if assignment of hosts to a Puppetserver will remain.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed it is weird, that's because it is not finished, both Puppet and Puppet CA should be extracted, according to my last info.

@@ -15,8 +15,6 @@ module HostInfoExtensions
included do
# Add default providers
HostInfo.register_info_provider(HostInfoProviders::StaticInfo)
HostInfo.register_info_provider(HostInfoProviders::ConfigGroupsInfo)
HostInfo.register_info_provider(HostInfoProviders::PuppetInfo)
Copy link
Member

Choose a reason for hiding this comment

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

I really dislike that this removes the puppetmaster and puppet_ca parameters. Especially since those remain in the host model. I also think you'll drop the ip, ip6 and mac.

config/routes/api/v2.rb Outdated Show resolved Hide resolved
shiramax and others added 20 commits July 18, 2021 15:43
Removes uses of environment from Host
Remove uses of environment from Hostgroup.
Removes uses of Environment from Nic::Managed model
Removes uses of Environment from Report model.
Removes taxation of Environment.
Removes uses of Environment from User.

Extraction of `#visible_environments`:
  theforeman/foreman_puppet#126
Removes uses of Environment in PuppetFactParser and implement plugin
guard to import plugin Envrionment if plugin is installed.
Removes Environment related notes from tests, mostly just Host being
uselessly provided an environment.
We need to introduce a guard for Environment being present in settings,
as we are about to drop Environment.
Reimplementation and movement of the Puppet settings is separate commit.
This is completely dropped, in plugin we use factories instead.
We changed the namespace of the plugin to foreman_puppet.
These components are no longer used after puppet has been extracted.
This is implemented in theforeman/foreman_puppet#163
Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Thanks @ezr-ondrej and all others involved! Let's see what breaks 😝

@tbrisker tbrisker merged commit 91c28da into theforeman:develop Jul 19, 2021
@ares
Copy link
Member

ares commented Jul 19, 2021

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Demo worthy Headline feature candidate for headline feature Legacy JS PRs making changes in the legacy Javascript stack Needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants