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

Add support for multiple network configuration options #732

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

ryan-dyer-sp
Copy link
Contributor

What does this PR do?

Updates the network integration to support the current set of options available.

Motivation

need support for ENA metrics in the network integration via puppet

Additional Notes

Describe your test plan

Just manual testing using the module and updating various variables to ensure they rendered conf.yaml contained the correct data.

@ryan-dyer-sp ryan-dyer-sp requested a review from a team as a code owner May 13, 2022 20:14
Copy link
Contributor

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

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

Hi 👋. Thanks a lot for submitting this PR. Overall it looks good, but I requested some minor changes (see the comments inline) to make it consistent with the actual defaults. Once these get fixed, I can merge.

manifests/integrations/network.pp Outdated Show resolved Hide resolved
manifests/integrations/network.pp Outdated Show resolved Hide resolved
manifests/integrations/network.pp Outdated Show resolved Hide resolved
templates/agent-conf.d/network.yaml.erb Show resolved Hide resolved
@ryan-dyer-sp
Copy link
Contributor Author

@bkabrda comments addressed except for the service one. If you're wanting me to add service to init_config along with that which is in instance, lmk.

@bkabrda
Copy link
Contributor

bkabrda commented Jun 1, 2022

@ryan-dyer-sp thanks for doing the changes. Could you possibly rebase on top of latest main branch? This should fix the CI - once that is green, I can merge the PR. Thanks!

Signed-off-by: Ryan Dyer <ryan-dyer-sp@users.noreply.github.com>
@ryan-dyer-sp ryan-dyer-sp force-pushed the add-network-options branch from d18598e to 80e1d62 Compare June 1, 2022 13:46
@ryan-dyer-sp
Copy link
Contributor Author

@bkabrda rebased and CI now passing

Copy link
Contributor

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks for the PR!

@bkabrda bkabrda merged commit 7e63d59 into DataDog:main Jun 2, 2022
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