-
Notifications
You must be signed in to change notification settings - Fork 290
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
Cyclical dependencies when using Sensu Enterprise and the Enterprise API #815
Comments
|
It looks like the sensu_enterprise_dashboard_api_config type has code to automatically setup a notify (https://github.com/sensu/sensu-puppet/blob/master/lib/puppet/type/sensu_enterprise_dashboard_api_config.rb#L11-L15) on the sensu-enterprise-dashboard service. When you use the Sensu::Enterprise::Dashboard::Api defined type, it's defining a sensu_enterprise_dashboard_api_config instance as well as setting up a dependency on the sensu::enterprise::dashboard class (https://github.com/sensu/sensu-puppet/blob/master/manifests/enterprise/dashboard/api.pp#L42). The type sets up a notify on the service (which implies 'before' as well as the refresh), and the class sets up a require on the sensu::enterprise::dashboard class which contains the Service['sensu-enterprise-dashboard] declaration (https://github.com/sensu/sensu-puppet/blob/master/manifests/enterprise/dashboard.pp#L82-L90), which creates a circular dependency. We can tackle this a number of ways (including getting the service in its own class so it can be included and not required), but I don't have as much familiarity with the module as @ghoneycutt does - do you have a recommendation Garrett before I start moving things around? |
|
We recently did work to pull in subclasses that did not stand on their own. So classes with names like ::config ::service ::install were consolidated and we don't want to go back to that model. |
|
Sure, that makes sense when there's no reason for those to stand alone (i.e. you're not doing multiple OSes). I'm going to continue working a couple of angles to find the best way to handle it |
…oard::api See issue sensu#815 on Github for more details. Previously, when declaring the sensu class along with an instance of the sensu::enterprise::dashboard::api defined type, a circular dependency was raised by Puppet. This was caused because first, the sensu::enterprise::dashboard::api defined type declares an instance of the sensu_enterprise_dashboard_api_config type, and the sensu_enterprise_dashboard_api_config type contains, within the initialize method, an automatic "notify" on the "sensu-enterprise-dashboard" service (i.e. Service['sensu-enterprise-dashboard']). The sensu::enterprise::dashboard::api defined type also uses the require function on the sensu::enterprise::dashboard class, which sets a 'require' dependency on every resource within the class. Because Service['sensu-enterprise-dashboard'] is located within the sensu::enterprise::dashboard class, that means that Service['sensu-enterprise-dashboard'] contains a circular dependency on any instance of the sensu::enterprise::dashboard::api defined type. To resolve the circular dependency, this commit changes the require function within the sensu::enterprise::dashboard::api defined type to the include function which eliminates the 'require' dependency. I tested this change out with vagrant and was able to get JSON back from the API, but I'd prefer someone with greater Sensu Enterprise knowledge to validate and ensure that converting 'require' to 'include' didn't upset any existing dependencies.
|
I suppose this one can be closed now @ghoneycutt |
|
Thanks @glarizza @alvagante |
Description of problem
Brought up
sensu-server-enterprisevagrant VM and applied the code below.Got an error about a cyclical dependency
No error.
Apply this puppet code. Note that
FACTER_SE_USERandFACTER_SE_PASSenvironment variables need to be populated.Command used and debugging output
Using the
sensu-server-enterprisevagrant VM.masterless
Platform and version information
# /opt/puppetlabs/puppet/bin/ruby --version ruby 2.1.9p490 (2016-03-30 revision 54437) [x86_64-linux]sensu-enterprise-2.6.1-1.noarchAnything else to add that you think will be helpful?
Start with figuring out what is different between this config which does not work and the config used in the VM which does.
The text was updated successfully, but these errors were encountered: