You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The defined(Service[$puppet_service_name]) pattern in this module is parse-order dependent, and I was unable to declare a service { 'puppetserver' ...} in some code I was writing because this module was parsed first and had already declared the resource.
To work around this, I've added a parameter to the class, which defaults to the existing behavior of true so it won't break existing behavior for anyone. However, one can override this to false which skips the creation of the $puppet_service_name service resource in this module and thereby avoids a duplicate resource error when compiling the catalog.
First, by way of consistency, what do you think about changing this param to be named manage_puppet_service or something similar?
Second, could we ask for a test case (probably in spec/unit/classes/master/config_spec.rb) to exercise this? It's clearly simple code, but it'll help us keep it working in the future.
My personal opinion is that manage_puppet_service is not an appropriate name, because this code is really not managing the service. Rather it's creating the resource if it doesn't exist yet in the portion of the catalog that's been evaluated thus far. While I understand why you're doing this, subtle changes can suddenly cause this to break - in my particular case it was the simple rename of a class that caused that things to be evaluated in a different order and therefore start throwing a duplicate resource error. So I personally believe that "create puppet service resource" is exactly what this is doing, but it's not really managing anything. However I'll defer to your wishes regarding the variable name, so if you want manage_puppet_service or something else, even in light of this explanation, tell me what you want it to be named and I'll make it happen.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
defined(Service[$puppet_service_name])pattern in this module is parse-order dependent, and I was unable to declare aservice { 'puppetserver' ...}in some code I was writing because this module was parsed first and had already declared the resource.To work around this, I've added a parameter to the class, which defaults to the existing behavior of
trueso it won't break existing behavior for anyone. However, one can override this tofalsewhich skips the creation of the$puppet_service_nameservice resource in this module and thereby avoids a duplicate resource error when compiling the catalog.