Skip to content

Conversation

@jessereynolds
Copy link
Contributor

@jessereynolds jessereynolds commented Aug 7, 2025

Replace puppetlabs-puppet_agent with puppet-openvox_bootstrap for automatic installation of openvox-agent rather than puppet-agent packages. (Nb, this doesn’t yet support Windows).

@jessereynolds jessereynolds added the backwards-incompatible This change will lead to a major version bump for the next release label Aug 7, 2025
@donoghuc
Copy link
Contributor

donoghuc commented Aug 7, 2025

I dont think you want another feature (feature in the senese of an attrribute on a target). I think from bolt's perspective the puppet-agent feature just means there is a "puppet librar\y" (meaning a ruby interpreter a puppet executable and a puppet gem w/ its deps) available on a target.

IMO what should be done here is to simply replace the puppet_agent module in the Puppetfile and point to that in the default puppet_library hook. Note you will want the openvox_bootstrap module to indicate what task to run with a bolt_plugin.json file. https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/main/bolt_plugin.json

@jpartlow
Copy link
Contributor

jpartlow commented Aug 7, 2025

I opened voxpupuli/puppet-openvox_bootstrap#34 for this.

What Cas is saying makes sense to me. Plus if we rename puppet-agent as the feature, we break existing plans that are testing for that feature.

@jpartlow
Copy link
Contributor

jpartlow commented Aug 7, 2025

I think I also have an incompatibility in openvox_bootstrap. It has a package parameter, because it can install any openvox package. That has a default of openvox-agent, but the type is String[1], rather than Optional[String], and I think that's breaking the specs here?

I think patching that to be Optional[String] is probably ok, although it feels a little loose to me.

@donoghuc
Copy link
Contributor

donoghuc commented Aug 7, 2025

I dont think you necessarily need to change the task spec. Bolt should just have a sane default parameter that is required here

DEFAULT_PLUGIN_HOOKS = { 'puppet_library' => { 'plugin' => 'puppet_agent', 'stop_service' => true } }.freeze

@jpartlow
Copy link
Contributor

jpartlow commented Aug 7, 2025

I dont think you necessarily need to change the task spec. Bolt should just have a sane default parameter that is required here

DEFAULT_PLUGIN_HOOKS = { 'puppet_library' => { 'plugin' => 'puppet_agent', 'stop_service' => true } }.freeze

Ah ok, so this pr needs something like:

 DEFAULT_PLUGIN_HOOKS = { 'puppet_library' => { 'plugin' => 'openvox_bootstrap', 'package' => 'openvox-agent', 'stop_service' => true } }.freeze

And then I need to implement stop_service in the openvox_bootstrap::install task. I thought the packages installed with the service stopped? Or perhaps that's not the case on Windows? It should be easy enough to add.

Does the bolt_plugin.json in the openvox_bootstrap module need any parameter defaults? It looks like that just names the task associated with puppet_library. Although, I'm also not quite sure what bolt_plugin.json does given the above default in the lib/bolt/plugin.rb :)

@donoghuc
Copy link
Contributor

donoghuc commented Aug 7, 2025

My reccolection of all the exact details is a but hazy... But yeah, that all seems about right!

Does the bolt_plugin.json in the openvox_bootstrap module need any parameter defaults? It looks like that just names the task associated with puppet_library. Although, I'm also not quite sure what bolt_plugin.json does given the above default in the lib/bolt/plugin.rb :)

No defaults in the bolt_plugin.json. All that does is let bolt know that this module has a puppet_library implementation and points it to that task. Anything about task parameter defaults, requirements, or implentations etc belongs in the task metadata that points to.

jpartlow added a commit to jpartlow/puppet-openvox_bootstrap that referenced this pull request Aug 8, 2025
Strictly speaking, given that the parameters have defaults, it should
not be necessary to type them 'Optional' in install.json.

But the install task is used as the puppet_library plugin for installing
openvox via apply_prep, and something about how Bolt constructs the task
there causes a failure for non optional parameters, even if they have
defaults:

  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ cat plans/test.pp
  plan kvm_automation_tooling::test() {
    apply_prep('b-alma9-ov8-agent-1')
  }
  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ be bolt plan run kvm_automation_tooling::test --inventory terraform/instances/inventory.b-alma9-ov8.yaml
  Starting: plan kvm_automation_tooling::test
  Starting: install puppet and gather facts on b-alma9-ov8-agent-1
  Finished: plan kvm_automation_tooling::test in 0.27 sec
  Failed on b-alma9-ov8-agent-1:
    Error executing plugin task from puppet_library: Task openvox_bootstrap::install:
     expects a value for parameter 'package'
  Failed on 1 target: b-alma9-ov8-agent-1
  Ran on 1 target

Some parameters will be wired into openbolt's DEFAULT_PLUGIN_HOOKS
(https://github.com/OpenVoxProject/openbolt/blob/b9bff5a8dfe2f41218281bf8c5e6ad901bcae460/lib/bolt/plugin.rb#L133)
and stop_service is already one of them.
(See OpenVoxProject/openbolt#49)

But to keep everything consistent, I'm setting all the parameters
optional with the defaults explicit so that it's at least clear to a
caller running 'bolt task show openvox_bootstrap::install' what must be
provided and what the defaults are.
jpartlow added a commit to jpartlow/puppet-openvox_bootstrap that referenced this pull request Aug 8, 2025
Strictly speaking, given that the parameters have defaults, it should
not be necessary to type them 'Optional' in install.json.

But the install task is used as the puppet_library plugin for installing
openvox via apply_prep, and something about how Bolt constructs the task
there causes a failure for non optional parameters, even if they have
defaults:

  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ cat plans/test.pp
  plan kvm_automation_tooling::test() {
    apply_prep('b-alma9-ov8-agent-1')
  }
  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ be bolt plan run kvm_automation_tooling::test --inventory terraform/instances/inventory.b-alma9-ov8.yaml
  Starting: plan kvm_automation_tooling::test
  Starting: install puppet and gather facts on b-alma9-ov8-agent-1
  Finished: plan kvm_automation_tooling::test in 0.27 sec
  Failed on b-alma9-ov8-agent-1:
    Error executing plugin task from puppet_library: Task openvox_bootstrap::install:
     expects a value for parameter 'package'
  Failed on 1 target: b-alma9-ov8-agent-1
  Ran on 1 target

Some parameters will be wired into openbolt's DEFAULT_PLUGIN_HOOKS
(https://github.com/OpenVoxProject/openbolt/blob/b9bff5a8dfe2f41218281bf8c5e6ad901bcae460/lib/bolt/plugin.rb#L133)
and stop_service is already one of them.
(See OpenVoxProject/openbolt#49)

But to keep everything consistent, I'm setting all the parameters
optional with the defaults explicit so that it's at least clear to a
caller running 'bolt task show openvox_bootstrap::install' what must be
provided and what the defaults are.
jpartlow added a commit to jpartlow/puppet-openvox_bootstrap that referenced this pull request Aug 8, 2025
Strictly speaking, given that the parameters have defaults, it should
not be necessary to type them 'Optional' in install.json.

But the install task is used as the puppet_library plugin for installing
openvox via apply_prep, and something about how Bolt constructs the task
there causes a failure for non optional parameters, even if they have
defaults:

  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ cat plans/test.pp
  plan kvm_automation_tooling::test() {
    apply_prep('b-alma9-ov8-agent-1')
  }
  jpartlow@archimedes:~/work/src/kvm_automation_tooling$ be bolt plan run kvm_automation_tooling::test --inventory terraform/instances/inventory.b-alma9-ov8.yaml
  Starting: plan kvm_automation_tooling::test
  Starting: install puppet and gather facts on b-alma9-ov8-agent-1
  Finished: plan kvm_automation_tooling::test in 0.27 sec
  Failed on b-alma9-ov8-agent-1:
    Error executing plugin task from puppet_library: Task openvox_bootstrap::install:
     expects a value for parameter 'package'
  Failed on 1 target: b-alma9-ov8-agent-1
  Ran on 1 target

Some parameters will be wired into openbolt's DEFAULT_PLUGIN_HOOKS
(https://github.com/OpenVoxProject/openbolt/blob/b9bff5a8dfe2f41218281bf8c5e6ad901bcae460/lib/bolt/plugin.rb#L133)
and stop_service is already one of them.
(See OpenVoxProject/openbolt#49)

But to keep everything consistent, I'm setting all the parameters
optional with the defaults explicit so that it's at least clear to a
caller running 'bolt task show openvox_bootstrap::install' what must be
provided and what the defaults are.
@jpartlow
Copy link
Contributor

jpartlow commented Aug 8, 2025

@jessereynolds there's a pr up at voxpupuli/puppet-openvox_bootstrap#35 to help with this.

If you add a temporary commit pinning openvox_bootstrap in the Puppetfile to that jpartlow:gh-35-add-stop-service-to-install branch we can see what happens to the tests here.

@jessereynolds
Copy link
Contributor Author

jessereynolds commented Aug 8, 2025

@jessereynolds there's a pr up at voxpupuli/puppet-openvox_bootstrap#35 to help with this.

If you add a temporary commit pinning openvox_bootstrap in the Puppetfile to that jpartlow:gh-35-add-stop-service-to-install branch we can see what happens to the tests here.

Great, thanks @jpartlow ! I’ll add that and see what it looks like. From this earlier test failure https://github.com/OpenVoxProject/openbolt/actions/runs/16810148208/job/47613105677?pr=49#step:8:347 it looks like either the install task will need to have a default for the ‘package’ param, or bolt will need to supply it. (correction, I see you’ve addressed that in jpartlow/puppet-openvox_bootstrap@6c07d34 )

I’ll also be re-doing this PR without the addition of the ‘openvox-agent’ as a synonym for ‘puppet-agent'

@jessereynolds jessereynolds force-pushed the openvox_bootstrap branch 2 times, most recently from 2538808 to ec2abaf Compare August 9, 2025 07:44
@jessereynolds
Copy link
Contributor Author

jessereynolds commented Aug 9, 2025

For interest I created a new branch that has this branch rebased on #48 to see if some tests might fare better. PR in my fork here: jessereynolds#10 … but now I’m wishing we had better data visualisation / quantification for the all the rspec examples (eg maybe something like https://github.com/SonicGarden/rspec-report-action or https://github.com/ctrf-io/github-test-reporter ) to help understand which examples are fixed, and which are newly broken, compared to this PR.

@jpartlow
Copy link
Contributor

For the Apply Tests on linux, the issue is that the openvox_bootstrap::check returns a :test_version value rather than a :version value, which could be fixed either place.

The Apply Tests on windows won't work until openbox_bootstrap gets windows support.

For the BoltSpec Tests, openbox_bootstrap::check isn't fulfilling the role the spec is expecting. It's expecting to be able to run it successfully regardless of whether openvox is installed on that node as a test of whether openvox is installed on that node. I wrote check to take some of the redundancy out of version testing for openbox_bootstrap tests. This could be sorted out either place.

The Linux Integration specs are having container/sudo issues for me locally, not certain if its the same problem as in gha, which has steps related to sudo; my brain isn't working well enough this week to sort it out.

The other two window spec clusters are, um, windowy.

Puppetfile Outdated
mod 'puppetlabs-service', '3.1.0'
mod 'puppetlabs-puppet_agent', '4.25.0'
mod 'puppet-openvox_bootstrap',
git: 'https://github.com/jpartlow/puppet-openvox_bootstrap.git',
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to remove this pin to Josh's fork now that the PR has been merged.

Might need a release to remove the git: ref entirely though

austb and others added 6 commits October 26, 2025 13:25
the puppet-openvox_bootstrap check task returns the version under a
puppet_Version key.

The full result currently looks like
```
[{"action"=>"task",
  "object"=>"openvox_bootstrap::check",
  "status"=>"success",
  "target"=>"ssh://bolt@localhost:20022",
  "value"=>{"puppet_version"=>"8.19.1", "valid"=>true}}]
```
openvox_bootstrap check task fails if puppet is not present, previously
puppet_agent::version would succeed and return a nil version.
@jessereynolds jessereynolds added the enhancement New feature or request label Oct 26, 2025
@jessereynolds jessereynolds marked this pull request as ready for review October 26, 2025 23:45
@austb austb mentioned this pull request Oct 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompatible This change will lead to a major version bump for the next release enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants