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

The fix for issue #192 broke service_managed false #197

Closed
mbaldessari opened this issue May 5, 2017 · 2 comments
Closed

The fix for issue #192 broke service_managed false #197

mbaldessari opened this issue May 5, 2017 · 2 comments
Labels
bug Something isn't working

Comments

@mbaldessari
Copy link
Contributor

I have the following hiera values:
redis::managed_by_cluster_manager": true
redis::service_manage": false
redis::notify_service": false

But now an 'include ::redis' gives me:
Error: Could not find dependent Service[redis] for Augeas[Systemd redis ulimit] at /etc/puppet/modules/redis/manifests/ulimit.pp:44

Did you mean to make https://github.com/arioch/puppet-redis/blob/master/manifests/ulimit.pp#L31 an 'elsif' and not an 'if'?

@petems
Copy link
Member

petems commented May 5, 2017

Good point, I should probably only refresh the service if it's being managed by Puppet.

In fact, since this is in the config class, it should automatically notify the service if it's enabled anyway.

I'll open a PR and also add an acceptance test with this setting to make sure this doesn't regress 😄

@mbaldessari
Copy link
Contributor Author

Perfect, thanks :)

petems added a commit to petems/puppet-redis that referenced this issue May 5, 2017
* If the redis service is unmanaged, like so:
```
redis::managed_by_cluster_manager": true
redis::service_manage": false
redis::notify_service": false
```
then you get a compilation failure as it tries to
look for the redis service to notify:
```
But now an 'include ::redis' gives me:
Error: Could not find dependent Service[redis] for Augeas[Systemd redis ulimit] at /etc/puppet/modules/redis/manifests/ulimit.pp:44
```
* In fact, we don't need any service notification 
in the redis::ulimit class, as it's already in the
redis::config class, which notifies the service 
class anyways
* Adds spec for this use-case to avoid regression

Closes voxpupuli#197
petems added a commit that referenced this issue May 5, 2017
* If the redis service is unmanaged, like so:
```
redis::managed_by_cluster_manager": true
redis::service_manage": false
redis::notify_service": false
```
then you get a compilation failure as it tries to
look for the redis service to notify:
```
But now an 'include ::redis' gives me:
Error: Could not find dependent Service[redis] for Augeas[Systemd redis ulimit] at /etc/puppet/modules/redis/manifests/ulimit.pp:44
```
* In fact, we don't need any service notification 
in the redis::ulimit class, as it's already in the
redis::config class, which notifies the service 
class anyways
* Adds spec for this use-case to avoid regression

Closes #197
@petems petems added the bug Something isn't working label May 11, 2017
cegeka-jenkins pushed a commit to cegeka/puppet-redis that referenced this issue Feb 16, 2021
* If the redis service is unmanaged, like so:
```
redis::managed_by_cluster_manager": true
redis::service_manage": false
redis::notify_service": false
```
then you get a compilation failure as it tries to
look for the redis service to notify:
```
But now an 'include ::redis' gives me:
Error: Could not find dependent Service[redis] for Augeas[Systemd redis ulimit] at /etc/puppet/modules/redis/manifests/ulimit.pp:44
```
* In fact, we don't need any service notification 
in the redis::ulimit class, as it's already in the
redis::config class, which notifies the service 
class anyways
* Adds spec for this use-case to avoid regression

Closes voxpupuli#197
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants