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

Fixes #33137 - Align with puppet moving to a plugin #494

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

xprazak2
Copy link
Contributor

Since the plan is to have foreman_puppet installed for both new and existing users on 3.0, we do not need to switch policy deploy type just yet.

@ekohl
Copy link
Member

ekohl commented Jul 26, 2021

Since the plan is to have foreman_puppet installed for both new and existing users on 3.0, we do not need to switch policy deploy type just yet.

Note that we do plan on making it possible, but it'll be experimental.

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@xprazak2
Copy link
Contributor Author

xprazak2 commented Aug 2, 2021

Test are quite red, though passing for me locally. I'll need to take a closer look.

@xprazak2 xprazak2 force-pushed the extract-puppet branch 2 times, most recently from 39642fe to 45ea62a Compare August 4, 2021 11:52
@@ -21,13 +21,17 @@ def inline_help
}
end

def collection_method
:puppetclasses
Copy link
Member

Choose a reason for hiding this comment

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

This will not work, HG, nor Host exposes puppetclasses and all_puppetclasses.
It has moved to (host || hg).puppet (what is a Facet for puppet)

Copy link
Member

Choose a reason for hiding this comment

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

couldn't we let the config to extract the collection? it would give us more flexibility and we could call the object.puppet.collection 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do that as an emergency solution, it should work with theforeman/foreman_puppet#180

@ezr-ondrej
Copy link
Member

This will work once the 1.0 puppet is released

@lzap
Copy link
Member

lzap commented Aug 19, 2021

Tests are failing, any plans to merge this?

@xprazak2
Copy link
Contributor Author

Tests on Jenkins will not pass due to missing dependencies, but GH actions are green, so I think this is ready.

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Looks ready enough to me 👍

@lzap
Copy link
Member

lzap commented Aug 23, 2021

Would be fantastic to have it, I am working on some big refactoring of the plugin and green tests would be very welcome :-)

@xprazak2 xprazak2 merged commit d2e08d3 into theforeman:master Aug 23, 2021
@xprazak2
Copy link
Contributor Author

Thanks everyone!

@lzap
Copy link
Member

lzap commented Aug 23, 2021

Running tests locally, crossing my fingers! No luck...

Traceback (most recent call last):
	6: from /home/lzap/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:5:in `<main>'
	5: from /home/lzap/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:5:in `select'
	4: from /home/lzap/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:17:in `block in <main>'
	3: from /home/lzap/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:17:in `require'
	2: from /home/lzap/work/foreman_openscap/test/functional/api/v2/compliance/arf_reports_controller_test.rb:1:in `<top (required)>'
	1: from /home/lzap/work/foreman_openscap/test/functional/api/v2/compliance/arf_reports_controller_test.rb:1:in `require'
/home/lzap/work/foreman_openscap/test/test_plugin_helper.rb:8:in `<top (required)>': uninitialized constant ForemanPuppet (NameError)
rake aborted!

This fails here:

  # Add factories from foreman_ansible
  FactoryBot.definition_file_paths << File.join(ForemanAnsible::Engine.root, '/test/factories')
  FactoryBot.definition_file_paths << File.join(ForemanPuppet::Engine.root, '/test/factories') # HERE
  FactoryBot.reload

I think we need a defined? there? I also see few tests failing, they are all assuming that Puppet plugin is present. I don't have it installed at the moment.

@lzap
Copy link
Member

lzap commented Aug 23, 2021

Filed a PR to fix this: #500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants