-
Notifications
You must be signed in to change notification settings - Fork 239
Add OpenVox support #935
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
base: master
Are you sure you want to change the base?
Add OpenVox support #935
Conversation
This is a resubmission of #933. I pasically added |
76dc42e
to
b43a016
Compare
a7a6572
to
b207a35
Compare
ah yeah, we didn't backport that to 7.x. We probably should. Lemme chat with the team. |
EDIT: found it! So I just installed this module and it's active, and I applied the https://github.com/theforeman/puppet-puppet/pull/935.diff for this PR and now I can install openvox with: if $facts['networking']['hostname'] == 'labrat' {
class {
'puppet':
runmode => 'cron',
client_package => 'openvox-agent',
agent_server_hostname => 'theforeman.example.com',
}
} Although it doesn't set up the repo yet. |
I think it's also required that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it reflect that OpenVox introduced an implementation
fact?
@bastelfreak Any plans to continue this? (looks like there are conflicts as #936 was already merged) |
de0091a
to
629ed30
Compare
629ed30
to
186387d
Compare
@bastelfreak The failling tests are not related to this change. Am I right? What do we need to merge this then? How can we help? |
186387d
to
0d9ed0b
Compare
|
3d4624e
to
b458215
Compare
This adds a fact that discovers if the Perforce or OpenVox agent is installed. If none are installed yet, we default to the Perforce package for backwards compatibility.
b458215
to
0804e8a
Compare
$server_package = "puppetserver${puppet_major}" | ||
# Add support for OpenVox. Default to puppet if nothing is installed yet | ||
$puppet_flavor = pick($facts['implementation'], 'puppet') | ||
$puppetserver_flavor = regsubst("${puppet_flavor}server", 'openvox', 'openvox-') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ewww. (no, I am not sure I can come with something more elegant, but wanted to express my eww here ;) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.
$puppetserver_flavor = bool2str($puppet_flavor == 'openvox', 'openvox-server', 'puppetserver')
Yes, this "breaks" any other flavor besides puppet and openvox, but… do we care?
} | ||
``` | ||
|
||
If you replace the Perforce packages and switch to the OpenVox implementation by hand, the module will detect this and just keep working, no changes required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not true for the terminus_package
, right? So why is it here? Did you want to say "This module works with both Puppet and Openvox (agent and server), but if you want DB you need to override the package name"?
$server_package = if ($facts['os']['family'] =~ /(FreeBSD|DragonFly)/) { | ||
"${puppetserver_flavor}${puppet_major}" | ||
} else { | ||
$server_package = undef | ||
$puppetserver_flavor | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This becomes puppet::server_package
:
puppet-puppet/manifests/init.pp
Line 704 in b8b34ff
Optional[Variant[String, Array[String]]] $server_package = $puppet::params::server_package, |
And then
puppet::server::package
:puppet-puppet/manifests/server.pp
Line 408 in b8b34ff
Optional[Variant[String, Array[String]]] $package = $puppet::server_package, |
And is then used in
puppet-puppet/manifests/server/install.pp
Line 36 in b8b34ff
$server_package = pick($puppet::server::package, 'puppetserver') |
and
$puppetserver_package = pick($puppet::server::package, 'puppetserver') |
The later two use pick
as they assume puppet::server::package
is sometimes Undef
, which it isn't anymore after this change.
I think that's totally fine, but should we then get rid of the pick
? Just to make it more obvious there are no more fallbacks?
👀 @ekohl
This adds a fact that discovers if the Perforce or OpenVox agent is installed. If none are installed yet, we default to the Perforce package for backwards compatibility.