-
Notifications
You must be signed in to change notification settings - Fork 261
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
Conversation
There was a problem hiding this 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.
@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. |
@ryan-dyer-sp thanks for doing the changes. Could you possibly rebase on top of latest |
Signed-off-by: Ryan Dyer <ryan-dyer-sp@users.noreply.github.com>
d18598e
to
80e1d62
Compare
@bkabrda rebased and CI now passing |
There was a problem hiding this 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!
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.