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

[815] Resolve circular dependency when using sensu::enterprise::dashboard::api #816

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

glarizza
Copy link

@glarizza glarizza commented Sep 20, 2017

See issue #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, contains 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.

…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.
@ghoneycutt
Copy link
Collaborator

@agoddard Is getting JSON back from the API test enough or should we be testing something else?

@ghoneycutt
Copy link
Collaborator

@agoddard ping

@cwjohnston
Copy link
Contributor

@ghoneycutt getting a HTTP 200 and JSON response body back from the API is sufficient to indicate that the API is up and available. Are you looking for a way to test that the API is online or that the Dashboard is online?

@alvagante
Copy link
Collaborator

FYI, this is addressed also in #828

@ghoneycutt ghoneycutt merged commit 0476ca8 into sensu:master Oct 20, 2017
@ghoneycutt
Copy link
Collaborator

Released in v2.36.0

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.

5 participants