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

Fix `updated_by_last_action` value of `monitor` provider #229

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@olivielpeau
Copy link
Member

commented Sep 10, 2015

Replaces #168 (see discussion there for more context on this issue)

use_inline_resources is the way to go to ensure that updated_by_last_action is correct.

It makes the provider actions run in their own run context, so we need to redefine the datadog-agent service in each action.

The behavior of the provider is preserved, except that the datadog-agent service gets restarted every time a config file is updated (instead of only being restarted once) when multiple files are updated in the same run.

Fix `updated_by_last_action` value of `monitor` provider
`use_inline_resources` is the way to go to ensure that
`updated_by_last_action` is correct.

It makes the provider actions run in their own run context,
so we need to redefine the `datadog-agent` service in each action.

@olivielpeau olivielpeau added the feature label Sep 10, 2015

@olivielpeau olivielpeau added this to the Next minor milestone Sep 10, 2015

@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2015

@olivielpeau I'm not certain I follow. I think the desire is to have datadog-agent restart once, at the end of the run, if there are any services notifying it to do so - this appears to define extra resources with the same name (which causes the infamous resource cloning), and I'm not sure that's the correct behavior, since we can have multiple restarts per addition to the file.

I'm also not sure about the removal of the updated_by_last_action - that's something we'd likely have to test - can you conceive of a way to test that?

@olivielpeau

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2015

@miketheman I agree that restarting the service multiple times per run is a big issue...

Given that the provider's actions are executed in their own run contexts, the only way that I see to notify the service in the main run context would be to remove the notifications in the provider and add:

notifies :restart, 'service[datadog-agent]', :delayed

to every datadog_monitor resource block. But that changes the scope of what the resource does.
Unless I'm missing something.

I'm not sure what we could test exactly on the updated_by_last_action. Basically its current value doesn't make much sense (for instance it's always false for the add action, even when the config file has been changed). But I'm not sure I understand what you mean by testing its removal.

We could definitely test the whole provider with chefspec though.

@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Oct 15, 2015

Thanks @olivielpeau !

@miketheman miketheman deleted the olivielpeau/provider-updated-value branch Oct 15, 2015

@miketheman miketheman added optimize bug and removed feature optimize labels Oct 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.