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

Include 'datadog_agent' class in the catalog when using the generic i… #697

Merged
merged 1 commit into from
May 12, 2021

Conversation

stantona
Copy link
Contributor

…ntegration

What does this PR do?

This commit includes the datadog_agent class in the catalog when using datadog_agent::integrations::generic.

Motivation

When working with this integration, we received the following error:

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Comparison of: Undef Value > Integer, is not possible. Caused by 'Only Strings, Numbers, Timespans, Timestamps, and Versions are comparable'. (file: /<path>/datadog_agent/manifests/integrations/generic.pp, line: 26, column: 45) on node <host>

We noticed that the integration does not explicitly include datadog_agent in the catalog like the other integrations.

Describe your test plan

No detailed test plan beyond testing the before and after states. Including the class fixed the issue for us and seems to be the most straightforward and consistent solution that follows the general pattern for the integrations.

…ntegration

We encountered an issue when using `datadog_agent::integrations::generic` where the `::datadog_agent::_agent_major_version` variable was undefined. It looks like the class was not including the `datadog_agent` class in the catalog like all other integrations:

```
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Comparison of: Undef Value > Integer, is not possible. Caused by 'Only Strings, Numbers, Timespans, Timestamps, and Versions are comparable'. (file: /<path>/datadog_agent/manifests/integrations/generic.pp, line: 26, column: 45) on node <host>
```

This commit simply adds the include, bringing this integration in line with the rest.
@stantona stantona requested a review from a team as a code owner May 12, 2021 14:05
Copy link
Contributor

@albertvaka albertvaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to merge this for consistency, but I think the right way would be to require the datadog_agent class to be defined instead of just including it. The reason being that when this is the first include of that class in the catalog then it will be initialized with the default parameters. See #193 (which has been unfortunately ignored for a long time) for more details.

@albertvaka albertvaka merged commit 93ed6a6 into DataDog:master May 12, 2021
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.

2 participants