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

feature: Add possibilty to not manage the openvpn service with puppet. #158

Merged
merged 1 commit into from
May 19, 2015
Merged

feature: Add possibilty to not manage the openvpn service with puppet. #158

merged 1 commit into from
May 19, 2015

Conversation

andrekeller
Copy link
Contributor

When you have redundant pairs of firewalls you sometimes want to use
other means to start/stop the service (i.e. keepalived scripts).

This is primarly a request for comments. If somebody has a better approach,
I'd be happy to dig into it.

hasstatus => true,
class openvpn::service ($manage_service = true) {

if $manage_service {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding a parameter, please reference $::openvpn::manage_service directly (as you do for the systemd part.

@luxflux
Copy link
Contributor

luxflux commented May 12, 2015

Well, the approach seems okay for me. I only had one comment, please check.

@andrekeller
Copy link
Contributor Author

I added it as parameter so I do not have to worry about the existing test case which invokes the service class directly. I'll give it another go.

@luxflux
Copy link
Contributor

luxflux commented May 12, 2015

You can handle this with a precondition:

let(:pre_condition) { 'class { "openvpn": manage_service => true }' }

@andrekeller
Copy link
Contributor Author

Hmm does not seem to work that way... Any idea?

@luxflux
Copy link
Contributor

luxflux commented May 12, 2015

Expand the facts to be like this, then it should work...

let(:facts) do
  {
    :osfamily => 'Debian',
    :operatingsystem => 'Debian',
    :concat_basedir => '/var/lib/puppet/concat'
  }
end

@andrekeller
Copy link
Contributor Author

Now it looks better, thank you.

Sorry this rspec stuff is pretty new to me, I do not yet fully understand the concepts and structure of these tests.

When you have redundant pairs of firewalls you sometimes want to use
other means to start/stop the service (i.e. keepalived scripts).
luxflux added a commit that referenced this pull request May 19, 2015
feature: Add possibilty to not manage the openvpn service with puppet.
@luxflux luxflux merged commit 87b1305 into voxpupuli:master May 19, 2015
@luxflux
Copy link
Contributor

luxflux commented May 19, 2015

Thanks for your work! Looks good! I will add a reverse test to complete it.

luxflux added a commit that referenced this pull request May 19, 2015
@andrekeller andrekeller deleted the feature-disable-service branch May 19, 2015 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants