Skip to content

Fixes 38993: add OpenVox repository#10816

Open
d1nuc0m wants to merge 1 commit intotheforeman:developfrom
d1nuc0m:voxpupuli-repo
Open

Fixes 38993: add OpenVox repository#10816
d1nuc0m wants to merge 1 commit intotheforeman:developfrom
d1nuc0m:voxpupuli-repo

Conversation

@d1nuc0m
Copy link
Contributor

@d1nuc0m d1nuc0m commented Jan 14, 2026

Add OpenVox repository when relevant parameters are in use.

Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

I haven't tested this, but the changes look correct to me

<%= snippet 'puppet_setup' %>
<% end -%>

<% if !host_param_true?('skip-openvox-setup') && (host_puppet_server.present? || host_param_true?('force-openvox')) -%>
Copy link
Member

Choose a reason for hiding this comment

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

I think this will mean it runs both Puppet and OpenVox setup. In app/views/unattended/provisioning_templates/user_data/userdata_default.erb you solved it with a force-puppet parameter and IMHO this part should be aligned.

Copy link
Contributor Author

@d1nuc0m d1nuc0m Jan 26, 2026

Choose a reason for hiding this comment

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

Thinking about it maybe it should be like this:

  • if Puppet is explicitly forced, install it
  • Otherwise, default to OpenVox (instead of Puppet) if a Puppet Server is present and OpenVox setup is not explicitly skipped; or if OpenVox setup is forced

Btw, which should be the proper way to add an exception in case both force-puppet and force-openvox are true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And, is there a way to add tests to templates?

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it maybe it should be like this:

I think that's good

Btw, which should be the proper way to add an exception in case both force-puppet and force-openvox are true?

I'd also be OK with just assuming OpenVox then. In environments where Katello is used you control the availability via synced content anyway and it's irrelevant. You only need the configuration to be done.

And, is there a way to add tests to templates?

We render snapshots, so that can be used. It's a fairly heavy process though because it usually results in huge files to be included.

Copy link
Contributor Author

@d1nuc0m d1nuc0m Jan 27, 2026

Choose a reason for hiding this comment

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

I'd also be OK with just assuming OpenVox then

Ok, it should work like this now

We render snapshots, so that can be used. It's a fairly heavy process though

There isn't something like Rspec to render templates with variables and check if they contain/do not contain what's expected?

Copy link
Contributor Author

@d1nuc0m d1nuc0m Jan 28, 2026

Choose a reason for hiding this comment

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

Thanks, I tried to write a test based on your code and existing tests, where could I find some docs for FactoryBot.create? I'd like to add more OSes. Also, is the test part of the suite now or should I add it somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

We use factory_bot which has its own documentation: https://thoughtbot.github.io/factory_bot/

Copy link
Member

Choose a reason for hiding this comment

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

Also, in #9516 there's some work to clean up the factories and make them more extensible..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I am implementing Debian/Ubuntu tests and just discovered it doesn't work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ekohl done, to add OSes like Ubuntu 24.04, it should be added to test/factories/operatingsystem.rb before? And, is it enough to place the voxpupuli repo test file in test/unit/foreman/templates/snippets or I should declare it somewhere?

@d1nuc0m d1nuc0m force-pushed the voxpupuli-repo branch 8 times, most recently from c80a5b2 to 62c80b9 Compare January 28, 2026 13:31
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 think this is starting to look very good.

- |
<%= indent(2) { snippet('chef_client') } %>
<% end -%>
<% if openvox_enabled -%>
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to prevent the case where both Vox and Puppet repos are enabled so perhaps this can be:

  • if openvox_enabledsnippet('voxpupuli_repo')
  • else if puppet_enabledsnippet('puppetlabs_repo')
  • if openvox_enabled or puppet_enabledsnippet('puppet_setup')

That probably needs some redefinition of puppet_enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll try after Fosdem/CfgMgmtCamp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm back, had some "offline" issues meanwhile

<% if host_param_true?('enable-puppetlabs-repo') || host_param_true?('enable-official-puppet8-repo') || host_param_true?('enable-official-puppet7-repo') || host_param_true?('enable-puppetlabs-puppet6-repo') || host_param_true?('enable-puppetlabs-puppet5-repo') -%>
<%= indent(2) { snippet 'puppetlabs_repo' } %>
<% end -%>
<% if host_param_true?('enable-openvox8') || host_param_true?('enable-openvox7') -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

actually, enabling the openvox repos is no necessary, if the packages are provided by katello. Can you please add a guard?
This can be done at this stage or later on in the voxpupuli_repo snippet.

Copy link
Contributor

@sbernhard sbernhard left a comment

Choose a reason for hiding this comment

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

The voxpupuli_repo repo setup is not required, if the openvox packages are delivered by katello.

@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Feb 25, 2026

@sbernhard any suggestion on how to implement it? I never used Katello and was copying the Puppet setup

@sbernhard
Copy link
Contributor

@sbernhard any suggestion on how to implement it? I never used Katello and was copying the Puppet setup

One possibility would be to "just" check for:
if (plugin_present?('katello') in the template.

But this would mean, that for every katello we would expect that openvox is delivered through katello (which I would prefer if I would katello).

Another idea would be, to check for an additonal host param like "use-openvox-voxpupuli-repo" - this parameter can then be used, if maybe a user want to use the openvox packages delieverd by the OS vendor, too.

@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Mar 2, 2026

Another idea would be, to check for an additonal host param like "use-openvox-voxpupuli-repo" - this parameter can then be used, if maybe a user want to use the openvox packages delieverd by the OS vendor, too.

userdata_default.erb already has a skip-openvox-setup variable, maybe is extending that a solution?

@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Mar 5, 2026

@ekohl can I ask for help with the tests? I have added a file to test puppet_setup snippet, and locally it fails with

PuppetSetupTest::RHEL 9#test_0001_enable-openvox8:
NoMethodError: undefined method `find_snippet' for #<OpenStruct name="Test", content="<%#\nkind: snippet\nname: puppet_setup\nmodel: ProvisioningTemplate\nsnippet: true\ndescription: 
(...) 
app/services/foreman/renderer/scope/macros/snippet_rendering.rb:49:in `snippet'
app/services/foreman/renderer/safe_mode_renderer.rb:7:in `render'
app/services/foreman/renderer/base_renderer.rb:18:in `render'
test/unit/foreman/templates/snippets/puppet_setup.rb:25:in `render_template'
test/unit/foreman/templates/snippets/puppet_setup.rb:36:in `block (2 levels) in <class:PuppetSetupTest>'

(etc, same for other OSes)

I don't know how to handle it, and the GH pipeline reports tests are successful, so I guess these new two aren't being run?

@d1nuc0m d1nuc0m force-pushed the voxpupuli-repo branch 3 times, most recently from 0d96b86 to 2d18d15 Compare March 6, 2026 11:15
* Add OpenVox repository when relevant parameters are in use
* Test VoxPupuli repository and Puppet setup snippets when OpenVox
  parameters are set
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.

4 participants